Fixed activity theme handling in RobolectricPackageManager

[jrgonzalezg]

It was giving an IllegalStateException for the style not being fully qualified. Fix is based on similar code on org.robolectric.shadows.ShadowActivity.

[JakeWharton]

This should be on the previous line

} else {

[jrgonzalezg]

Fixed!

[coreydowning]

@jrgonzalezg Could you write a test for this new behavior? Thanks!

[jrgonzalezg]

I have been looking at the Robolectric tests and this "is already tested" on src/test/java/org/robolectric/shadows/ThemeTest.java but there are some problems with the tests here. My fix should affect the behaviour of the test whenNotSetOnActivityInManifest_activityGetsThemeFromApplicationInManifest(), but its implementation is wrong and it does not actually test what the name suggest. I think at some point the implementation of this test and the whenSetOnActivityInManifest_activityGetsThemeFromActivityInManifest() were swapped and they need to replace one another again.

With the above change, the relevant test is now:

@Test public void whenNotSetOnActivityInManifest_activityGetsThemeFromApplicationInManifest() throws Exception {
  TestActivity activity = buildActivity(TestActivity.class).create().get();
  Button theButton = (Button) activity.findViewById(R.id.button);
  assertThat(theButton.getBackground()).isEqualTo(new ColorDrawable(0xff00ff00));
}

The problem is that the test passes both with my patch and without it even if the code without the patch gives problems on other projects. I have to say that i am quite new to unit testing and maybe i made the mistake of starting directly in Robolectric to learn unit testing ;).

From my current knowledge, my guess is that the whenNotSetOnActivityInManifest_activityGetsThemeFromApplicationInManifest() test passes there both with and without my patch due to specific package names and my patch is only useful when the package name differs but i do not know how to reproduce this on a test.

Additionally, my patch is based on the strategy followed for the same purpose on the ShadowActivity class. The related function on ShadowActivity is this:

public boolean setThemeFromManifest() {
     ShadowApplication shadowApplication = shadowOf(realActivity.getApplication());
     AndroidManifest appManifest = shadowApplication.getAppManifest();
     if (appManifest == null) return false;

     String themeRef = appManifest.getThemeRef(realActivity.getClass());

     if (themeRef != null) {
       ResName style = ResName.qualifyResName(themeRef.replace("@", ""), appManifest.getPackageName(), "style");
       Integer themeRes = shadowApplication.getResourceLoader().getResourceIndex().getResourceId(style);
       if (themeRes == null)
         throw new Resources.NotFoundException("no such theme " + style.getFullyQualifiedName());
       realActivity.setTheme(themeRes);
       return true;
     }
     return false;
   }

but i can't find a Robolectric test for that function either, so i can not use it as the basis for a new test. So i am currently stuck at this point. But the thing is that if that code is correct in ShadowActivity it is probably correct too in the RobolectricPackageManager patch or at least it fixes the problems i was having with my tests without breaking any of the current Robolectric tests.

Regards,
Juan Ramón González

[coreydowning]

@jrgonzalezg Thanks for the awesome explanation. I'll dig into this a little bit and see if I can find a way to test it. ☕

[coreydowning]

Just fixed the problem of ThemeTest sucking on my branch here cd7a22c. ColorDrawable#equals() returns true for any garbage you pass it which would explain the always passing.

What does the AndroidManifest look like for the error you're having in your project?

[jrgonzalezg]

Thanks for looking into this! I can not look at it on the computer until tomorrow here. But basically my manifests set a Theme.App in the application tag and no themes on activities. Theme.App inherits from a very basic Theme.Lib on a library project (gradle .aar) which may also be affecting the package resolution for the theme in Robolectric (maybe this is the reason for the patch not being needed on some other cases). Lib project has package naming of the form com.example.lib.android and app package is generally something like com.example.app.android Hope this helps or i could provide further info or specific error messages when i gain access to the computer tomorrow.

[jrgonzalezg]

The exception that is produced is:

java.lang.IllegalStateException: "@style/Theme.App" is not fully qualified
at org.robolectric.res.ResName.(ResName.java:27)
at org.robolectric.res.builder.RobolectricPackageManager.getActivityInfo(RobolectricPackageManager.java:70)

But just now i have found that it also fails when setting the theme on the activity. I have just made a test on another previously untested activity that has a custom theme @style/Theme.App.SettingsActivity (that inherits from Theme.App that inherits from Theme.Lib) and it also crashes with similar error:

java.lang.IllegalStateException: "@style/Theme.App.SettingsActivity" is not fully qualified
at org.robolectric.res.ResName.(ResName.java:27)
at org.robolectric.res.builder.RobolectricPackageManager.getActivityInfo(RobolectricPackageManager.java:70)

so i guess now that the problem is not either what the tests we were commenting test. It is not that the activity does not get the theme from the application but that the theme it gets does not get properly qualified without the patch. Both previous exception are gone when the patch is used.

I am not sure if this also happens when using Robolectric with maven since i don't have any projects based on maven now. But for running my tests with gradle and Robolectric 2.3-SNAPSHOT i also have to use an older version of org/robolectric/res/PackageResourceLoader.java since lasts commits merged break the loading of resources with Gradle folder structure (all last merges related to separators and requiring a folder named "res" as the root of the resources ( gradle uses things like build/res/all/debug instead as i already commented in #647 ).

[coreydowning]

This is probably a gradle .aar problem. Going to merge it since it doesn't break any existing behavior.

😇

[jrgonzalezg]

Thanks! :D. I tried a bit to set up a test for this but i wasn't getting to far, since it requires a lot of setup to reach the status needed for the testing...