Skip to content

Commit

Permalink
Test cleanup and simplification (apache#4799)
Browse files Browse the repository at this point in the history 
* Simplified assert statements in the tests. Switch to usage of static imports in tests. (Part 1)

* Simplify assert statements in the tests and use the appropriate assert statements. Switch to usage of static imports in tests. Remove unused imports (Part 2)
  • Loading branch information
@vzhikserg @merlimat
vzhikserg authored and merlimat committed Jul 25, 2019
1 parent 87c02a9 commit fb6895d
Show file tree
Hide file tree
Showing 134 changed files with 1,262 additions and 1,300 deletions.
9 changes: 4 additions & 5 deletions managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/EntryCacheManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;

import java.util.List;
Expand All @@ -29,8 +30,6 @@
import org.apache.bookkeeper.common.util.OrderedScheduler;
import org.apache.bookkeeper.mledger.Entry;
import org.apache.bookkeeper.mledger.ManagedCursor;
import org.apache.bookkeeper.mledger.ManagedLedger;
import org.apache.bookkeeper.mledger.ManagedLedgerFactory;
import org.apache.bookkeeper.mledger.ManagedLedgerFactoryConfig;
import org.apache.bookkeeper.test.MockedBookKeeperTestCase;
import org.testng.annotations.BeforeClass;
Expand Down Expand Up @@ -128,13 +127,13 @@ void doubleInsert() throws Exception {
EntryCacheManager cacheManager = factory.getEntryCacheManager();
EntryCache cache1 = cacheManager.getEntryCache(ml1);

assertEquals(cache1.insert(EntryImpl.create(1, 1, new byte[4])), true);
assertEquals(cache1.insert(EntryImpl.create(1, 0, new byte[3])), true);
assertTrue(cache1.insert(EntryImpl.create(1, 1, new byte[4])));
assertTrue(cache1.insert(EntryImpl.create(1, 0, new byte[3])));

assertEquals(cache1.getSize(), 7);
assertEquals(cacheManager.getSize(), 7);

assertEquals(cache1.insert(EntryImpl.create(1, 0, new byte[5])), false);
assertFalse(cache1.insert(EntryImpl.create(1, 0, new byte[5])));

assertEquals(cache1.getSize(), 7);
assertEquals(cacheManager.getSize(), 7);
Expand Down
14 changes: 8 additions & 6 deletions ...ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorConcurrencyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
package org.apache.bookkeeper.mledger.impl;

import static org.apache.bookkeeper.mledger.util.SafeRun.safeRun;
import static org.testng.Assert.*;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNull;

import com.google.common.collect.Lists;
import java.util.List;
Expand Down Expand Up @@ -126,7 +128,7 @@ public void run() {

counter.await();

assertEquals(gotException.get(), false);
assertFalse(gotException.get());
}

@Test
Expand Down Expand Up @@ -210,7 +212,7 @@ public void closeFailed(ManagedLedgerException exception, Object ctx) {

counter.await();

assertEquals(gotException.get(), false);
assertFalse(gotException.get());
assertEquals(closeFuture.get(), CLOSED);
}

Expand Down Expand Up @@ -266,7 +268,7 @@ public void testAckAndClose() throws Exception {

counter.await();

assertEquals(gotException.get(), false);
assertFalse(gotException.get());
}

@Test(timeOut = 30000)
Expand Down Expand Up @@ -312,7 +314,7 @@ public void testConcurrentIndividualDeletes() throws Exception {

counter.await();

assertEquals(gotException.get(), false);
assertFalse(gotException.get());
assertEquals(cursor.getMarkDeletedPosition(), addedEntries.get(addedEntries.size() - 1));
}

Expand All @@ -338,7 +340,7 @@ public void testConcurrentReadOfSameEntry() throws Exception {
final CyclicBarrier barrier = new CyclicBarrier(numCursors);
final CountDownLatch counter = new CountDownLatch(numCursors);

AtomicReference<String> result = new AtomicReference<String>();
AtomicReference<String> result = new AtomicReference<>();

for (int i = 0; i < numCursors; i++) {
final int cursorIndex = i;
Expand Down
12 changes: 8 additions & 4 deletions ...d-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorContainerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
*/
package org.apache.bookkeeper.mledger.impl;

import static org.testng.Assert.*;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;

import com.google.common.base.Predicate;
import com.google.common.collect.Lists;
Expand Down Expand Up @@ -309,7 +313,7 @@ public double getThrottleMarkDelete() {
@Test
void simple() throws Exception {
ManagedCursorContainer container = new ManagedCursorContainer();
assertEquals(container.getSlowestReaderPosition(), null);
assertNull(container.getSlowestReaderPosition());

ManagedCursor cursor1 = new MockManagedCursor(container, "test1", new PositionImpl(5, 5));
container.add(cursor1);
Expand Down Expand Up @@ -350,7 +354,7 @@ void simple() throws Exception {
assertFalse(container.isEmpty());

container.removeCursor(cursor4.getName());
assertEquals(container.getSlowestReaderPosition(), null);
assertNull(container.getSlowestReaderPosition());

assertTrue(container.isEmpty());

Expand Down Expand Up @@ -409,7 +413,7 @@ void removingCursor() throws Exception {

assertEquals(container, Lists.newArrayList(cursor1, cursor3));

assertEquals(container.get("test2"), null);
assertNull(container.get("test2"));

assertEquals(container.getSlowestReaderPosition(), new PositionImpl(1, 1));

Expand Down
60 changes: 32 additions & 28 deletions managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.testng.Assert.*;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotEquals;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;

import com.google.common.base.Charsets;
import com.google.common.collect.Lists;
Expand Down Expand Up @@ -221,19 +226,19 @@ void testNumberOfEntries() throws Exception {
ManagedCursor c5 = ledger.openCursor("c5");

assertEquals(c1.getNumberOfEntries(), 4);
assertEquals(c1.hasMoreEntries(), true);
assertTrue(c1.hasMoreEntries());

assertEquals(c2.getNumberOfEntries(), 3);
assertEquals(c2.hasMoreEntries(), true);
assertTrue(c2.hasMoreEntries());

assertEquals(c3.getNumberOfEntries(), 2);
assertEquals(c3.hasMoreEntries(), true);
assertTrue(c3.hasMoreEntries());

assertEquals(c4.getNumberOfEntries(), 1);
assertEquals(c4.hasMoreEntries(), true);
assertTrue(c4.hasMoreEntries());

assertEquals(c5.getNumberOfEntries(), 0);
assertEquals(c5.hasMoreEntries(), false);
assertFalse(c5.hasMoreEntries());

List<Entry> entries = c1.readEntries(2);
assertEquals(entries.size(), 2);
Expand Down Expand Up @@ -331,13 +336,13 @@ void testNumberOfEntriesWithReopen() throws Exception {
c3 = ledger.openCursor("c3");

assertEquals(c1.getNumberOfEntries(), 2);
assertEquals(c1.hasMoreEntries(), true);
assertTrue(c1.hasMoreEntries());

assertEquals(c2.getNumberOfEntries(), 1);
assertEquals(c2.hasMoreEntries(), true);
assertTrue(c2.hasMoreEntries());

assertEquals(c3.getNumberOfEntries(), 0);
assertEquals(c3.hasMoreEntries(), false);
assertFalse(c3.hasMoreEntries());

factory2.shutdown();
}
Expand Down Expand Up @@ -538,7 +543,7 @@ void testResetCursor() throws Exception {
}

assertTrue(moveStatus.get());
assertTrue(cursor.getReadPosition().equals(resetPosition));
assertEquals(resetPosition, cursor.getReadPosition());
cursor.close();
ledger.close();
}
Expand Down Expand Up @@ -571,7 +576,7 @@ public void resetFailed(ManagedLedgerException exception, Object ctx) {
});
countDownLatch.await();
assertTrue(moveStatus.get());
assertTrue(cursor.getReadPosition().equals(resetPosition));
assertEquals(resetPosition, cursor.getReadPosition());
cursor.close();
ledger.close();
}
Expand Down Expand Up @@ -624,7 +629,7 @@ public void resetFailed(ManagedLedgerException exception, Object ctx) {
}
});
countDownLatch.await();
assertTrue(cursor.getReadPosition().equals(resetPosition));
assertEquals(resetPosition, cursor.getReadPosition());
cursor.close();

return moveStatus;
Expand Down Expand Up @@ -779,7 +784,7 @@ void markDeleteSkippingMessage() throws Exception {
assertEquals(cursor.getNumberOfEntries(), 4);

cursor.markDelete(p1);
assertEquals(cursor.hasMoreEntries(), true);
assertTrue(cursor.hasMoreEntries());
assertEquals(cursor.getNumberOfEntries(), 3);

assertEquals(cursor.getReadPosition(), p2);
Expand All @@ -790,7 +795,7 @@ void markDeleteSkippingMessage() throws Exception {
entries.forEach(e -> e.release());

cursor.markDelete(p4);
assertEquals(cursor.hasMoreEntries(), false);
assertFalse(cursor.hasMoreEntries());
assertEquals(cursor.getNumberOfEntries(), 0);

assertEquals(cursor.getReadPosition(), new PositionImpl(p4.getLedgerId(), p4.getEntryId() + 1));
Expand Down Expand Up @@ -1371,7 +1376,7 @@ void testSkipEntries(boolean useOpenRangeSet) throws Exception {
// skip more than the current set of entries
c1.skipEntries(10, IndividualDeletedEntries.Exclude);
assertEquals(c1.getNumberOfEntries(), 0);
assertEquals(c1.hasMoreEntries(), false);
assertFalse(c1.hasMoreEntries());
assertEquals(c1.getReadPosition(), pos.getNext());
assertEquals(c1.getMarkDeletedPosition(), pos);
}
Expand Down Expand Up @@ -1426,22 +1431,22 @@ void testClearBacklog(boolean useOpenRangeSet) throws Exception {

assertEquals(c1.getNumberOfEntriesInBacklog(), 3);
assertEquals(c1.getNumberOfEntries(), 3);
assertEquals(c1.hasMoreEntries(), true);
assertTrue(c1.hasMoreEntries());

c1.clearBacklog();
c3.clearBacklog();

assertEquals(c1.getNumberOfEntriesInBacklog(), 0);
assertEquals(c1.getNumberOfEntries(), 0);
assertEquals(c1.hasMoreEntries(), false);
assertFalse(c1.hasMoreEntries());

assertEquals(c2.getNumberOfEntriesInBacklog(), 2);
assertEquals(c2.getNumberOfEntries(), 2);
assertEquals(c2.hasMoreEntries(), true);
assertTrue(c2.hasMoreEntries());

assertEquals(c3.getNumberOfEntriesInBacklog(), 0);
assertEquals(c3.getNumberOfEntries(), 0);
assertEquals(c3.hasMoreEntries(), false);
assertFalse(c3.hasMoreEntries());

ManagedLedgerFactory factory2 = new ManagedLedgerFactoryImpl(bkc, bkc.getZkHandle());
ledger = factory2.open("my_test_ledger", new ManagedLedgerConfig().setMaxEntriesPerLedger(1));
Expand All @@ -1452,15 +1457,15 @@ void testClearBacklog(boolean useOpenRangeSet) throws Exception {

assertEquals(c1.getNumberOfEntriesInBacklog(), 0);
assertEquals(c1.getNumberOfEntries(), 0);
assertEquals(c1.hasMoreEntries(), false);
assertFalse(c1.hasMoreEntries());

assertEquals(c2.getNumberOfEntriesInBacklog(), 2);
assertEquals(c2.getNumberOfEntries(), 2);
assertEquals(c2.hasMoreEntries(), true);
assertTrue(c2.hasMoreEntries());

assertEquals(c3.getNumberOfEntriesInBacklog(), 0);
assertEquals(c3.getNumberOfEntries(), 0);
assertEquals(c3.hasMoreEntries(), false);
assertFalse(c3.hasMoreEntries());
factory2.shutdown();
}

Expand Down Expand Up @@ -1803,9 +1808,8 @@ void testFindNewestMatchingEdgeCase1() throws Exception {
ManagedLedger ledger = factory.open("my_test_ledger");

ManagedCursorImpl c1 = (ManagedCursorImpl) ledger.openCursor("c1");
assertEquals(
c1.findNewestMatching(entry -> Arrays.equals(entry.getDataAndRelease(), "expired".getBytes(Encoding))),
null);
assertNull(c1.findNewestMatching(
entry -> Arrays.equals(entry.getDataAndRelease(), "expired".getBytes(Encoding))));
}

@Test(timeOut = 20000)
Expand Down Expand Up @@ -2353,7 +2357,7 @@ void cancelReadOperation() throws Exception {
ManagedCursor c1 = ledger.openCursor("c1");

// No read request so far
assertEquals(c1.cancelPendingReadRequest(), false);
assertFalse(c1.cancelPendingReadRequest());

CountDownLatch counter = new CountDownLatch(1);

Expand All @@ -2369,7 +2373,7 @@ public void readEntriesFailed(ManagedLedgerException exception, Object ctx) {
}
}, null);

assertEquals(c1.cancelPendingReadRequest(), true);
assertTrue(c1.cancelPendingReadRequest());

CountDownLatch counter2 = new CountDownLatch(1);

Expand All @@ -2390,7 +2394,7 @@ public void readEntriesFailed(ManagedLedgerException exception, Object ctx) {
Thread.sleep(100);

// Read operation should have already been completed
assertEquals(c1.cancelPendingReadRequest(), false);
assertFalse(c1.cancelPendingReadRequest());

counter2.await();
}
Expand Down
9 changes: 6 additions & 3 deletions managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerErrorsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
*/
package org.apache.bookkeeper.mledger.impl;

import static org.testng.Assert.*;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.fail;

import java.util.List;
import java.util.concurrent.CountDownLatch;
Expand Down Expand Up @@ -49,7 +52,7 @@ public void removingCursor() throws Exception {
ManagedLedger ledger = factory.open("my_test_ledger");
ManagedCursor c1 = ledger.openCursor("c1");

assertEquals(zkc.exists("/managed-ledgers/my_test_ledger/c1", false) != null, true);
assertNotNull(zkc.exists("/managed-ledgers/my_test_ledger/c1", false));

zkc.failNow(Code.BADVERSION);

Expand All @@ -65,7 +68,7 @@ public void removingCursor() throws Exception {
// Cursor ledger deletion will fail, but that should not prevent the deleteCursor to fail
ledger.deleteCursor("c1");

assertEquals(zkc.exists("/managed-ledgers/my_test_ledger/c1", false) != null, false);
assertNull(zkc.exists("/managed-ledgers/my_test_ledger/c1", false));
assertEquals(bkc.getLedgers().size(), 2);
}

Expand Down
6 changes: 4 additions & 2 deletions ...edger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerSingleBookieTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
package org.apache.bookkeeper.mledger.impl;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;

import com.google.common.base.Charsets;
import java.nio.charset.Charset;
Expand Down Expand Up @@ -57,12 +59,12 @@ public void simple() throws Exception {

ManagedCursor cursor = ledger.openCursor("c1");

assertEquals(cursor.hasMoreEntries(), false);
assertFalse(cursor.hasMoreEntries());
assertEquals(cursor.readEntries(100), new ArrayList<Entry>());

ledger.addEntry("dummy-entry-2".getBytes(Encoding));

assertEquals(cursor.hasMoreEntries(), true);
assertTrue(cursor.hasMoreEntries());

List<Entry> entries = cursor.readEntries(100);
assertEquals(entries.size(), 1);
Expand Down
Loading

0 comments on commit fb6895d

Please sign in to comment.