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

Feature idea: Annotation Rules #1074

Open
BenRomberg opened this issue Jan 22, 2015 · 12 comments
Open

Feature idea: Annotation Rules #1074

BenRomberg opened this issue Jan 22, 2015 · 12 comments
Labels

Comments

@BenRomberg
Copy link

I've recently run into the problem that I wanted to use an annotation on certain test-methods, but didn't require the explicit rule as a field.

Considering an example, let's say there's a DatabaseUnavailable annotation and a corresponding DatabaseUnavailableRule setting up a defect database connection in case the annotation is present.

@Rule
public DatabaseUnavailableRule databaseUnavailableRule = new DatabaseUnavailableRule();

@Test
@DatabaseUnavailable
public void databaseUnavailable_ThrowsException() {
  // ...
}

// more tests not requiring @DatabaseUnavailable

The rule is completely unnecessary if the test doesn't need to reference the rule's field. We could instead let the annotation reference the rule through a class reference in the annotation. This would change our example to:

// no Rule declaration necessary

@Test
@DatabaseUnavailable
public void databaseUnavailable_ThrowsException() {
  // ...
}

With the DatabaseUnavailable annotation referencing DatabaseUnavailableRule within a meta annotation, e.g. looking like:

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
@AnnotationRule(DatabaseUnavailableRule.class)
public @interface DatabaseUnavailable {

}

The only limitation I can think of is that the DatabaseUnavailableRule would have to have a constructor with no parameters so we can instantiate it with reflection. It would only be instantiated for tests annotated with the annotation.

What do you think? I'd be happy to submit a pull-request if it sounds viable.

Could also think of this working as a class annotation for all test-methods.

@kcooney
Copy link
Member

kcooney commented Jan 22, 2015

Related feature requests: #200 and #766

@kcooney
Copy link
Member

kcooney commented Jan 25, 2015

Note that in this particular example, you could have DatabaseUnavailableRule only apply to methods annotated with @DatabaseUnavailable by having the rule look at the annotations on the Description object.

If you prefer not to have a field, you can apply @Rule to a method that returns a TestRule

@BenRomberg
Copy link
Author

Right, that's how my rule currently works - looking at the Description if the method has the Annotation.

I just want to avoid the hassle of having the Rule-annotated field/method in my test-class if it's not necessary. We have quite a few different test-"modifiers" like that in our test-tools. Requiring a Rule field/method for each of those bloats up the test-classes unnecessarily.

Also, it would reduce the necessary code within the rule leaving the above mentioned annotation-check out.

@kcooney
Copy link
Member

kcooney commented Jan 25, 2015

Whether you use an annotation or a field, it still takes up one or two lines in the class (depending on your coding style), so personally I've never quite understood the concern about bloat. YMMV of course :-)

If we do decide to allow annotations to declare rules, we need to decide on how to determine what order to apply the rules (see lengthy discussion about this in #766). The ordering problem has been the big blocker for other proposals similar to this one.

You do have a good point about the possibility that the rule implementation could be simpler, but that's only the case if the definition of the annotation on the test method is something you control. If you wanted to apply a third-party annotation to a method and have a rule add behavior to methods with that annotation, you'll need to look at the Description passed to TestRule.apply().

@BenRomberg
Copy link
Author

The purpose of this ticket is not to allow annotations to declare rules. It is also not about annotations vs. fields.

It is about selectively activating a rule for some (not all) test-methods within a class. I've hoped to make that clear in the example provided in the first post. You need the rule-declaration (field/method as of now) and the annotation or a line of code activating the rule within the method.

In my opinion, if it is not needed the rule-declaration can be made obsolete with the proposal above.

I've looked at #766 where you have provided a very similar solution (the SpringLoaded example) on a class level.

I don't understand your point about using third-party annotations on test-methods. Why would I use a third-party annotation for my own rule and not just create my own?

@kcooney
Copy link
Member

kcooney commented Jan 26, 2015

I understand what you were originally proposing.

If we allow users to apply rules to classes (or methods) by adding a custom annotation, then we need to decide how to handle the case where there are multiple annotations (the ordering of annotations returned by the reflection APIs is undefined). We will also have to decide what to do if there are annotations on the base class, or on the interfaces that the class implements.

@BenRomberg
Copy link
Author

Yes I've read about that problem in #766, and don't have any idea how that could be solved with a clean API. For my use cases however, it's not an issue, as almost all of those rule are independent of each other. A case could be made that if you need to control rule ordering, you have to use the old-fashioned way declaring them in a RuleChain.

I'll prolly just end up implementing an own generic AnnotationAwareRule that respects method annotations as described above. At least that means only 1 rule declaration for each test class that wants to use this mechanism.

@BenRomberg
Copy link
Author

Just implemented this, works very nicely. Maybe we'll release it as a JUnit extension sometime in the future. Of course it would be a lot better without requiring the AnnotationAwareRule in every test-class, but AFAIK there's currently no way to avoid some kind of adjustment in every test-class.

If anyone is interested, I've made a small project with the code.

@biswajit86
Copy link

Based on the documentation for Rules , I do not find a way to define a rule that can be used explicitly at a function level via annotation.

How do I do a +1 for this issue ( I am new to github issues)

@kcooney
Copy link
Member

kcooney commented May 10, 2015

@biswajit86 see #1074 (comment)

@Tibor17
Copy link
Contributor

Tibor17 commented Nov 19, 2015

@BenRomberg
Multiple Rules and their order on the top of test method should not be any problem.
Just keep requiring this feature. It makes sense!
The Java EE has elegant paradigms.
If you specify several @AnnotationRule stereotypes then they are picked up randomly
@Rule1 @Rule2 @Test public void test() {
but if you aggregate them
@Rules({Rule1.class, Rule2.class}) @Test public void test() { their order is guaranteed.
Just recall JEE interceptors http://docs.oracle.com/javaee/6/tutorial/doc/gkedm.html

@BenRomberg
Copy link
Author

@Tibor17 thanks for the feedback. It looks like the JUnit Lambda project is going into this direction, at least that's how I understand the Prototype documentation. Let's wait and see what they come up with :-)

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