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

JUnit should process annoations in package-info.java #670

Open
brianegge opened this issue May 2, 2013 · 22 comments
Open

JUnit should process annoations in package-info.java #670

brianegge opened this issue May 2, 2013 · 22 comments

Comments

@brianegge
Copy link

It would be useful to be able to add package level annotations to package-info.java. For example adding @org.junit.Ignore to the package should ignore all tests in that package.

@dsaff
Copy link
Member

dsaff commented May 2, 2013

FYI, I think we looked into this years ago, and found that package-level annotations were not available at runtime. If you find information that this is not true in later versions of Java, please let me know.

@brianegge
Copy link
Author

I see a reference to package-level annoations here:
http://www.onjava.com/pub/a/onjava/2004/04/21/declarative.html?page=3

Also, I can see the annotation in the class file:

$ strings package-info.class
SourceFile
package-info.java
RuntimeVisibleAnnotations
Lorg/junit/Ignore;

@flpa
Copy link
Contributor

flpa commented Oct 25, 2013

Would this be a feature worth looking into?
Checking annotations on package-level seems pretty straightforward:

TheTestClass.class.getPackage().isAnnotationPresent(Ignore.class);

According to javadoc, this method exists since JDK 1.5.

@dsaff
Copy link
Member

dsaff commented Oct 26, 2013

Hmm. I wonder what convinced us this wouldn't work?

Assuming we were just crazy, we'd still have to proceed with caution:

  1. a developer who didn't pay careful attention to release notes might be surprised to see her test classes start changing behavior without any local changes.

  2. We'd have to think about how package annotations would match up with superclass annotations.

I think I'd be open to something like an opt-in @packaged annotation which meant "package annotations apply to this class".

@brianegge
Copy link
Author

I think the package info class is so little used that a change would not likely have much of an effect. I seriously doubt too many other people have tried using package level annotations for JUnit, for if they had, someone in the past few year would probably have posted "Me too" on this request. I do think there are a number of use cases for package level annotations, and if supported, it would see some adoption.

@flpa
Copy link
Contributor

flpa commented Oct 26, 2013

@brianegge I think it's not about the immediate effect that this feature might have but about the additional layer of complexity it adds when trying to figure out why a particular test has been ignored, for example. Probably that's a matter of inter-team communication, though... At least I'd expect a colleague starting to use package-level annotations or changing such settings in packages that might affect me to drop me a line.

I guess this feature could both be quite useful, when used with care, and annoying, when you're desperately looking through class and superclass commit histories to find out why your test is behaving differently now.

Another thing that came to my mind while thinking about this is inheritance to sub-packages. I guess as soon as package-level annotations are supported, someone might ask for inheritance as well...

@dsaff, regarding 2):
I would probably expect package-level annotations to overrule superclass annotations - but that's just my first thought.
I'll also think about the opt-in approach.

@kcooney
Copy link
Member

kcooney commented Jan 26, 2014

(going through old bugs)

Before proceeding with this, it would be useful to know what annotations other than @Ignore make sense at the package level. This would help, for example, in deciding whether package-level annotations would overrule superclass annotations.

@brianegge
Copy link
Author

Probably just the ignore annotation. Otherwise, being able to set a default timeout for all classes in a package would be useful, but I don't think one can do that at the class level today.

@marcphilipp
Copy link
Member

As @dsaff, I'm in favor of an opt-in annotation on test classes, so there's at least a small hint there. I think @InheritPackageAnnotations would be a better name.

@kcooney
Copy link
Member

kcooney commented Jan 27, 2014

I'm not so sure about the opt-in annotation idea. If we required an opt-in annotation, and @Ignore is the only use case, then I don't think people would use this feature. When developers want to disable all classes in a package, I think would end up just annotating the classes with @Ignore rather than annotate the package-info with @Ignore and updating every class to to have the opt-in annotation.

@dsaff
Copy link
Member

dsaff commented Jan 29, 2014

@kcooney, the idea was that a team would agree to add @InheritPackageAnnotations to all the classes in their project, and, having thus opted-in in advance, they could use package annotations later to @ignore out a package.

