Close android.database.Cursor objects in tests

Around 30-40% of log lines in GitHub CI contain CloseGuard stack traces from open cursors being closed. Also close some ContentProviderClient objects. PiperOrigin-RevId: 413737346
robolectric · Dec 6, 2021 · 4a5419a · 4a5419a
1 parent bcb31db
commit 4a5419a
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 35 deletions.
diff --git a/...ctric/src/test/java/org/robolectric/android/controller/ContentProviderControllerTest.java b/...ctric/src/test/java/org/robolectric/android/controller/ContentProviderControllerTest.java
@@ -1,19 +1,22 @@
 package org.robolectric.android.controller;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
 
 import android.app.Application;
 import android.content.ContentProviderClient;
 import android.content.ContentResolver;
 import android.content.pm.PathPermission;
 import android.content.pm.ProviderInfo;
 import android.net.Uri;
+import android.os.Build;
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.Robolectric;
+import org.robolectric.RuntimeEnvironment;
 import org.robolectric.shadows.testing.TestContentProvider1;
 import org.robolectric.shadows.testing.TestContentProvider3And4;
 
@@ -42,7 +45,7 @@ public void shouldInitializeFromManifestProviderInfo() throws Exception {
     assertThat(myContentProvider.getReadPermission()).isEqualTo("READ_PERMISSION");
     assertThat(myContentProvider.getWritePermission()).isEqualTo("WRITE_PERMISSION");
 
-    assertThat(myContentProvider.getPathPermissions()).asList().hasSize(1);
+    assertThat(myContentProvider.getPathPermissions()).hasLength(1);
     PathPermission pathPermission = myContentProvider.getPathPermissions()[0];
     assertThat(pathPermission.getPath()).isEqualTo("/path/*");
     assertThat(pathPermission.getType()).isEqualTo(PathPermission.PATTERN_SIMPLE_GLOB);
@@ -55,10 +58,10 @@ public void shouldRegisterWithContentResolver() throws Exception {
     controller.create().get();
 
     ContentProviderClient client =
-        contentResolver.acquireContentProviderClient(
-            "org.robolectric.authority1");
-    client.query(Uri.parse("something"), new String[]{"title"}, "*", new String[]{}, "created");
+        contentResolver.acquireContentProviderClient("org.robolectric.authority1");
+    client.query(Uri.parse("something"), new String[] {"title"}, "*", new String[] {}, "created");
     assertThat(controller.get().transcript).containsExactly("onCreate", "query for something");
+    maybeClose(client);
   }
 
   @Test
@@ -70,6 +73,7 @@ public void shouldResolveProvidersWithMultipleAuthorities() throws Exception {
         contentResolver.acquireContentProviderClient("org.robolectric.authority3");
     client.query(Uri.parse("something"), new String[] {"title"}, "*", new String[] {}, "created");
     assertThat(contentProvider.transcript).containsExactly("onCreate", "query for something");
+    maybeClose(client);
   }
 
   @Test
@@ -98,9 +102,11 @@ public void withoutManifest_shouldRegisterWithContentResolver() throws Exception
     providerInfo.authority = "some-authority";
     controller.create(providerInfo);
 
-    ContentProviderClient client = contentResolver.acquireContentProviderClient(providerInfo.authority);
-    client.query(Uri.parse("something"), new String[]{"title"}, "*", new String[]{}, "created");
+    ContentProviderClient client =
+        contentResolver.acquireContentProviderClient(providerInfo.authority);
+    client.query(Uri.parse("something"), new String[] {"title"}, "*", new String[] {}, "created");
     assertThat(controller.get().transcript).containsExactly("onCreate", "query for something");
+    maybeClose(client);
   }
 
   @Test
@@ -111,11 +117,17 @@ public void contentProviderShouldBeCreatedBeforeBeingRegistered() throws Excepti
     ContentProviderClient contentProviderClient =
         contentResolver.acquireContentProviderClient("x-authority");
     assertThat(contentProviderClient.getLocalContentProvider()).isSameInstanceAs(xContentProvider);
+    maybeClose(contentProviderClient);
   }
 
