Remove restrictions on resource directory names. #744
Conversation
| if (!resourcePath.resourceBase.toString().endsWith(separator + "res")) | ||
| { | ||
| throw new IllegalArgumentException("Resource path must end in \"" + separator + "res\""); | ||
| if (resourcePath.packageName.equals("android") && !resourcePath.resourceBase.toString().endsWith("/res")) { |
There was a problem hiding this comment.
Should this be File.separator + "res"?
There was a problem hiding this comment.
Android package resources are loaded from a jar. And jars always have "/" separator.
There was a problem hiding this comment.
Oh, whoops. I didn't even look at that package name.
There was a problem hiding this comment.
Well, the android resource path should never end in anything other than res if it is indeed an android jar. This check was mostly due to app resources declared by gradle that didn't end in res. maybe the whole check can be removed now.
|
No, We can't remove the restriction! The restriction has a purpose. The res name MUST end in res to avoid conflicts with something like this:
because the project name has values or xml or anything that may confuse robolectric into thinking the folder is a resources folder it will start behaving wrong. enforcing the folder to end in res makes it possible to guarantee this will not happen. See this #648 #647 for more info on this restriction. |
|
Gradle can have an additional task to copy the resources to a folder ending in res and will make it through the restriction. better to have it that way. This plugin already copies the resources to a folder ending in res and such passes robolectric restriction: |
|
Ew, that's gross. Forcing people to copy? No thanks. We need a better solution than that. |
|
Couldn't we just replace |
|
Or checking for |
|
I think that directory matching filter is used in more places than just resource loading. changing the logic might affect other parts. its not as simple as that. The directory matching filter is there to filter if ANY part of the path contains a filter. changing the logic will work for the resource loading but as I said. there are more parts of robolectric that use this. I'm not sure how would this affect. |
|
Maybe a new ResourceDirectoryMatchingFilter that is used only in resource loading. then it would work I guess. |
|
I've checked usage before making the change. And you can see also that I made the filter to be package private. Perhaps it's even better to make it an inner class. |
|
If It is not used anywhere else, then I guess that works and the enforcing can be removed. |
|
ok, it seems to be finished now. |
|
Hello all, does anyone know how to get past this in gradle? I'm using Gradle 1.6 from AndroidStudio's android-studio/sdk/tools/templates/gradle/wrapper/gradlew . My build.gradle has I know that it's working because if I change it to './blah', I get a bunch of... So as far as I can tell, gradle knows it ends in /res, but yet I keep getting this Ex thrown at me. As far as I can see, my resource dir truly ends in "/res". I don't understand what to put in my build.gradle that will make robolectric happy. |
|
@tokunbo It does not depend on you source directory name. Gradle puts all the merged resources to a separate output directory located at |
|
Oh... so there's no way for me to solve this now other than to wait for this pull request to make it into a new robolectric.jar at https://oss.sonatype.org/index.html#nexus-search;quick~org.robolectric ? Can I alter gradle's output directory to something that robolectric-2.2.jar would be happy with? ...even risking the conflicts that SuperJugy talks about? EDIT : I'm trying to change the source code of the plugin to output to a dir that this robolectric-2.2.jar is happy with. |
|
You can use my gradle plugin and it will copy the resources to a dir that ends in res. https://github.com/JCAndKSolutions/android-unit-test |
|
I kinda want to be on robolectric-2.2, which I'm told uses PhoneWindow allowing ActionBarSherlock to work easily. |
|
Needs rebase. |
|
The only context I know of in regards to "rebase", is "git rebase". :-) Is this a hint on what I need to change to make squareup plugin's do the copy to "/res"? ...or is this advice that what I'm trying to do is a lot more work than some quick hack I'm trying to pull off? :-) |
|
Copying to a folder ending in Rebase was towards the PR author, not you. |
|
Oh dear, okay... I'll slowly walk backwards out of this debate ^_^; |
|
Okay, I did THEE DIRTIEST hack ever. Everyone on this thread will hate me for it, but I'm posting it here for anyone else who is foolish enough to do what I just did.
Caused by: java.lang.IllegalStateException: System services not available to Activities before onCreate() |
|
Okay again... so I switched to trying... about_activity = Robolectric.buildActivity(AboutActivity.class).create().get(); But that failed because of ActionBar stuff, but the following stackoverflow got me past that: So now: java.lang.RuntimeException: Couldn't find content container view |
|
I'll rebase as soon as get to the working desk.
|
…tric to use gradle output directory which does not necessarily have a name ending with /res. change directories matcher logic turn directories filter into inner class, add tests for tricky resource path loading remove path checks depending on file separator remove all the checks on resources path
|
Rebased and squashed! 💥 |
|
@tokunbo you ended up doing manually what my plugin would have done automatically, lol. The problem with your approach is that if you change your resources, you'll have to manually copy them again. Also, if you have flavors with different resources, you won't be able to handle those cases. My plugin solves all that automatically. Until this PR gets merged and 2.3 released, the only way that I know of to automatically handle robolectric 2.2 with gradle 1.8 is with my plugin or a very big build.gradle file. Even if Robolectric 2.3 is released, I would still have to support copying of files since many people might still use robolectric 2.2. The only other approach would be to create a symlink between a dir that ends in res and the dir that android plugin uses. avoiding the copy completely. |
|
@SuperJugy ....but I really need a solution to the Runtime Exception I can write other unittests for things not needing an Activity, but if this wasn't blocking me... I could write some very useful unittests really fast, throw 'em into JenkinsCI and everything would be great.... especially since the "GUI in JVM" thing is the main selling-point that I talk about ^_^; So... I guess this issue is going to be closed soon since the actual topic of "/res" has been decided. Where do I open an issue for my RuntimeException? Is this a ActionSherlock issue or Robolectric or a bit of both? ....or I'm doing something wrong? |
|
Whoa! Hey there! If I do this: ....it works! ...but only for one of my activities.... the other activity still gives the container view error.... how odd. |
|
@tokunbo My plugin works for 2.1 and 2.2 and even 2.3-snapshot of robolectric. the difference between them doesn't mather. The plugin doesn't even need robolectric, all it does is to launch JUnit tests. if the tests happen to be robolectric related that is other thing. About your problem: What's the full stacktrace? at what point does it crash? in the onCreate, onStart or onResume? (which I think you aren't calling). You really need to provide more info and in a separate Ticket since this has nothing to do with this PR. |
|
@SuperJugy Though, good to know that I can use your plugin the moment my current setup becomes a pain and this PR isn't in a useable jar yet. The code that fails is: Robolectric.buildActivity(AboutActivity.class).create().start().get(); I'm using @RunWith(RobolectricTestRunner.class) The trace is: java.lang.RuntimeException: Couldn't find content container view |
|
Ask on the mailing list. Your runtime failure is unrelated to this ticket. |
|
Will do. Okay, that ends my stdout on this thread. Thanks & seee y'all around -^_^- |
|
Hasn't anyone taken a look at the PR? |
|
@roman-mazur taking a look now! |
Remove restrictions on resource directory names.
|
Thanks for this!. It is the first time in a long time that i can run my robolectric tests on the latest snapshot without custom "tricks" :) |
|
@SuperJugy Man, you saved my days 👍 I'm a kinda newbie in Android platform, and I'm digging on this Android studio, gradle, Robolectric(2.2) for more than 1 weeks. and finally, your github project https://github.com/JCAndKSolutions/android-unit-test comes out :) |
|
@kwosu87 I'm glad I could help you 😄 |
|
@Superjugy Anyway, I have a question.Do you know why testCompile libraries don't show in External Libraries in Android Studio after syncinh with buld.gradle?I still can't figure out why :( |
|
Oh that's because test compile is defined in my plugin not in android I know it's annoying but there is no other easy way currently. |
|
@SuperJugy Oh i see! I'll try i and thank you for the direction!! :) |
We can point robolectric to use gradle output directory (with a system property) which does not necessarily have a name ending with /res (like build/res/dev/debug).