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

Is it / should it be possible to have a single rule act before, between, and after tests? #793

Open
rowanhill opened this issue Dec 26, 2013 · 14 comments
Labels

Comments

@rowanhill
Copy link
Contributor

Hi,

As of 4.11 (and specifically #343 / #339), it's no longer possible to annotate a single rule as both @ClassRule and @Rule (as the former must be static and the latter non-static).

Is it still possible to have a single rule which acts both before & after the class' tests (e.g. to set up and tear down an external resource) and between each tests (e.g. to reset the external resource)?

If not, is there any appetite for allowing such a thing?

Note that there are currently work-arounds:

  1. Split the responsibilities into independent @ClassRule and @Rule implementations.
  2. Define a static @ClassRule field, then declare a non-static @Rule and set it's value to that of the static field.
  3. Create a @Rule that keeps static state (e.g. the external resource), and infers whether it should perform a before-class type action (e.g. set up the resource) or a per-test type action (e.g. reset the resource).

They have drawbacks, however: 1 and 2 require a developer to remember to do two things when writing their test (i.e. there are two coupled rules, and the test won't work as they expect if either is missing); 3 doesn't allow for after-class type actions (e.g. cleaning up an external resource), although these can be provided via an @AfterClass method (although, again, that couples two elements in the test).

I'd be interested to hear of any other suggestions (either in terms of workarounds or, if there is any interest, in how support for this could be implemented as a JUnit feature).

Thanks,
Rowan

P.S. See also my question on StackOverflow about the same subject.

@kcooney kcooney added the rules label Mar 9, 2014
@phoenix384
Copy link

In my opinion forcing rules to be non-static was an annoying change and an absolutely unnecessary restriction.
I can do it anyway by e.g. assigning the value of the static field to a non-static field like Rowan already said (2.). But it is an ugly workaround that I'm forced to use in some cases.
If you ask me the change of #343 / #339 should be reverted.

@kcooney
Copy link
Member

kcooney commented Jun 15, 2014

@phoenix384 I would be willing to accept a pull request where fields annotated with both @Rule and @ClassRule could be static. IIMHO, if a static field is annotated with @Rule but not @ClassRule it is most likely an error.

@rowanhill
Copy link
Contributor Author

@kcooney That sounds perfectly reasonable to me, and would satisfy my use case. I'm willing to at least look into submitting a pull request. I'll post back here if I'm not going to be able to sort that out in the short term.

@kcooney
Copy link
Member

kcooney commented Jun 19, 2014

Thanks to @rowanhill fields and methods annotated with @Rule can be static as long as they are also annotated with @ClassRule

I still think we need a real solution to the problem of having a single entity that can act as a class rule and a test rule, so I'm leaving this bug open.

@sbrannen
Copy link
Member

fields and methods annotated with @rule can be static as long as they are also annotated with @ClassRule

...except that such a field cannot implement MethodRule. :(

I still think we need a real solution to the problem of having a single entity that can act as a class rule and a test rule, so I'm leaving this bug open.

I couldn't agree more!

@kcooney
Copy link
Member

kcooney commented May 15, 2015

@sbrannen A MethodRule cannot work on the class level, so it I think it's reasonable to have a validation error for that.

It sounds like you want an API that works on the test method level and the class level. If someone has ideas for how we might design a new API that works on the class level and method level, I suggest writing up a concrete proposal in Google Docs and adding a link to that doc on this bug

@kcooney kcooney closed this as completed May 15, 2015
@kcooney
Copy link
Member

kcooney commented May 15, 2015

Woops! Didn't mean to close this.

@sbrannen
Copy link
Member

@kcooney, while it's certainly true that a MethodRule cannot work on the class level, that doesn't mean that JUnit should not support a class that implements both TestRule and MethodRule.

See my comment here: #351 (comment)

Now, I agree that we should define a new API, but I would slate that for JUnit 5 instead of 4.x.

In 4.13 we have the opportunity to support a rule that is applied at both the class level and method level and has access to the test instance.

As it stands now, it is possible to have a rule that is applied at both the class level and method level, but such a rule does not have access to the test instance since it cannot implement MethodRule.

And well, that's a bit of a lie: it can implement MethodRule (based on pure Java), but... JUnit ignores the fact that it implements MethodRule and instead invokes it as a TestRule even for method-level callbacks.

In summary, what I'm suggesting is that JUnit -- for method-level application -- always invoke a rule as a MethodRule if it implements MethodRule and as a TestRule otherwise. This would mean that such a rule would not be invoked as a TestRule at the method level if it implements both MethodRule and TestRule. If a developer implements both of the interfaces, I feel confident that the developer expects the rule to be invoked via the MethodRule API for method-level callbacks and via the TestRule API for class-level callbacks. The fact that JUnit currently silently ignores the fact that a TestRule implements MethodRule as well is a bug in my opinion.

Make sense?

Cheers,

Sam

@kcooney
Copy link
Member

kcooney commented May 16, 2015

I think it's would be confusing and possibly error prone to have a rule be called via the TestRule API in one context and via the MetodRule API in another.

We might skip 4.13 and jump directly to 5.0, so now is a good time to think of a replacement API.

@sbrannen
Copy link
Member

If we don't want to allow TestRule to be combined with MethodRule in the same class and we still want to stick with the concept of rules, we could support something similar to the ComplexRule API suggested by @nealeu in nealeu@da6c5e5 (along with the proposed modification to Description here nealeu@81407fb).

Something like the following could work:

public interface ClassAndMethodRule {

  Statement applyAtClassLevel(Statement base, Description description);

  Statement applyAtMethodLevel(Statement base, Description description, Object testInstance);

  // or
  // Statement applyAtMethodLevel(Statement base, FrameworkMethod method, Object testInstance);

  // or
  // Statement applyAtMethodLevel(Statement base, Description description, FrameworkMethod method, Object testInstance);

}

Thoughts?

@jlink
Copy link
Contributor

jlink commented May 16, 2015

What about the idea of decorators I suggested in proposal 2 for JUnit Lambda: https://github.com/junit-team/junit-lambda/blob/master/src/test/java/org/junit/lambda/proposal02/examples/DecoratedTests.java

Johannes

Von meinem iPad gesendet

Am 16.05.2015 um 14:54 schrieb Sam Brannen [email protected]:

If we don't want to allow TestRule to be combined with MethodRule in the same class and we still want to stick with the concept of rules, we could support something similar to the ComplexRule API suggested by @nealeu in nealeu/junit@da6c5e5.

Something like the following could work:

public interface ClassAndMethodRule {

Statement applyAtClassLevel(Statement base, Description description);

Statement applyAtMethodLevel(Statement base, Description description, Object testInstance);

// or
// Statement applyAtMethodLevel(Statement base, FrameworkMethod method, Object testInstance);

// or
// Statement applyAtMethodLevel(Statement base, Description description, FrameworkMethod method, Object testInstance);

}
Thoughts?


Reply to this email directly or view it on GitHub.

@kcooney
Copy link
Member

kcooney commented May 16, 2015

@jlink I would.love to.use annotations to.define rules, but there are a few problems.with having a class-level annotation for applying rules. The order of class annotations are you nefined, and annotations would limit what data you could pass in to create the Rule

@kcooney
Copy link
Member

kcooney commented May 16, 2015

@sbrannen thanks for the proposal. We would need to define how you apply instances of the new rule type to a test, how you order them, and how they interact with TestRule and MethodRule.

Again I find it easier to discuss bigger design proposals in a Google Doc, but we can try to discuss here

@jlink
Copy link
Contributor

jlink commented May 16, 2015

2015-05-16 15:57 GMT+02:00 Kevin Cooney [email protected]:

@jlink https://github.com/jlink I would.love to.use annotations
to.define rules, but there are a few problems.with having a class-level
annotation for applying rules. The order of class annotations are you
nefined,

Is that a problem when we are dealing with a single type of annotation? We
can always collect all instances and have the order of evaluation being
determined by other criteria.

and annotations would limit what data you could pass in to create the Rule

Right. That's why decorator classes in my proposal can be injected into
tests and before/after methods to perform any necessary parameterization /
setup etc.

Johannes


Reply to this email directly or view it on GitHub
#793 (comment).

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

No branches or pull requests

5 participants