-  @Test(expected = IllegalArgumentException.class)
+  @Test
   public void createContentProvider_nullAuthority() throws Exception {
-    Robolectric.buildContentProvider(XContentProvider.class).create(new ProviderInfo()).get();
+    assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            Robolectric.buildContentProvider(XContentProvider.class)
+                .create(new ProviderInfo())
+                .get());
   }
 
   static class XContentProvider extends TestContentProvider1 {
@@ -129,9 +141,19 @@ public boolean onCreate() {
           contentProviderClient == null
               ? "x-authority" + " not registered" + " yet"
               : "x-authority" + " is registered");
+      if (contentProviderClient != null) {
+        maybeClose(contentProviderClient);
+      }
       return false;
     }
   }
 
   static class NotInManifestContentProvider extends TestContentProvider1 {}
+
+  /** {@link ContentProviderClient#close is only implemented in SDK > M} */
+  private static void maybeClose(ContentProviderClient client) {
+    if (RuntimeEnvironment.getApiLevel() > Build.VERSION_CODES.M) {
+      client.close();
+    }
+  }
 }
diff --git a/robolectric/src/test/java/org/robolectric/shadows/SQLiteCursorTest.java b/robolectric/src/test/java/org/robolectric/shadows/SQLiteCursorTest.java
@@ -39,6 +39,7 @@ public void setUp() throws Exception {
   @After
   public void tearDown() {
     database.close();
+    cursor.close();
   }
 
   @Test

diff --git a/robolectric/src/test/java/org/robolectric/shadows/SQLiteQueryBuilderTest.java b/robolectric/src/test/java/org/robolectric/shadows/SQLiteQueryBuilderTest.java
@@ -66,20 +66,32 @@ public void tearDown() {
   public void shouldBeAbleToMakeQueries() {
     Cursor cursor = builder.query(database, new String[] {"rowid"}, null, null, null, null, null);
     assertThat(cursor.getCount()).isEqualTo(2);
+    cursor.close();
   }
 
   @Test
   public void shouldBeAbleToMakeQueriesWithSelection() {
-    Cursor cursor = builder.query(database, new String[] {"rowid"}, COL_VALUE + "=?", new String[] {"record1"}, null, null, null);
+    Cursor cursor =
+        builder.query(
+            database,
+            new String[] {"rowid"},
+            COL_VALUE + "=?",
+            new String[] {"record1"},
+            null,
+            null,
+            null);
     assertThat(cursor.getCount()).isEqualTo(1);
     assertThat(cursor.moveToNext()).isTrue();
     assertThat(cursor.getLong(0)).isEqualTo(firstRecordId);
+    cursor.close();
   }
 
   @Test
   public void shouldBeAbleToMakeQueriesWithGrouping() {
-    Cursor cursor = builder.query(database, new String[] {"rowid"}, null, null, COL_GROUP, null, null);
+    Cursor cursor =
+        builder.query(database, new String[] {"rowid"}, null, null, COL_GROUP, null, null);
     assertThat(cursor.getCount()).isEqualTo(1);
+    cursor.close();
   }
 
 }
diff --git a/robolectric/src/test/java/org/robolectric/shadows/ShadowCursorAdapterTest.java b/robolectric/src/test/java/org/robolectric/shadows/ShadowCursorAdapterTest.java
@@ -10,6 +10,7 @@
 import android.widget.CursorAdapter;
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -43,6 +44,12 @@ public void setUp() throws Exception {
     adapter = new TestAdapter(curs);
   }
 
+  @After
+  public void tearDown() {
+    database.close();
+    curs.close();
+  }
+
   @Test
   public void testChangeCursor() {
     assertThat(adapter.getCursor()).isNotNull();

diff --git a/robolectric/src/test/java/org/robolectric/shadows/ShadowMergeCursorTest.java b/robolectric/src/test/java/org/robolectric/shadows/ShadowMergeCursorTest.java
@@ -7,6 +7,7 @@
 import android.database.sqlite.SQLiteCursor;
 import android.database.sqlite.SQLiteDatabase;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -52,6 +53,13 @@ public void setUp() throws Exception {
             "SELECT * FROM table_2;");
   }
 
+  @After
+  public void tearDown() throws Exception {
+    database.close();
+    dbCursor1.close();
+    dbCursor2.close();
+  }
+
   private SQLiteCursor setupTable(final String createSql, final String[] insertions, final String selectSql) {
     database.execSQL(createSql);
 

diff --git a/robolectric/src/test/java/org/robolectric/shadows/ShadowSimpleCursorAdapterTest.java b/robolectric/src/test/java/org/robolectric/shadows/ShadowSimpleCursorAdapterTest.java
@@ -8,6 +8,7 @@
 import android.widget.SimpleCursorAdapter;
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -16,19 +17,42 @@
 public class ShadowSimpleCursorAdapterTest {
 
   private Application context;
+  private SQLiteDatabase database;
+  private Cursor cursor;
 
   @Before
   public void setUp() throws Exception {
     context = ApplicationProvider.getApplicationContext();
+
+    database = SQLiteDatabase.create(null);
+    database.execSQL("CREATE TABLE table_name(_id INT PRIMARY KEY, name VARCHAR(255));");
+    String[] inserts = {
+      "INSERT INTO table_name (_id, name) VALUES(1234, 'Chuck');",
+      "INSERT INTO table_name (_id, name) VALUES(1235, 'Julie');",
+      "INSERT INTO table_name (_id, name) VALUES(1236, 'Chris');",
+      "INSERT INTO table_name (_id, name) VALUES(1237, 'Brenda');",
+      "INSERT INTO table_name (_id, name) VALUES(1238, 'Jane');"
+    };
+
+    for (String insert : inserts) {
+      database.execSQL(insert);
+    }
+
+    String sql = "SELECT * FROM table_name;";
+    cursor = database.rawQuery(sql, null);
+  }
+
+  @After
+  public void tearDown() {
+    database.close();
+    cursor.close();
   }
 
   @Test
   public void testChangeCursor() {
     SimpleCursorAdapter adapter =
         new SimpleCursorAdapter(context, 1, null, new String[] {"name"}, new int[] {2}, 0);
 
-    Cursor cursor = setUpDatabase();
-
     adapter.changeCursor(cursor);
 
     assertThat(adapter.getCursor()).isSameInstanceAs(cursor);
@@ -39,8 +63,6 @@ public void testSwapCursor() {
     SimpleCursorAdapter adapter =
         new SimpleCursorAdapter(context, 1, null, new String[] {"name"}, new int[] {2}, 0);
 
-    Cursor cursor = setUpDatabase();
-
     adapter.swapCursor(cursor);
 
     assertThat(adapter.getCursor()).isSameInstanceAs(cursor);
@@ -51,30 +73,9 @@ public void testSwapCursorToNull() {
     SimpleCursorAdapter adapter =
         new SimpleCursorAdapter(context, 1, null, new String[] {"name"}, new int[] {2}, 0);
 
-    Cursor cursor = setUpDatabase();
-
     adapter.swapCursor(cursor);
     adapter.swapCursor(null);
 
     assertThat(adapter.getCursor()).isNull();
   }
-
-  private Cursor setUpDatabase() {
-    SQLiteDatabase database = SQLiteDatabase.create(null);
-    database.execSQL("CREATE TABLE table_name(_id INT PRIMARY KEY, name VARCHAR(255));");
-    String[] inserts = {
-        "INSERT INTO table_name (_id, name) VALUES(1234, 'Chuck');",
-        "INSERT INTO table_name (_id, name) VALUES(1235, 'Julie');",
-        "INSERT INTO table_name (_id, name) VALUES(1236, 'Chris');",
-        "INSERT INTO table_name (_id, name) VALUES(1237, 'Brenda');",
-        "INSERT INTO table_name (_id, name) VALUES(1238, 'Jane');"
-    };
-
-    for (String insert : inserts) {
-      database.execSQL(insert);
-    }
-
-    String sql = "SELECT * FROM table_name;";
-    return database.rawQuery(sql, null);
-  }
 }