-
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
Support for hierarchical context hierarchies #817
Conversation
currentTestClass = getEnclosingClassForNonStaticMemberClass(currentTestClass)) { | ||
RunWith annotation = currentTestClass.getAnnotation(RunWith.class); | ||
if (annotation != null) { | ||
return buildRunner(annotation.value(), currentTestClass); |
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.
Are you sure you don't want to pass testClass
instead of currentTestClass
here? Passing the outermost class with @RunWith
doesn't appear to solve #816
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.
Thanks for this. Of course, testClass needs to be passed with buildRunner. I changed it and updated the pull request.
Could you add a test? |
Yes, of course. I couldn't find any tests for this class so I also added tests for the default case. Hope that's fine. Let me know if it needs any changes on those. |
LGTM @dsaff any objections? |
With the last commit, I didn't change any more line of production code, just added two more test cases that show that annotation on inner member classes are also handled while going upwards to the top level class. Still, this has no effect on the JUnit default runner, because it does not allow inner member (non-static) classes. |
I'm OK with the change. But I do think there may need to be some documentation that devs can use in terms of the expected search path, especially about the fact that, if I'm reading it right, a superclass of the inner class will be used before the outer class. (This is understandable behavior, but should be doc'ed.) |
Sounds Good. I will provide docs soon. Prob. at the weekend. |
I added the documentation. |
LGTM. I'm assigning this to @dsaff since he requested the documentation. |
@dsaff RunWith is inherited, so inner will run with R2 |
@kcooney, right. I think this is worth calling out explicitly in the documentation, because it's consistent behavior, but also potentially surprising. |
I'm sorry it took so long to reply. We had tremendous troubles in the last 5 weeks on work so I had hardly time for anything else. I modified the documentation. Thanks for reviewing. |
Looks good to me, thanks! |
…ion will be resolved along the hierarchy of non-static member classes.
Sure, makes totally sence. I squashed all commits into one. Thanks. |
Support for hierarchical context hierarchies
Thanks. |
@bechte no problem. Glad to help. Could you please update the release notes? |
Sure, done. |
This is a pull-request for issue #816.
#816: Support for hierarchical context hierarchies - RunWith annotation will be resolved along the hierarchy of non-static member classes.
Discussions on this topic should happen in #816. I provided this pull-request mostly to show the changes more graphically.