Skip to content

Commit

Permalink
Fix remaining CloseGuard warnings in Robolectric tests
Browse files Browse the repository at this point in the history 
CloseGuard warnings and stack traces accounted for ~40% of log
lines during Robolectric tests.

This will remove around ~3.5k CloseGuard errors and stack traces in the
Robolectric test suite.

PiperOrigin-RevId: 413781118
  • Loading branch information
@hoisie
hoisie committed Dec 6, 2021
1 parent 5b8c1bf commit 516ba49
Show file tree
Hide file tree
Showing 22 changed files with 243 additions and 105 deletions.
15 changes: 8 additions & 7 deletions ...ctric/src/test/java/org/robolectric/android/controller/ContentProviderControllerTest.java
View file
Expand Up @@ -61,7 +61,7 @@ public void shouldRegisterWithContentResolver() throws Exception {
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);
close(client);
}

@Test
Expand All @@ -73,7 +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);
close(client);
}

@Test
Expand Down Expand Up @@ -106,7 +106,7 @@ public void withoutManifest_shouldRegisterWithContentResolver() throws Exception
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);
close(client);
}

@Test
Expand All @@ -117,7 +117,7 @@ public void contentProviderShouldBeCreatedBeforeBeingRegistered() throws Excepti
ContentProviderClient contentProviderClient =
contentResolver.acquireContentProviderClient("x-authority");
assertThat(contentProviderClient.getLocalContentProvider()).isSameInstanceAs(xContentProvider);
maybeClose(contentProviderClient);
close(contentProviderClient);
}

@Test
Expand All @@ -142,18 +142,19 @@ public boolean onCreate() {
? "x-authority" + " not registered" + " yet"
: "x-authority" + " is registered");
if (contentProviderClient != null) {
maybeClose(contentProviderClient);
close(contentProviderClient);
}
return false;
}
}

static class NotInManifestContentProvider extends TestContentProvider1 {}

/** {@link ContentProviderClient#close is only implemented in SDK > M} */
private static void maybeClose(ContentProviderClient client) {
private static void close(ContentProviderClient client) {
if (RuntimeEnvironment.getApiLevel() > Build.VERSION_CODES.M) {
client.close();
} else {
client.release();
}
}
}
1 change: 1 addition & 0 deletions robolectric/src/test/java/org/robolectric/shadows/SQLiteCursorTest.java
View file
Expand Up @@ -483,6 +483,7 @@ private Cursor createCursor() {

private void setupEmptyResult() {
database.execSQL("DELETE FROM table_name;");
cursor.close();
cursor = createCursor();
}

Expand Down
47 changes: 42 additions & 5 deletions robolectric/src/test/java/org/robolectric/shadows/SQLiteDatabaseTest.java
View file
Expand Up @@ -122,6 +122,7 @@ public void testInsertAndQuery() {

assertThat(stringValueFromDatabase).isEqualTo(stringColumnValue);
assertThat(byteValueFromDatabase).isEqualTo(byteColumnValue);
cursor.close();
}

@Test
Expand All @@ -146,6 +147,7 @@ public void testInsertAndRawQuery() {

assertThat(stringValueFromDatabase).isEqualTo(stringColumnValue);
assertThat(byteValueFromDatabase).isEqualTo(byteColumnValue);
cursor.close();
}

@Test(expected = android.database.SQLException.class)
Expand Down Expand Up @@ -173,6 +175,7 @@ public void testInsertOrThrow() {
String stringValueFromDatabase = cursor.getString(1);
assertThat(stringValueFromDatabase).isEqualTo(stringColumnValue);
assertThat(byteValueFromDatabase).isEqualTo(byteColumnValue);
cursor.close();
}

@Test(expected = IllegalArgumentException.class)
Expand All @@ -194,12 +197,14 @@ public void testRawQueryCountWithOneArgument() {
"select second_column, first_column from rawtable WHERE" + " `id` = ?",
new String[] {"1"});
assertThat(cursor.getCount()).isEqualTo(1);
cursor.close();
}

@Test
public void testRawQueryCountWithNullArgs() {
Cursor cursor = database.rawQuery("select second_column, first_column from rawtable", null);
assertThat(cursor.getCount()).isEqualTo(2);
cursor.close();
}

@Test
Expand All @@ -208,6 +213,7 @@ public void testRawQueryCountWithEmptyArguments() {
database.rawQuery(
"select second_column, first_column" + " from" + " rawtable", new String[] {});
assertThat(cursor.getCount()).isEqualTo(2);
cursor.close();
}

@Test(expected = IllegalArgumentException.class)
Expand Down Expand Up @@ -235,6 +241,7 @@ public void testEmptyTable() {
null);

assertThat(cursor.moveToFirst()).isFalse();
cursor.close();
}

@Test
Expand Down Expand Up @@ -325,6 +332,7 @@ public void testUpdate() {
assertThat(cursor.getCount()).isEqualTo(1);

assertIdAndName(cursor, 1234L, "Buster");
cursor.close();
}

@Test
Expand All @@ -339,6 +347,7 @@ public void testUpdateNoMatch() {
assertThat(cursor.getCount()).isEqualTo(1);

assertIdAndName(cursor, 1234L, "Chuck");
cursor.close();
}

@Test
Expand All @@ -361,6 +370,7 @@ public void testUpdateAll() {
assertThat(cursor.moveToNext()).isFalse();
assertThat(cursor.isAfterLast()).isTrue();
assertThat(cursor.moveToNext()).isFalse();
cursor.close();
}

@Test
Expand Down Expand Up @@ -402,13 +412,15 @@ public void testExecSQL() {
assertThat(cursor).isNotNull();
assertThat(cursor.moveToNext()).isTrue();
assertThat(cursor.getInt(0)).isEqualTo(1);
cursor.close();

cursor = database.rawQuery("SELECT * FROM table_name", null);
assertThat(cursor).isNotNull();
assertThat(cursor.moveToNext()).isTrue();

assertThat(cursor.getInt(cursor.getColumnIndex("id"))).isEqualTo(1234);
assertThat(cursor.getString(cursor.getColumnIndex("name"))).isEqualTo("Chuck");
cursor.close();
}

@Test
Expand All @@ -429,6 +441,7 @@ public void testExecSQLParams() {
assertThat(cursor).isNotNull();
assertThat(cursor.moveToNext()).isTrue();
assertThat(cursor.getInt(0)).isEqualTo(2);
cursor.close();

cursor = database.rawQuery("SELECT `id`, `name` ,`lastUsed` FROM `routine`", null);
assertThat(cursor).isNotNull();
Expand All @@ -443,6 +456,7 @@ public void testExecSQLParams() {
assertThat(cursor.getInt(cursor.getColumnIndex("id"))).isEqualTo(2);
assertThat(cursor.getInt(cursor.getColumnIndex("lastUsed"))).isEqualTo(1);
assertThat(cursor.getString(cursor.getColumnIndex("name"))).isEqualTo("Bench Press");
cursor.close();
}

@Test(expected = SQLiteException.class)
Expand Down Expand Up @@ -487,6 +501,7 @@ public void testExecSQLInsertNull() {
int nameIndex = cursor.getColumnIndex("name");
assertThat(cursor.getString(nameIndex)).isEqualTo(name);
assertThat(cursor.getString(firstIndex)).isEqualTo(null);
cursor.close();
}

@Test
Expand Down Expand Up @@ -525,6 +540,7 @@ public void shouldStoreGreatBigHonkingIntegersCorrectly() {
database.query("table_name", new String[] {"big_int"}, null, null, null, null, null);
assertThat(cursor.moveToFirst()).isTrue();
assertEquals(1234567890123456789L, cursor.getLong(0));
cursor.close();
}

@Test
Expand All @@ -537,6 +553,7 @@ public void testSuccessTransaction() {
Cursor cursor = database.rawQuery("SELECT COUNT(*) FROM table_name", null);
assertThat(cursor.moveToNext()).isTrue();
assertThat(cursor.getInt(0)).isEqualTo(1);
cursor.close();
}

@Test
Expand All @@ -557,6 +574,7 @@ public void testFailureTransaction() {
cursor = database.rawQuery(select, null);
assertThat(cursor.moveToNext()).isTrue();
assertThat(cursor.getInt(0)).isEqualTo(0);
cursor.close();
}

@Test
Expand All @@ -573,6 +591,7 @@ public void testSuccessNestedTransaction() {
Cursor cursor = database.rawQuery("SELECT COUNT(*) FROM table_name", null);
assertThat(cursor.moveToNext()).isTrue();
assertThat(cursor.getInt(0)).isEqualTo(2);
cursor.close();
}

@Test
Expand All @@ -588,6 +607,7 @@ public void testFailureNestedTransaction() {
Cursor cursor = database.rawQuery("SELECT COUNT(*) FROM table_name", null);
assertThat(cursor.moveToNext()).isTrue();
assertThat(cursor.getInt(0)).isEqualTo(0);
cursor.close();
}

@Test
Expand All @@ -600,6 +620,8 @@ public void testTransactionAlreadySuccessful() {
} catch (IllegalStateException e) {
assertThat(e.getMessage()).contains("transaction");
assertThat(e.getMessage()).contains("successful");
} finally {
database.endTransaction();
}
}

Expand Down Expand Up @@ -629,6 +651,7 @@ public void testReplace() {

assertThat(cursor.moveToNext()).isTrue();
assertThat(cursor.getString(cursor.getColumnIndex("name"))).isEqualTo("Norris");
cursor.close();
}

