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

Refactoring Suite, Parameterized and class runners to allow reuse in custom Runners #1348

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 8 additions & 12 deletions src/main/java/org/junit/runners/ParentRunner.java
Expand Up @@ -189,9 +189,9 @@ private void validateClassRules(List<Throwable> errors) {
protected Statement classBlock(final RunNotifier notifier) {
Statement statement = childrenInvoker(notifier);
if (!areAllChildrenIgnored()) {
statement = withBeforeClasses(statement, testClass);
statement = withAfterClasses(statement, testClass);
statement = withClassRules(statement, testClass, getDescription());
statement = withBeforeClasses(statement);
statement = withAfterClasses(statement);
statement = withClassRules(statement, getDescription());
}
return statement;
}
Expand All @@ -209,9 +209,8 @@ private boolean areAllChildrenIgnored() {
* Returns a {@link Statement}: run all non-overridden {@code @BeforeClass} methods on this class
* and superclasses before executing {@code statement}; if any throws an
* Exception, stop execution and pass the exception on.
* @param testClass
*/
public static Statement withBeforeClasses(Statement statement, TestClass testClass) {
protected Statement withBeforeClasses(Statement statement) {
Copy link
Member

@kcooney kcooney Jul 21, 2016

Choose a reason for hiding this comment

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

Curious why you want to call these directly vs. calling classBlock(). If you cannot use classBlock() then perhaps instead of making these three protected we should extract one protected method that calls all three so 1) your runner isn't hard-coding the order and 2) we can add more calls in the future (ex withClassFixtures()). Perhaps call the new method withClassStatements()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like your proposal to introduce withClassStatements(). I also had something like this in mind before, but didn't brought it to an end. I will incorporate it now.

I'd like to propose to leave withBeforeClasses(...) (and the others) protected anyway. But the documentation should encourage to use withClassStatements().

I can't call ParentRunner.classBlock(...) in my custom Runner because between invoking the children and applying the class statements, I'd like to apply @TestRules. ParentRunner doesn't support these.

        Statement statement = childrenInvoker(notifier);

        List<TestRule> testRules = BlockJUnit4ClassRunner.getTestRules(testInstance, getTestClass());
        statement = BlockJUnit4ClassRunner.withTestRules(testRules, getDescription(), statement);

        statement = withClassStatements(statement);
        return statement;

Copy link
Member

Choose a reason for hiding this comment

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

@PeterWippermann I don't understand what you mean by "I can't call ParentRunner.classBlock(...) in my custom Runner because between invoking the children and applying the class statements, I'd like to apply @testrules". The method withClassRules() applies all of the rules annotated with @ClassRule. You shouldn't invoke rules annotated with TestRule in ParentRunner because there is no instance of the class.

List<FrameworkMethod> befores = testClass
.getAnnotatedMethods(BeforeClass.class);
return befores.isEmpty() ? statement :
Expand All @@ -224,9 +223,8 @@ public static Statement withBeforeClasses(Statement statement, TestClass testCla
* always executed: exceptions thrown by previous steps are combined, if
* necessary, with exceptions from AfterClass methods into a
* {@link org.junit.runners.model.MultipleFailureException}.
* @param testClass
*/
public static Statement withAfterClasses(Statement statement, TestClass testClass) {
protected Statement withAfterClasses(Statement statement) {
List<FrameworkMethod> afters = testClass
.getAnnotatedMethods(AfterClass.class);
return afters.isEmpty() ? statement :
Expand All @@ -239,23 +237,21 @@ public static Statement withAfterClasses(Statement statement, TestClass testClas
* annotated with {@link ClassRule}.
*
* @param statement the base statement
* @param testClass
* @param description the description to pass to the {@link Rule}s
* @return a RunRules statement if any class-level {@link Rule}s are
* found, or the base statement
*/
public static Statement withClassRules(Statement statement, TestClass testClass, Description description) {
List<TestRule> classRules = classRules(testClass);
protected Statement withClassRules(Statement statement, Description description) {
List<TestRule> classRules = classRules();
return classRules.isEmpty() ? statement :
new RunRules(statement, classRules, description);
}

/**
* @param testClass
* @return the {@code ClassRule}s that can transform the block that runs
* each method in the tested class.
*/
protected static List<TestRule> classRules(TestClass testClass) {
protected List<TestRule> classRules() {
List<TestRule> result = testClass.getAnnotatedMethodValues(null, ClassRule.class, TestRule.class);
result.addAll(testClass.getAnnotatedFieldValues(null, ClassRule.class, TestRule.class));
return result;
Expand Down