-
Notifications
You must be signed in to change notification settings - Fork 38.3k
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
Provide @Rule alternative to SpringJUnit4ClassRunner [SPR-7731] #12387
Comments
Neale Upstone commented There's a push for this from the JUnit end, where David Saff has suggested a move to |
Neale Upstone commented Sam. I'm looking into this and will hopefully get to prototype it on Thursday. Let me know if you make a start. I suspect you may find some limitations to the I'd be interested on feedback on whether this looks useful for the |
Neale Upstone commented Here's a proof of concept based on a reasonable patch for ComplexRule support in JUnit 4.9. To use, you'll need the two commits from this branch: https://github.com/nealeu/junit/commits/complex-rules. i.e. checkout the complex-rules branch from [email protected]:nealeu/junit.git |
Neale Upstone commented Sam & Dave If you can find an elegant way to avoid the need for ComplexRule, then I'll buy you a beer! Failing that, I suggest you checkout my JUnit git branch and use the attached file as a starting point, such that you can give some yay-nay feedback in time to get some agreement on what support is needed at the JUnit end. As noted in the javadoc of my attachment, I think this would be most elegant if JUnit also allowed I'll park it there for now. |
Dave Syer commented I'll have a look at your junit code. Any chance of an executive summary of what it does and why it's important for Spring test context? Is it just because a Spring test context manager has a role at the class and the method level? I imagine you could hack around that with a static instance of a normal Note that you can submit pull requests to Spring via https://github.com/cbeams/spring-framework. |
Neale Upstone commented Pull requests... that's a bit 21st century isn't it :p Didn't know, so thanks. Forkin marvellous as some might say. Static With just that enhancement to JUnit's Description class, your job becomes a lot easier, as the static Introducing ComplexRule does a couple of things (as you'll see at nealeu/junit@da6c5e5#diff-0):
Actually I didn't need the Class param, as that's avail as Description.getTestClass(). So, in summary, it looks like minimal requirements at JUnit end are:
I could rustle up an alternative JUnit patch later if that looks about right. Dave, I've given you commit rights on my junit fork if you want to hack on what I've already done. |
Neale Upstone commented Take 2: Just pushed to branch enhanced-rule what I think is sufficient for above. See https://github.com/nealeu/junit/commits/enhanced-rules |
Neale Upstone commented Tweaked to work with lower impact enhanced-rule proposal for JUnit instead of introducing ComplexRule. |
Neale Upstone commented There's a related JUnit issue, https://github.com/KentBeck/junit/issues/#issue/32, which looks for JUnit to manage the instantiation of Rules. I've commented on how I think this can go forwards and requested feedback before diving in with another proactive patch. For replacing features currently requiring Runners I'm suggesting that we're really looking at a broader interceptor model, which would be a good foundation for both Rules and Runner replacements. |
Dave Syer commented I think an interceptor model would be good for JUnit rules - I commented to that effect on one of their issues (not sure if it was that one). If a rule had the ability to modify the chain it was involved in by returning a decorated version it would open up a lot of useful possibilities (e.g. switching off the |
Neale Upstone commented Looks like modifying the chain could be on the cards: From https://github.com/KentBeck/junit/issues/199:
|
Sam Brannen commented Example of a limited, partial solution in the wild: SpringIntegration Rule from the Thucydides project. |
The Alchemist commented Any updates on this? |
Sam Brannen commented The Alchemist, this issue is currently assigned to the 3.3 backlog and will be reassigned to a concrete release pending available resources and demand from the community. Regards, Sam |
Sam Brannen commented Pull request submitted by Philippe Marschall in conjunction with #14850: |
Neale Upstone commented Just noticed that I've broken links here to my github account. I changed account name from nealeoc to nealeu - edit the links and they'll work, if you need to get at them. |
Vasiliy Gagin commented I'm very interested in this being implemented. I've being discussing this with JUnit guys, asking them to provide better integration points, no luck yet. |
Sam Brannen commented
No, a change of this magnitude would not be possible since it would break backwards compatibility. Furthermore, JUnit's |
Eric Rizzo commented Pull request is more than 2 years old; what's the status? What's holding back getting this issue addressed? |
Elnur Abdurrakhimov commented I want/need this too. It's a shame it hasn't been implemented yet. Each developer/team has to come up with their own hacks to solve the same common problem. |
Sam Brannen commented FYI: development work for this issue is tracked in my #12387 feature branch. Feel free to follow along or try it out for yourself. Note, however, that this is a work in progress (based off the proof of concept from Philippe Marschall). |
Sam Brannen commented UpdateThe work on this issue is now (hopefully) feature complete. The following is an example of how the new @ContextConfiguration
public class BaseAppCtxRuleTests {
@Configuration
static class Config {
@Bean
public String foo() {
return "foo";
}
}
@ClassRule
public static final SpringClassRule SPRING_CLASS_RULE = new SpringClassRule();
@Rule
public final SpringMethodRule springMethodRule = new SpringMethodRule(this);
@Autowired
String foo;
@Test
public void test() {
assertEquals("foo", foo);
}
} I would appreciate feedback within the next few days since we plan to complete development for Spring Framework 4.2 RC1 next week. So, thanks in advance for any input you can provide! Sam |
The Alchemist commented @Sam Brannen: Thanks, Sam! Is there a SNAPSHOT release on a Maven repo somewhere we can try? |
Sam Brannen commented Hi The Alchemist, There's not yet a snapshot in a public Maven repository, but I'm attaching a locally built Regards, Sam p.s. I've just pushed my feature branch to the official spring-framework GitHub repository, and that's building now: https://build.spring.io/browse/SPR-PUB3-1 So hopefully (if all works well), there should be a publicly available snapshot soon. |
Sam Brannen commented Hi The Alchemist, You should now be able to use the following to try out the snapshot.
Let me know how it goes! |
Dave Syer commented Looks awesome and seems to work with Spring Boot. Do we have any guice with the JUnit devs (you must know how to communicate with them by now) over the 2 rules thing? Can you explain a bit what the problem is? I don't like the names particularly ("ClassRule" and "Rule" are duplicated). Maybe they could be more descriptive, like other I also wondered if the ClassRule could live in an interface so it doesn't have to be in every test class (I tried it and it didn't work but I don't know if there's something about the implementation of the MethodRule that might need to change to support that). |
Mark Michaelis commented At least with JUnit 4.12 it is possible to annotate a static field with both, @ClassRule
@Rule
public static TestRule temporaryFolder = new TemporaryFolder(); For details see:
Unfortunately without changing JUnit we still require two rules as we need a non-static field to refer to See:
Derived from the discussion mentioned above the following might have worked: @ClassRule
@Rule
public static CombinedTestRuleMethodRule THE_RULE = new CombinedTestRuleMethodRule(); where Unfortunately // We disallow:
// - static MethodRule members
// - static @Rule annotated members
// - UNLESS they're also @ClassRule annotated
// Note that MethodRule cannot be annotated with @ClassRule Thus to get rid of the two rules above we would require issue #351 to be fixed -- possibly by just removing this validation for MethodRules. I will add a comment to the issue with reference to this issue. |
Sam Brannen commented Hi Dave,
Great. Thanks for trying it out! I'll now respond to your points in separate comments. Cheers, Sam |
Sam Brannen commented
There have certainly been numerous discussions over the years. If you're interested in knowing the gory details, you'll have to read through the comments here:
|
Sam Brannen commented
Yes, I can do that. ;) The Spring TestContext Framework cannot work properly unless all of the lifecycle callbacks in Core API of TestContextManager:
Spring must therefore be able to invoke JUnit rules can implement either There are several restrictions to using JUnit rules (see JUnit's
In summary, the above restrictions simply make it impossible for Spring to provide a single rule. :( The state of affairs may potentially (hopefully!) change with JUnit 5, but that likely will not happen until 2016. |
Sam Brannen commented
As I mentioned earlier, the responsibilities are split between the class-level and instance/method-level callbacks. Purely descriptive alternatives could include something like So, if you or anyone else can come up with better names, I'd be glad to hear them! :) |
Sam Brannen commented
It's unfortunately not possible to declare it on an interface. JUnit's infrastructure for finding annotations is very limited in comparison to Spring's But you shouldn't have any problem declaring the |
Sam Brannen commented Mark Michaelis, thank you for the thorough analysis and numerous links! And... it appears we were composing similar answers simultaneously. ;) |
Sam Brannen commented Hi everybody, I'm going to hold off on pushing my branch to That will give more of you a chance to try out the snapshot (see comments above) and provide feedback. Cheers, Sam |
Sam Brannen commented The Alchemist, did you have a chance to try out the new rule support yet? |
Sam Brannen commented FYI: I have committed some changes with bug fixes and complete Javadoc. These are included in latest Feel free to give it a try! :) |
Sam Brannen commented Implemented as described in GitHub commit d1b1c4f:
|
Sam Brannen commented FYI: minor changes in latest commit:
Consequently, the required configuration now looks like this: @ClassRule
public static final SpringClassRule SPRING_CLASS_RULE = new SpringClassRule();
@Rule
public final SpringMethodRule springMethodRule = new SpringMethodRule(); The only change from the user's perspective is that Regards, Sam |
Thomas Darimont commented Just stumbled upon this issue, this is great stuff Sam! I just wondered whether it would be possible to extend this PoC to add support for specifying test case specific configuration classes which comes in handy If you want to be able to test different configurations with just minor differences. One could specify an additional config class to use via (a new) @ContextConfiguration(loader = AnnotationConfigContextLoader.class)
public class SpringJunit4RunnerRuleTests {
@ClassRule public static SpringContext springContext = new SpringContext();
@Autowired Foo foo;
@Test
public void test1_should_use_ConcreteFoo() {
Assert.assertNotNull(foo);
Assert.assertTrue(foo instanceof ConcreteFoo);
}
@Test
@Use(SpecialConfig.class)
public void test2_should_use_SpecialFoo() {
Assert.assertNotNull(foo);
Assert.assertTrue(foo instanceof SpecialFoo);
}
@Test
public void test3_should_use_ConcreteFoo() {
Assert.assertNotNull(foo);
Assert.assertTrue(foo instanceof ConcreteFoo);
}
static interface Foo {}
static class ConcreteFoo implements Foo {};
static class SpecialFoo implements Foo {}
@Configuration
static class Config {
@Bean
public Foo foo() {
return new ConcreteFoo();
}
}
// Not visible for the test directly
public static class Nested {
@Configuration
public static class SpecialConfig extends Config {
@Bean
public Foo foo() {
return new SpecialFoo();
}
}
}
} |
Dave Syer commented I like that (I think I suggested using |
Sam Brannen commented Thanks, Thomas! Glad you like the Rule support. :) Regarding your proposal, we already have something similar in SPR-12031. So could you please post your comments to that JIRA issue? Cheers, Sam |
Dave Syer opened SPR-7731 and commented
Overview
Sometimes (always?) it would be nice to have a
@Rule
implementation that did the job of theSpringJUnit4ClassRunner
.The main motivation for this is to be able to use other runners (e.g.
@Parameterized
,@Categories
, Mockito, etc.).There might even be a case for deprecating the runner in favor of a
@Rule
?Further Resources
Attachments:
Issue Links:
@Rule
executes outside of transaction when using the TransactionalTestExecutionListener ("is depended on by")25 votes, 30 watchers
The text was updated successfully, but these errors were encountered: