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

Added validtion that @ClassRule should only be implementation of TestRule #1020

Merged
merged 1 commit into from
Nov 5, 2014

Conversation

npathai
Copy link
Contributor

@npathai npathai commented Nov 2, 2014

Fixes #1019

  • Added validation that @ClassRule must be an implementation of TestRule

@npathai
Copy link
Contributor Author

npathai commented Nov 2, 2014

@kcooney Have fixed the validation problem for @ClassRule as we discussed. Please review the changes.

@@ -164,6 +164,7 @@ private static boolean isMethodRule(FrameworkMember<?> member) {
* Requires the validated member to be non-static
*/
private static final class MemberMustBeNonStaticOrAlsoClassRule implements RuleValidator {
@Override
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still use JDK5, which doesn't use @Override when implementing methods from an interface

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kcooney Oh sorry for that. Have reverted that and squashed the changes.

@marcphilipp
Copy link
Member

What did happen when using @ClassRule with MethodRule before this change?

@npathai
Copy link
Contributor Author

npathai commented Nov 5, 2014

@marcphilipp Before this change there was no validation error, but the rule did not get call during execution same problem with issue #589.
Then after discussion with @kcooney we added validation that @ClassRule will not allow MethodRule

kcooney added a commit that referenced this pull request Nov 5, 2014
Added validtion that @ClassRule should only be implementation of TestRule
@kcooney kcooney merged commit b697400 into junit-team:master Nov 5, 2014
@kcooney
Copy link
Member

kcooney commented Nov 5, 2014

Thanks.

@marcphilipp do we need to update the release notes before building a new RC? I can try to find time to update them this week

@marcphilipp
Copy link
Member

@kcooney Yes, we should. Thanks in advance! :-)

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

Successfully merging this pull request may close these issues.

Add validation check that @ClassRule can only return an implementation of TestRule
4 participants