Add JUnit Assumptions to core test sources #142

Open
wants to merge 1 commit into
from

Projects

None yet

3 participants

@rwinch
Member
rwinch commented Sep 10, 2012

As per discussion in this weeks stand up I have added a utility class that allows assuming JDK version in order for the test to run. Also see the junit-assumptions-demo branch to see this in action (since there are no use cases in the actual build now).

Previously it was cumbersome for tests to assert if the current JDK was at
least JDK 1.7.

There is now a utility class for JUnit assumptions that provides a method
that allows tests to assume the current JDK is at least JDK 1.7. If the
assumption is not met, a AssumptionViolatedException is thrown to indicate
the test should be skipped.

@cbeams
Contributor
cbeams commented Sep 10, 2012

Hey Rob,

I see this is under src/test in spring-core. In our current Gradle build arrangement, this will be accessible only to classes within spring-core. i.e. Assumptions could not be used from spring-beans.

I have been meaning to look into sharing test classpaths across dependend projects -- it would allow us to clean up many duplicate test classes, e.g. the many variants of TestBean. Even if we wouldn't actually take the time to clean the old ones up, it would avoid the creation of further dupes in the future (See MockHttpServletRequest for a particularly nasty example)

@cbeams cbeams was assigned Sep 10, 2012
@philwebb
Member

@cbeams I have seen maven projects produce additional test jars that can then be consumed [1], I assume gradle can be even smarter. Perhaps we can automatically add a test dependency for each internal dependency?

@rwinch Do you think this approach could be extended to use annotations, perhaps using a Rule or Runner? Something like @RunIfJdk(JavaVersion.VERSION_7). I have not used 'assume' before, will it work for a single test in class?

[1] http://maven.apache.org/guides/mini/guide-attached-tests.html

@rwinch
Member
rwinch commented Sep 10, 2012

@cbeams Thanks for pointing this out. My update to build.gradle should allow the test source to be shared. We have used a similar approach successfully in Spring Security. The junit-assumptions-demo branch shows this working with spring-web. However, you are correct that spring-beans does not appear to be able to use Assertions for some reason. I will look into this further.

@philwebb Thanks for the input. My thoughts on the current approach vs custom JUnit Runner or Rule:

  • Static imports require less custom code
  • Static imports are more concise than a JUnit Rule.
  • Static imports are more flexible than a JUnit runner (not tied to a specific runner).

I mentioned these thoughts to @cbeams this morning and he seemed to agree on the logic (although he is pretty busy so I could see a change of heart). The advantage for the using a custom Rule or Runner is that some might state that it is more readable. I'm not sure I am convinced of this, but if a custom runner and/or rules this is something that we are looking for it could be added.

@rwinch
Member
rwinch commented Sep 11, 2012

The reason it did not work with spring-beans was due to GRADLE-2471. I have modified the build so that it works around the issue. I also updated the junit-assumptions-demo branch to demonstrate that all the projects can have use Assumptions as a test dependency. There are two commits in the branch. The first demos it all working. The second demonstrates that the build will actually fail if ran against JDK 1.6 and Assumptions are not used.

@rwinch
Member
rwinch commented Sep 11, 2012

@philwebb I realized I missed one of your questions. Yes you can use it for an entire test or for a specific test method. If you like, you can see the class level javadoc of newly added Assumptions class for examples. Sorry for overlooking this previously.

@rwinch rwinch Add JUnit Assumptions to core test sources
Previously it was cumbersome for tests to assert if the current JDK was at
least JDK 1.7.

There is now a utility class for JUnit assumptions that provides a method
that allows tests to assume the current JDK is at least JDK 1.7. If the
assumption is not met, a AssumptionViolatedException is thrown to indicate
the test should be skipped.
efdcadb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment