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

chore(ARQ-2172): merged the same runner and rule code to avoid code duplication #155

Merged
merged 2 commits into from
Jan 23, 2018

Conversation

MatousJobanek
Copy link
Contributor

Short description of what this resolves:

The code of the TestRunnerAdaptor management is same in both JUnit runner and JUnit class rule. The same can be also applied to the logic that invokes the test method.
I've moved the code to separated classes that are used by both implementations. As a benefit:

  • we don't duplicate the code
  • it is guaranteed that when the code is changed, it will affect both implementations.

When I was doing the refactoring, I've noticed that in the rule implementations there was missing enrichment of the other rules used in the test class:

adaptor.fireCustomLifecycle(new RulesEnrichment(target, testClass, testMethod, LifecycleMethodExecutor.NO_OP));

So I added it there.

… rules enrichment in Arquillian Rule implementation
Copy link
Contributor

@dipak-pawar dipak-pawar left a comment

Choose a reason for hiding this comment

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

@MatousJobanek Nice Improvement. Overall looks good except one comment I have.

}
}

protected void shutdown(TestRunnerAdaptor adaptor) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be package-private

Copy link
Member

@bartoszmajsak bartoszmajsak left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @MatousJobanek. Can you create JIRA ticket so we can link this PR (and also have clean release notes)?

@bartoszmajsak bartoszmajsak changed the title chore: merged the same runner and rule code to avoid code duplication chore(ARQ-2172): merged the same runner and rule code to avoid code duplication Jan 23, 2018
@bartoszmajsak bartoszmajsak merged commit ac923bc into arquillian:master Jan 23, 2018
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

Successfully merging this pull request may close these issues.

3 participants