In `ContentProviderTest.tearDown` we try to delete the databases created by the content provider in order to clean up after ourselves. However, we always fail because of an NPE, but TBPL doesn't seem to notice since this happens after all the assertions pass. 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): ----- begin exception ----- 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): java.lang.reflect.InvocationTargetException 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at java.lang.reflect.Method.invokeNative(Native Method) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at java.lang.reflect.Method.invoke(Method.java:511) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at org.mozilla.gecko.tests.ContentProviderTest.tearDown(ContentProviderTest.java:257) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at org.mozilla.gecko.tests.testDistribution.tearDown(testDistribution.java:333) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at junit.framework.TestCase.runBare(TestCase.java:130) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at junit.framework.TestResult$1.protect(TestResult.java:106) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at junit.framework.TestResult.runProtected(TestResult.java:124) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at junit.framework.TestResult.run(TestResult.java:109) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at junit.framework.TestCase.run(TestCase.java:118) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:545) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1551) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): Caused by: java.lang.NullPointerException 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): at org.mozilla.gecko.db.BrowserProvider.getDatabasePath(BrowserProvider.java:1959) 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): ... 13 more 11:52:39 INFO - 12-11 11:52:35.820 I/TestRunner( 4145): ----- end exception ----- Looking at TBPL logs from before/after bug 941357 landed, that seems to have introduced this problem :/ I suppose that's because we used to just automatically return the database name, rather than try getting the database helper out of the map first: https://hg.mozilla.org/mozilla-central/rev/904545658c9b#l1.129 All that being said, I don't think we actually need to delete these databases, since each robocop test runs with a new profile. Maybe that's a bad thing to rely on, but it would be a convenient excuse to remove this bit of reflection, as well as the getDatabasePath method in BrowserProvider.

This fixes the problem for me locally, and I think it's a nice way to get rid of some test-only methods in our code. Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=8094ee3c293e

Comment on attachment 8346221 [details] [diff] [review] Don't use reflection to get database name in robocop tests Review of attachment 8346221 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/tests/ContentProviderTest.java @@ +249,5 @@ > if (Build.VERSION.SDK_INT >= 11) { > mProvider.shutdown(); > } > > + if (mDatabaseName != null) { Note: I kept this null check because theoretically you could want to test a content provider that's not backed by a database.

I actually have no idea why we kept this getDatabasePath method in TabsProvider. TabsProvider doesn't even have any robocop tests! New try push: https://tbpl.mozilla.org/?tree=Try&rev=48379686595f

Comment on attachment 8346225 [details] [diff] [review] Don't use reflection to get database name in robocop tests Review of attachment 8346225 [details] [diff] [review]: ----------------------------------------------------------------- Also tagging wesj here in case he might be able to get to this sooner :)