@Test
Expand Down Expand Up @@ -657,28 +680,34 @@ public void testReplaceIsReplacing() {
assertThat(secondId).isEqualTo(id);
assertThat(firstCursor.getString(0)).isEqualTo(stringValueA);
assertThat(secondCursor.getString(0)).isEqualTo(stringValueB);
firstCursor.close();
secondCursor.close();
}

@Test
public void shouldCreateDefaultCursorFactoryWhenNullFactoryPassedToRawQuery() {
database.rawQueryWithFactory(null, ANY_VALID_SQL, null, null);
Cursor cursor = database.rawQueryWithFactory(null, ANY_VALID_SQL, null, null);
cursor.close();
}

@Test
public void shouldCreateDefaultCursorFactoryWhenNullFactoryPassedToQuery() {
database.queryWithFactory(null, false, "table_name", null, null, null, null, null, null, null);
Cursor cursor =
database.queryWithFactory(
null, false, "table_name", null, null, null, null, null, null, null);
cursor.close();
}

@Test
public void shouldOpenExistingDatabaseFromFileSystemIfFileExists() {

database.close();

SQLiteDatabase db =
SQLiteDatabase.openDatabase(databasePath.getAbsolutePath(), null, OPEN_READWRITE);
Cursor c = db.rawQuery("select * from rawtable", null);
assertThat(c).isNotNull();
assertThat(c.getCount()).isEqualTo(2);
c.close();
assertThat(db.isOpen()).isTrue();
db.close();
assertThat(db.isOpen()).isFalse();
Expand All @@ -687,6 +716,7 @@ public void shouldOpenExistingDatabaseFromFileSystemIfFileExists() {
SQLiteDatabase.openDatabase(databasePath.getAbsolutePath(), null, OPEN_READWRITE);
assertThat(reopened).isNotSameInstanceAs(db);
assertThat(reopened.isOpen()).isTrue();
reopened.close();
}

@Test(expected = SQLiteException.class)
Expand All @@ -701,6 +731,7 @@ public void shouldUseInMemoryDatabaseWhenCallingCreate() {
SQLiteDatabase db = SQLiteDatabase.create(null);
assertThat(db.isOpen()).isTrue();
assertThat(db.getPath()).isEqualTo(":memory:");
db.close();
}

@Test
Expand Down Expand Up @@ -732,12 +763,14 @@ public void testTwoConcurrentDbConnections() {
assertThat(c.getCount()).isEqualTo(1);
assertThat(c.moveToNext()).isTrue();
assertThat(c.getString(c.getColumnIndex("data"))).isEqualTo("d1");
c.close();

c = db2.rawQuery("select * from bar", null);
assertThat(c).isNotNull();
assertThat(c.getCount()).isEqualTo(1);
assertThat(c.moveToNext()).isTrue();
assertThat(c.getString(c.getColumnIndex("data"))).isEqualTo("d2");
c.close();
}

@Test(expected = SQLiteException.class)
Expand Down Expand Up @@ -802,6 +835,7 @@ public void testDataInMemoryDatabaseIsPersistentAfterClose() {
assertThat(c.getCount()).isEqualTo(1);
assertThat(c.moveToNext()).isTrue();
assertThat(c.getString(c.getColumnIndex("data"))).isEqualTo("d1");
c.close();
}

@Test
Expand All @@ -822,6 +856,7 @@ public void testRawQueryWithFactoryAndCancellationSignal() {
} catch (OperationCanceledException e) {
// expected
}
cursor.close();
}

@Test
Expand All @@ -848,8 +883,7 @@ public void shouldBeAbleToBeUsedFromDifferentThread() {
new Thread() {
@Override
public void run() {
try {
executeQuery("select * from table_name");
try (Cursor c = executeQuery("select * from table_name")) {
} catch (Throwable e) {
e.printStackTrace();
error[0] = e;
Expand Down Expand Up @@ -915,13 +949,15 @@ private void assertEmptyDatabase() {
assertThat(cursor.moveToFirst()).isFalse();
assertThat(cursor.isClosed()).isFalse();
assertThat(cursor.getCount()).isEqualTo(0);
cursor.close();
}

private void assertNonEmptyDatabase() {
Cursor cursor =
database.query("table_name", new String[] {"id", "name"}, null, null, null, null, null);
assertThat(cursor.moveToFirst()).isTrue();
assertThat(cursor.getCount()).isNotEqualTo(0);
cursor.close();
}

@Test
Expand Down Expand Up @@ -986,6 +1022,7 @@ public void shouldCorrectlyReturnNullValues() {
assertThat(nullValuesCursor.getString(i)).isNull();
}
assertThat(nullValuesCursor.getBlob(3)).isNull();
nullValuesCursor.close();
}

@Test
Expand Down

0 comments on commit 516ba49

Please sign in to comment.