@kcooney
Copy link
Member

kcooney commented Jan 29, 2014

@dsaff I understand the idea. I just doubt many teams would add @InheritPackageAnnotations to all the classes in their project, since it looks like noise. I hear a lot of people wanting to create base classes for their tests because having one @Rule is "too much noise".

In any case, is it all that common for a team to want to wire off all current and future tests in a package?

@marcphilipp
Copy link
Member

As an alternative to @InheritPackageAnnotations we could check for a
certain system property, e.g.
"org.junit.InheritPackageAnnotations=true". Thoughts?

@brianegge
Copy link
Author

Package annotations are used by Javadoc, JAXB, Struts, Hibernate, javac and other popular Java tools. In none of these do you need to opt-in in order to use the features. One can mark an entire package as @depreciated. If someone was trying to figure out why they were getting a depreciation warning, it might take them a few minutes to find it in the package-info. Package annotations have been part of Java for nine years now, so while they aren't used terribly often, they aren't entirely obscure either.

On Jan 29, 2014, at 12:42 AM, Marc Philipp notifications@github.com wrote:

As an alternative to @InheritPackageAnnotations we could check for a
certain system property, e.g.
"org.junit.InheritPackageAnnotations=true". Thoughts?

Reply to this email directly or view it on GitHub.

@dsaff
Copy link
Member

dsaff commented Jan 29, 2014

@kcooney, I think we might be in violent agreement. I agree that the feature feels unlikely to be frequently used in practice, and even more so if it requires per-class opt-in. But I really don't like the idea of supporting it at the core level without opt-in, so the feature in general would have to be sold better.

@marcphilipp, you raise an interesting point. We already have a command-line flag for filters, which could be used to simply not run a given subset of packages.

@brianegge, I've tried hard to hold a line to limit the number of places that could affect the execution of a test class, because I believe that test code ranks quite high on the kinds of code where one person is often called upon to quickly understand the execution of code written far away by someone else. In general, my feeling has been that if a feature is not useful enough to merit adding a single line to the Java classes it would affect, then the potential confusion cost is higher than the feature's benefit.

@adrianosimoes
Copy link

Is this still a valid issue?

@kcooney
Copy link
Member

kcooney commented Feb 28, 2017

@adrianosimoes I don't think the maintainers are thrilled with the idea of processing package-level annotations. I think we should leave it open in case someone has a use case other than @Ignore where it would be useful to process package-level annotations. If @Ignore is the only supported feature, then it doesn't seem very compelling IMHO.

@kcooney
Copy link
Member

kcooney commented Feb 28, 2017

One use case just came up (@RunWith). I could imagine wanting to specify @Category at a package level as well. For @RunWith it is clear what happens when the package has the annotation and some classes in the package are also annotated. For @Category the desired behavior is less clear.

@brianegge
Copy link
Author

Supporting package level annotations would be straight forward to do. However, the number of people actually using them would likely be between zero and one. It appears there is little demand for this feature, so it's probably best to close this request.

@kcooney kcooney reopened this Mar 5, 2017
@kcooney
Copy link
Member

kcooney commented Mar 5, 2017

There is discussion about possibly using them in #1423

@cesar1000
Copy link

cesar1000 commented Mar 5, 2017

@brianegge I disagree. A package-level @Ignore annotation as you suggested in your original request can be really useful, as there's no convenient way to disable a set of test un current JUnit. So is a @RunWith annotation for defining a default runner for a set of tests.

As to whether this would see adoption: maybe not if annotations need to be added manually, but IDEs could use this feature and expose it through a high level interface.

@kcooney
Copy link
Member

kcooney commented Apr 5, 2017

From a discussion in #1423

it appears that Java will only initialize Package
objects when first loading a class from the package.
This is ok if annotating the package containing the
tests, but won't work with parent packages (unless
adding placeholder classes to these packages and
loading them explicitly, but that's a bad hack).

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

No branches or pull requests

7 participants