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

SPR-10217 Implement JUnit 4 Support using Rules #222

Closed
wants to merge 1 commit into from

Conversation

marschall
Copy link
Contributor

Currently JUnit 4 support is provided by SpringJUnit4ClassRunner which
is a custom BlockJUnit4ClassRunner. There is no support for using other
runners like Theories or Parameterized or 3rd party runners like
MockitoJUnitRunner. A runner based approach does not seem to offer much
promise as runners are not composable, a custom Spring version of every
runner has to be developed and maintained.

With JUnit 4.9+ the preferred way to implement such behavior is to use
rules. Unlike runners there can be several ones of them and they can be
composed. In theory TestExecutionListener could be deprecated and be
replaced with standard JUnit rules but this seems to be a bit on the
drastic side.

This proposed implementation is using both a class rule and a method
rule. The class rule creates the TestContextManager, runs all the class
level callbacks and class level checks. The method rule runs all the
instance level callbacks and method level checks. I did not see a way
to implement the current functionality offered by
SpringJUnit4ClassRunner using only one rule.
Using two rules has the advantage that the implementation is cleaner
because it better separates the concerns. However it has the
disadvantage that it's harder to set up because both a method rule and
a class rule are needed. This also increases the potential for
misconfiguration.

The method rule has to be a MethodRule instead of a TestRule because
only the former has access to the test object with we need to perform
injection. This interface used to be deprecated once but doesn't seem
to be anymore. This creates a certain risk that it will be deprectated
again and potentially be remvoed in the future. An additional drawback
is that MethodRule unlike TestRule can only be defined in fields and
not methods. This is an unfortunate consequence of the implementation
of org.junit.runners.model.TestClass. As JUnit does not do
Field#setAccessible(true) this means that tests will have to be defined
in public fields.

Another minor issue is that tests not run because of IfProfileValue
will still show up in the Eclipse test tree, just blank.

In conclusion while the given implementation has some downsides I don't
see any other possible implementations given the current state of
affairs in JUnit.

  • Add SpringJUnitClassRule for all the class level processing
  • Add SpringJUnitMethodRule for all the method level processing
  • Add tests for the rules

SPR-10217

@ghost ghost assigned sbrannen Jan 26, 2013
@sbrannen
Copy link
Member

See SPR-7731 instead of SPR-10217.

@eeichinger
Copy link

just a quick note that the profile check could make use of assumptions: from the org.junit.Assume javadoc "The default JUnit runner treats tests with failing assumptions as ignored"

assumeTrue("required profile not active", ProfileValueUtils.isTestEnabledInThisEnvironment(getTestClass().getJavaClass()));

should do the trick

@eeichinger
Copy link

also it seems safe to stick with MethodRule - @deprecated has been removed as of junit 4.11 due to numerous user requests and the junit team has no intention to deprecate it again: https://github.com/KentBeck/junit/pull/519

@marschall
Copy link
Contributor Author

In theory we could have single rule that implements both TestRule and MethodRule but it would still have to be defined twice since @Rule fields and methods must not be static.

public class SampleTests {

    @ClassRule
    public static final SpringJUnitRule CLASS_RULE = new SpringJUnitRule();

    @Rule
    public MethodRule methodRule = CLASS_RULE;

} 

@marschall
Copy link
Contributor Author

@eeichinger good idea I updated the pull request

Currently JUnit 4 support is provided by SpringJUnit4ClassRunner which
is a custom BlockJUnit4ClassRunner. There is no support for using other
runners like Theories or Parameterized or 3rd party runners like
MockitoJUnitRunner. A runner based approach does not seem to offer much
promise as runners are not composable, a custom Spring version of every
runner has to be developed and maintained.

With JUnit 4.9+ the preferred way to implement such behavior is to use
rules. Unlike runners there can be several ones of them and they can be
composed. In theory TestExecutionListener could be deprecated and be
replaced with standard JUnit rules but this seems to be a bit on the
drastic side.

This proposed implementation is using both a class rule and a method
rule. The class rule creates the TestContextManager, runs all the class
level callbacks and class level checks. The method rule runs all the
instance level callbacks and method level checks. I did not see a way
to implement the current functionality offered by
SpringJUnit4ClassRunner using only one rule.
Using two rules has the advantage that the implementation is cleaner
because it better separates the concerns. However it has the
disadvantage that it's harder to set up because both a method rule and
a class rule are needed. This also increases the potential for
misconfiguration.

The method rule has to be a MethodRule instead of a TestRule because
only the former has access to the test object with we need to perform
injection. This interface used to be deprecated once but doesn't seem
to be anymore. This creates a certain risk that it will be deprectated
again and potentially be remvoed in the future. An additional drawback
is that MethodRule unlike TestRule can only be defined in fields and
not methods. This is an unfortunate consequence of the implementation
of org.junit.runners.model.TestClass. As JUnit does not do
Field#setAccessible(true) this means that tests will have to be defined
in public fields.

Another minor issue is that tests not run because of IfProfileValue
will still show up in the Eclipse test tree, just blank.

In conclusion while the given implementation has some downsides I don't
see any other possible implementations given the current state of
affairs in JUnit.

- Add SpringJUnitClassRule for all the class level processing
- Add SpringJUnitMethodRule for all the method level processing
- Add tests for the rules

SPR-10217
@eeichinger
Copy link

@marschall cool. not sure how many people are actually using @BeforeClass, but I mostly don't. so it's a bit of an inconvenience always having to the define both rules. Asfaik you can safely create two separate instances of TestContextManager as they're accessing the same static contextcache anyway. This way one has to define the @ClassRule only if you really need it for @BeforeClass

Other than that I've taken your code and use it 2 projects already, works like a charme!

@marschall
Copy link
Contributor Author

@eeichinger also TestExecutionListener#beforeTestClass and TestExecutionListener#afterTestClass wouldn't fire. Although TestExecutionListener is redundant when you have rules available. I can see your point it makes it easier to use OTOH it's harder to debug why something doesn't doesn't work. It's a trade off, I can live with either solution.

And thanks for testing.

Edit:
Also more TestExecutionListener instances would be created.

@eeichinger
Copy link

the TestExecutionListeners should be stateless anyway, so no problem. But I see your point that TestExecutionListeners might expect before/afterTestClass to be called before before/afterTest get called. None of the default TestExecutionListeners do so, but custom ones might break. Which I'd say leaves it up to the user to configure the @ClassRule if his test setups require so

just wondering: does the junit api allow for a classrule to register an instance rule?

@marschall
Copy link
Contributor Author

No, unfortunately a classrule (actually just a TestRule) just gets a callback to affect the class execution (before and after). Basically it's a higher order function.

There is a proposal for an API that would do exactly what we need but there doesn't seem to be any progress happening.

@eeichinger
Copy link

send a pull request... :)

@erizzo
Copy link

erizzo commented Apr 8, 2015

The comments in SpringJUnitMethodRule mention that ExpectedException is not supported; is that indeed true? If so, can you elaborate a bit?

@sbrannen
Copy link
Member

sbrannen commented Apr 9, 2015

The Javadoc for SpringJUnitMethodRule with regard to ExpectedException is admittedly misleading at a glance.

If you take a look at the imports in SpringJUnitMethodRule, you will notice that the class referenced in the Javadoc is org.springframework.test.annotation.ExpectedException (i.e., the @ExpectedException annotation that was supported in earlier versions of the Spring TestContext Framework).

So fear not: JUnit's org.junit.rules.ExpectedException (i.e., the ExpectedException Rule) should be fully supported when used in conjunction with SpringJUnitMethodRule.

@sbrannen
Copy link
Member

sbrannen commented Apr 9, 2015

For what it's worth, @ExpectedException was permanently removed from the spring-test module in Spring Framework 4.0 (see SPR-10499). Thus, for modern versions of Spring there should no longer be any confusion. ;)

@sbrannen
Copy link
Member

@marschall and @eeichinger, thanks so much for all of your ideas and collaboration on this!

I've just committed support for this in Spring Framework 4.2 RC1 here: d1b1c4f.

Cheers,

Sam

@sbrannen sbrannen closed this May 16, 2015
@sbrannen
Copy link
Member

Though... perhaps I should attribute the Assume idea in ProfileValueChecker to @eeichinger instead of @marschall. ;)

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

Successfully merging this pull request may close these issues.

4 participants