CBL-4409: Remove error-prone 'expected' annotation (#182)

* CBL-4409: Remove error-prone 'expected' annotation
* CBL-4344: Close autocloseables

android/androidTest/java/com/couchbase/lite/internal/AndroidExecutionServiceTest.kt

@@ -38,7 +38,7 @@ class AndroidExecutionServiceTest : BaseTest() {
  latch.countDown()
  }
 
- latch.await(BaseTest.STD_TIMEOUT_SEC, TimeUnit.SECONDS)
+ latch.await(STD_TIMEOUT_SEC, TimeUnit.SECONDS)
 
  Assert.assertEquals(Looper.getMainLooper().thread, thread1.get())
  }

common/test/java/com/couchbase/lite/ArrayTest.java

@@ -77,10 +77,10 @@ public void testCreateWithList() {
  assertEquals(data, savedDoc.getArray("array").toList());
  }
 
- @Test(expected = IllegalArgumentException.class)
+ @Test
  public void testRecursiveArray() {
  MutableArray array = new MutableArray();
- array.addArray(array);
+ assertThrows(IllegalArgumentException.class, () -> array.addArray(array));
  }
 
  @Test
@@ -1013,36 +1013,44 @@ public void testEnumeratingArray() {
  });
  }
 
- @Test(expected = ConcurrentModificationException.class)
+ @Test
  public void testArrayEnumerationWithDataModification1() {
  final MutableArray array = new MutableArray();
  for (int i = 0; i <= 2; i++) { array.addValue(i); }
 
  assertEquals(3, array.count());
  assertArrayEquals(new Object[] {0, 1, 2}, array.toList().toArray());
 
- int n = 0;
- for (Iterator<Object> itr = array.iterator(); itr.hasNext(); itr.next()) {
- if (n++ == 1) { array.addValue(3); }
- }
+ assertThrows(
+ ConcurrentModificationException.class,
+ () -> {
+ int n = 0;
+ for (Iterator<Object> itr = array.iterator(); itr.hasNext(); itr.next()) {
+ if (n++ == 1) { array.addValue(3); }
+ }
+ });
  }
 
- @Test(expected = ConcurrentModificationException.class)
+ @Test
  public void testArrayEnumerationWithDataModification2() {
- MutableArray array = new MutableArray();
+ final MutableArray array = new MutableArray();
  for (int i = 0; i <= 2; i++) { array.addValue(i); }
 
  assertEquals(3, array.count());
  assertArrayEquals(new Object[] {0, 1, 2}, array.toList().toArray());
 
  MutableDocument doc = new MutableDocument("doc1").setValue("array", array);
- array = saveDocInTestCollection(doc).toMutable().getArray("array");
- assertNotNull(array);
-
- int n = 0;
- for (Iterator<Object> itr = array.iterator(); itr.hasNext(); itr.next()) {
- if (n++ == 1) { array.addValue(3); }
- }
+ final MutableArray savedArray = saveDocInTestCollection(doc).toMutable().getArray("array");
+ assertNotNull(savedArray);
+
+ assertThrows(
+ ConcurrentModificationException.class,
+ () -> {
+ int n = 0;
+ for (Iterator<Object> itr = savedArray.iterator(); itr.hasNext(); itr.next()) {
+ if (n++ == 1) { savedArray.addValue(3); }
+ }
+ });
  }
 
  @Test
@@ -1845,8 +1853,10 @@ public void testArrayToJSON() throws JSONException {
  }
 
  // JSON 3.7.?
- @Test(expected = IllegalStateException.class)
- public void testArrayToJSONBeforeSave() { new MutableArray().toJSON(); }
+ @Test
+ public void testArrayToJSONBeforeSave() {
+ assertThrows(IllegalStateException.class, () -> new MutableArray().toJSON());
+ }
 
  // JSON 3.7.a-b
  @Test
@@ -1859,17 +1869,23 @@ public void testArrayFromJSON() throws JSONException {
  }
 
  // JSON 3.7.c.1
- @Test(expected = IllegalArgumentException.class)
- public void testArrayFromBadJSON1() { new MutableArray("["); }
+ @Test
+ public void testArrayFromBadJSON1() {
+ assertThrows(IllegalArgumentException.class, () -> new MutableArray("["));
+ }
 
  // JSON 3.7.c.2
- @Test(expected = IllegalArgumentException.class)
- public void testArrayFromBadJSON2() { new MutableArray("[ab cd]"); }
+ @Test
+ public void testArrayFromBadJSON2() {
+ assertThrows(IllegalArgumentException.class, () -> new MutableArray("[ab cd]"));
+ }
 
  // JSON 3.7.d
- @Test(expected = IllegalArgumentException.class)
+ @Test
  public void testDictFromArray() {
- new MutableArray(BaseDbTestKt.readJSONResource("dictionary.json"));
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> new MutableArray(BaseDbTestKt.readJSONResource("dictionary.json")));
  }
 
 

common/test/java/com/couchbase/lite/AuthenticatorTest.java

@@ -26,14 +26,16 @@ public void testBasicAuthenticatorInstance() {
  assertEquals(password, new String(auth.getPasswordChars()));
  }
 
- @Test(expected = IllegalArgumentException.class)
+ @Test
  public void testBasicAuthenticatorWithEmptyUsername() {
- new BasicAuthenticator(null, "somePassword".toCharArray());
+ assertThrows(IllegalArgumentException.class, () -> new BasicAuthenticator(null, "somePassword".toCharArray()));
  }
 
 
- @Test(expected = IllegalArgumentException.class)
- public void testBasicAuthenticatorWithEmptyPassword() { new BasicAuthenticator("someUsername", null); }
+ @Test
+ public void testBasicAuthenticatorWithEmptyPassword() {
+ assertThrows(IllegalArgumentException.class, () -> new BasicAuthenticator("someUsername", null));
+ }
 
  @Test
  public void testSessionAuthenticatorWithSessionID() {
@@ -52,8 +54,10 @@ public void testSessionAuthenticatorWithSessionIDAndCookie() {
  assertEquals(cookie, auth.getCookieName());
  }
 
- @Test(expected = IllegalArgumentException.class)
- public void testSessionAuthenticatorEmptySessionID() { new SessionAuthenticator(null, null); }
+ @Test
+ public void testSessionAuthenticatorEmptySessionID() {
+ assertThrows(IllegalArgumentException.class, () -> new SessionAuthenticator(null, null));
+ }
 
  @Test
  public void testSessionAuthenticatorEmptyCookie() {

common/test/java/com/couchbase/lite/BaseReplicatorTest.kt

@@ -83,8 +83,8 @@ internal class ReplicatorAwaiter(repl: Replicator, exec: Executor) : ReplicatorC
 }
 
 // A filter can actually hang the replication
-internal class DelayFilter(val name: String, val barrier: CyclicBarrier) : ReplicationFilter {
- val shouldWait = AtomicBoolean(true)
+internal class DelayFilter(val name: String, private val barrier: CyclicBarrier) : ReplicationFilter {
+ private val shouldWait = AtomicBoolean(true)
 
  override fun filtered(doc: Document, flags: EnumSet<DocumentFlag>): Boolean {
  if (shouldWait.getAndSet(false)) {

common/test/java/com/couchbase/lite/BaseTest.java

@@ -89,7 +89,27 @@ public static void tearDownBaseTestSuite() {
  @NonNull
  public static String getUniqueName(@NonNull String prefix) { return StringUtils.getUniqueName(prefix, 8); }
 
- public static <T extends Exception> void assertThrows(Class<T> ex, Fn.TaskThrows<Exception> test) {
+ // Run a boolean function every `waitMs` until it is true
+ // If it is not true within `maxWaitMs` fail.
+ @SuppressWarnings({"BusyWait", "ConditionalBreakInInfiniteLoop"})
+ protected static void waitUntil(long maxWaitMs, Fn.Provider<Boolean> test) {
+ final long waitMs = 100L;
+ final long endTime = System.currentTimeMillis() + maxWaitMs - waitMs;
+ while (true) {
+ if (test.get()) { break; }
+ if (System.currentTimeMillis() > endTime) { throw new AssertionError("Operation timed out"); }
+ try { Thread.sleep(waitMs); }
+ catch (InterruptedException e) { throw new AssertionError("Operation interrupted", e); }
+ }
+ }
+
+ /////////////////////////////// E X C E P T I O N A S S E R T I O N S ///////////////////////////////
+
+ // Please do *NOT* use the @Test(expected=...) annotation. It is entirely too prone to error.
+ // Even though it can work pretty will in a very limited number of cases, please, always prefer
+ // one of these methods (or their equvalents in C4BaseTest and OKHttpSocketTest
+
+ public static <T extends Exception> void assertThrows(Class<T> ex, @NonNull Fn.TaskThrows<Exception> test) {
  try {
  test.run();
  fail("Expecting exception: " + ex);
@@ -109,7 +129,10 @@ public static void assertIsCBLException(@Nullable Exception e, @Nullable String
  if (code > 0) { assertEquals(code, err.getCode()); }
  }
 
- public static void assertThrowsCBLException(@Nullable String domain, int code, Fn.TaskThrows<Exception> block) {
+ public static void assertThrowsCBLException(
+ @Nullable String domain,
+ int code,
+ @NonNull Fn.TaskThrows<Exception> block) {
  try {
  block.run();
  fail("Expected CBL exception (" + domain + ", " + code + ")");
@@ -119,20 +142,6 @@ public static void assertThrowsCBLException(@Nullable String domain, int code, F
  }
  }
 
- // Run a boolean function every `waitMs` until it is true
- // If it is not true within `maxWaitMs` fail.
- @SuppressWarnings({"BusyWait", "ConditionalBreakInInfiniteLoop"})
- protected static void waitUntil(long maxWaitMs, Fn.Provider<Boolean> test) {
- final long waitMs = 100L;
- final long endTime = System.currentTimeMillis() + maxWaitMs - waitMs;
- while (true) {
- if (test.get()) { break; }
- if (System.currentTimeMillis() > endTime) { throw new AssertionError("Operation timed out"); }
- try { Thread.sleep(waitMs); }
- catch (InterruptedException e) { throw new AssertionError("Operation interrupted", e); }
- }
- }
-
 
  protected ExecutionService.CloseableExecutor testSerialExecutor;
  private String testName;

common/test/java/com/couchbase/lite/BlobTest.java

@@ -55,17 +55,25 @@ public class BlobTest extends BaseDbTest {
  @Before
  public final void setUpBlobTest() { localBlobContent = StringUtils.randomString(100); }
 
- @Test(expected = IllegalArgumentException.class)
- public void testBlobCtorWithNullContentType() { new Blob(null, new byte[] {5, 6, 7, 8}); }
+ @Test
+ public void testBlobCtorWithNullContentType() {
+ assertThrows(IllegalArgumentException.class, () -> new Blob(null, new byte[] {5, 6, 7, 8}));
+ }
 
- @Test(expected = IllegalArgumentException.class)
- public void testBlobCtorWithNullContent() { new Blob("image/png", (byte[]) null); }
+ @Test
+ public void testBlobCtorWithNullContent() {
+ assertThrows(IllegalArgumentException.class, () -> new Blob("image/png", (byte[]) null));
+ }
 
- @Test(expected = IllegalArgumentException.class)
- public void testBlobCtorWithStreamAndNullContentType() { new Blob(null, PlatformUtils.getAsset("attachment.png")); }
+ @Test
+ public void testBlobCtorWithStreamAndNullContentType() {
+ assertThrows(IllegalArgumentException.class, () -> new Blob(null, PlatformUtils.getAsset("attachment.png")));
+ }
 
- @Test(expected = IllegalArgumentException.class)
- public void testBlobCtorsWithNullStream() { new Blob("image/png", (InputStream) null); }
+ @Test
+ public void testBlobCtorsWithNullStream() {
+ assertThrows(IllegalArgumentException.class, () -> new Blob("image/png", (InputStream) null));
+ }
 
  @Test
  public void testEquals() {
@@ -348,8 +356,10 @@ public void testDbGetBlob() {
  }
 
  // 3.1.c
- @Test(expected = IllegalStateException.class)
- public void testUnsavedBlobToJSON() { makeBlob().toJSON(); }
+ @Test
+ public void testUnsavedBlobToJSON() {
+ assertThrows(IllegalStateException.class, () -> makeBlob().toJSON());
+ }
 
  // 3.1.d
  @Test
@@ -361,59 +371,59 @@ public void testDbGetNonexistentBlob() {
  }
 
  // 3.1.e.0: null param
- @Test(expected = IllegalArgumentException.class)
+ @Test
  public void testDbGetNotBlob0() {
  Blob blob = makeBlob();
  getTestDatabase().saveBlob(blob);
- assertNull(getTestDatabase().getBlob(null));
+ assertThrows(IllegalArgumentException.class, () -> getTestDatabase().getBlob(null));
  }
 
  // 3.1.e.1: empty param
- @Test(expected = IllegalArgumentException.class)
+ @Test
  public void testDbGetNotBlob1() {
  Blob blob = makeBlob();
  getTestDatabase().saveBlob(blob);
- assertNull(getTestDatabase().getBlob(new HashMap<>()));
+ assertThrows(IllegalArgumentException.class, () -> getTestDatabase().getBlob(new HashMap<>()));
  }
 
  // 3.1.e.2: missing digest
- @Test(expected = IllegalArgumentException.class)
+ @Test
  public void testDbGetNotBlob2() {
  Map<String, Object> props = getPropsForSavedBlob();
  props.remove(Blob.PROP_DIGEST);
- assertNull(getTestDatabase().getBlob(props));
+ assertThrows(IllegalArgumentException.class, () -> getTestDatabase().getBlob(props));
  }
 
  // 3.1.e.3: missing meta-type
- @Test(expected = IllegalArgumentException.class)
+ @Test
  public void testDbGetNotBlob3() {
  Map<String, Object> props = getPropsForSavedBlob();
  props.remove(Blob.META_PROP_TYPE);
- assertNull(getTestDatabase().getBlob(props));
+ assertThrows(IllegalArgumentException.class, () -> getTestDatabase().getBlob(props));
  }
 
  // 3.1.e.4: length is not a number
- @Test(expected = IllegalArgumentException.class)
+ @Test
  public void testDbGetNotBlob4() {
  Map<String, Object> props = getPropsForSavedBlob();
  props.put(Blob.PROP_LENGTH, "42");
- assertNull(getTestDatabase().getBlob(props));
+ assertThrows(IllegalArgumentException.class, () -> getTestDatabase().getBlob(props));
  }
 
  // 3.1.e.5: bad content type
- @Test(expected = IllegalArgumentException.class)
+ @Test
  public void testDbGetNotBlob5() {
  Map<String, Object> props = getPropsForSavedBlob();
  props.put(Blob.PROP_CONTENT_TYPE, new Object());
- assertNull(getTestDatabase().getBlob(props));
+ assertThrows(IllegalArgumentException.class, () -> getTestDatabase().getBlob(props));
  }
 
  // 3.1.e.6: extra arg
- @Test(expected = IllegalArgumentException.class)
+ @Test
  public void testDbGetNotBlob6() {
  Map<String, Object> props = getPropsForSavedBlob();
  props.put("foo", "bar");
- assertNull(getTestDatabase().getBlob(props));
+ assertThrows(IllegalArgumentException.class, () -> getTestDatabase().getBlob(props));
  }
 
  // 3.1.f

common/test/java/com/couchbase/lite/CollectionQueryTest.kt

@@ -158,13 +158,15 @@ class CollectionQueryTest : BaseQueryTest() {
  // Use the following SQL++ query to create and execute the query:
  // SELECT name.first FROM person.names BY name.first LIMIT 1
  // Ensure that an error is returned or thrown when executing the query.
- @Test(expected = CouchbaseLiteException::class)
+ @Test
  fun testSQLPPQueryNonExistingCollection() {
  val collection = testDatabase.createCollection("names", "people")
  loadJSONResourceIntoCollection("names_100.json", collection = collection)
  val queryString = "SELECT name.first FROM person.names ORDER BY name.first LIMIT 1"
  val query = QueryBuilder.createQuery(queryString, testDatabase)
- query.execute()
+ assertThrowsCBLException(CBLError.Domain.CBLITE, CBLError.Code.INVALID_QUERY) {
+ query.execute().use { }
+ }
  }
 
  // 8.11.5a: Test that query by joining collections works as expected.
@@ -586,7 +588,7 @@ class CollectionQueryTest : BaseQueryTest() {
 
  // 8.11.9: Test that a multi-collection join query with match() function that
  // only uses index name without collection-alias-name prefix is failed to create.
- @Test(expected = CouchbaseLiteException::class)
+ @Test
  fun testFTSJoinQueryError() {
  testDatabase.createCollection("colors", "test")
 
@@ -595,7 +597,7 @@ class CollectionQueryTest : BaseQueryTest() {
  flowerCol.createIndex("DescIndex", IndexBuilder.fullTextIndex(FullTextIndexItem.property("description")))
 
  // create with query string
- QueryBuilder.createQuery(
+ val query = QueryBuilder.createQuery(
  """SELECT f.name, f.description, c.color
  FROM test.flowers AS f
  JOIN test.colors AS c
@@ -604,7 +606,10 @@ class CollectionQueryTest : BaseQueryTest() {
  ORDER BY f.name"""
  .trimIndent(),
  testDatabase
- ).execute()
+ )
+ assertThrowsCBLException(CBLError.Domain.CBLITE, CBLError.Code.INVALID_QUERY) {
+ query.execute().use { }
+ }
  }
 
  // 8.11.10a: Test that the result’s key names of the SELECT * are as follows.