-
Notifications
You must be signed in to change notification settings - Fork 3.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
Enables Inheritance on Category.java by adding @Inherited #566
Conversation
I want to add that @inherited supports overwriting the Annotation as far as i know. The approach in #563 would have aggregated the classes. If at some point the implementation is switched that behavioural change should be in awareness. |
@@ -23,6 +16,10 @@ | |||
import org.junit.runners.Suite.SuiteClasses; | |||
import org.junit.runners.model.InitializationError; | |||
|
|||
import static org.junit.Assert.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JUnit style is to import individual methods. Sorry.
Good point about replacing superclasses. At this point, no one is expecting any behavior related to categories on superclasses, so we're not breaking anything, and if someone needed the aggregated behavior, we can talk about adding it back in. |
@gaffa |
Yes. I confused something. Never mind because I figured it out. Thanks anyway! |
This is only about class inheritance of Category annotation and not the mehods? |
http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/annotation/Inherited.html "Note that this meta-annotation type has no effect if the annotated type is used to annotate anything other than a class." @category(A.class) class Inheritor extends Ancestor{} with @inherited Inheritor would behave exactly as if it would be annotated with @category(A.class) itself. PS: I still cant get other the guy called Inherited on github. I am sorry but you should really change your nickname :D |
@dsaff everything fine now? |
Enables Inheritance on Category.java by adding @inherited
Thanks for the patch. Can you please also update the release notes at https://github.com/KentBeck/junit/wiki/4.12-release-notes? |
@gaffa, you can get artifacts at https://junit.ci.cloudbees.com/job/JUnit/. @marcphilipp, is Cloudbees automatically maintaining a snapshot repo? @gaffa, it's been a pretty productive run post-4.11, and I had promised to get 4.12 out pretty quickly. Looking at the things currently marked for 4.12, I don't see anything that couldn't wait another month. @marcphilipp, any thoughts about timing on 4.12? |
Done. Thanks a lot for accepting this patch. Really makes things a lot better for us (and hopefully others) ;) |
Could a release of 4.12 be made? I'd really like to be able to use this feature, its been merged over a year ago! |
@jbcpollak You can follow #868 |
Thanks @kcooney, I will do that. If anyone is looking for a fix to just this problem, I have created a forked maintenance branch with this change, Maven support, and a few JDK 8 patches here: https://github.com/jbcpollak/junit/tree/r4.11.x It should give you 4.11 plus this fix in an easily compilable form. |
I have been trying to test the update done for @category with respect to @inherited meta-annotations with Junit 4.12 Beta 2 version and I get the following exception, Kindly advice if I am doing something wrong over here. java.lang.NullPointerException |
@ayazlakdawala you'll get more help on the mailing list |
I know this is an old post but curious if @ayazlakdawala figured out what is causing this and a fix? I am seeing the same error. |
@bemnet4u could you provide a minimal, complete and verifiable example (see https://stackoverflow.com/help/mcve) ? Also since 4.12 has already been released, you might get more eyes on your problem if you post to http://stackoverflow.com |
As discussed in https://github.com/KentBeck/junit/pull/563 @dsaff suggested to try and add @inherited. This pull request introduces that behaviour.