Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply @Config annotation to TestClass hierarchy. #1596

Closed
wants to merge 1 commit into from
Closed

Apply @Config annotation to TestClass hierarchy. #1596

wants to merge 1 commit into from

Conversation

dmorehouse
Copy link
Contributor

This change applies @config annotations for a test classes entire hierarchy. This is very useful for putting @RunsWith and @config settings in a parent test class and extending the parent for individual tests classes.

My company's projects currently extend RobolectricTestRunner and overload the getConfig method to add this functionality allowing for a common place for our @RunsWith and @config annotations. We find that > 80% of the time we don't override them in the base test classes.

NOTE: This change could possibly break existing Robolectric tests IFF there exists a hierarchy and a parent of a test class has an @config annotation which is currently being ignored. Seems like this would only happen if someone extended an actual test class with @test annotated methods which seems rare. For this reason, this doesn't seem like a good change for a bug fix release. It's probably a good candidate for 3.0 or a 2.x release.

… for easy TestClass extending allowing for easy sharing of common @config settings.
@jongerrish
Copy link
Contributor

Why not prefer to use the org.robolectric.Config.properties property file
to configure your default values?

See http://robolectric.org/configuring/

On 1 March 2015 at 06:47, Dave Morehouse notifications@github.com wrote:

This change applies @config https://github.com/Config annotations for a
test classes entire hierarchy. This is very useful for putting @RunsWith
and @config https://github.com/Config settings in a parent test class
and extending the parent for individual tests classes.

My company's projects currently extend RobolectricTestRunner and overload
the getConfig method to add this functionality allowing for a common place
for our @RunsWith and @config https://github.com/Config annotations. We
find that > 80% of the time we don't override them in the base test classes.

NOTE: This change could possibly break existing Robolectric tests IFF
there exists a hierarchy and a parent of a test class has an @config
https://github.com/Config annotation which is currently being ignored.
Seems like this would only happen if someone extended an actual test class
with @test https://github.com/Test annotated methods which seems rare.
For this reason, this doesn't seem like a good change for a bug fix

release. It's probably a good candidate for 3.0 or a 2.x release.

You can view, comment on, or merge this pull request online at:

#1596
Commit Summary

  • Added applying @config annotation to TestClass hierarchy. This
    allows for easy TestClass extending allowing for easy sharing of common
    @config settings.

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#1596.

@dmorehouse
Copy link
Contributor Author

We debated using the properties file. However, adding the @config hierarchy gave us more flexibility (it can be used for more than just defaults) and solved the problem of having a property file per variant/flavour (we customize our gradle source dirs by variant). This way a we can have multiple standard settings (standard per class) or a single standard across all our variants.

Another reason we like this approach is we find it more intuitive (yes that's subjective) since using Robolectric requires @RunsWith. @config behaving differently from @RunsWith originally surprised us as it seems the two go pretty much hand in hand. We didn't want to split our defaults between a properties file and our @RunsWith in a class hierarchy. Also, the @config has lots of overriding already and hierarchy for the test class seemed pretty natural.

That said, properties make sense for just defaults and I'm not a huge fan of hierarchies however, it seems like @RunsWith almost drives a test class hierarchy unless you want to annotate every test with @RunsWith. If there was a compile time JUnit 4 way to automatically propagate annotations from a parent test class to a child, we'd use that instead. To my knowledge there's no standard way to accomplish that.

@erd
Copy link
Member

erd commented Mar 15, 2015

I like the idea of keeping as much of the config in code as possible, since it makes it easier to use automated refactoring tools. This is a good candidate for 3.0, since we've been making a lot of backwards incompatible changes.

@erd erd closed this in fab0af7 Mar 15, 2015
@erd
Copy link
Member

erd commented Mar 15, 2015

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants