Revert changes to method search algorithm

[rdehuyss]

Hi there, I'm Ronald, the creator of JobRunr. When upgrading my dependencies to 5.10.1 from 5.10.0, I noticed a breaking change which was not declared in the release notes.

When using tests and inheritance (which I do for testing JobRunr against different DB's), I noticed that @Disabled was not taking into account anymore in v5.10.1 whereas that was the case in v5.10.0.

Steps to reproduce

Please see https://github.com/rdehuyss/junit-issue-disabled

If you run the test JUnitTest3, you notice that it fails. If you downgrade to 5.10.0, all is well again.

Context

• Used versions (Jupiter/Vintage/Platform): JUnit Jupiter
• Build Tool/IDE: both maven and gradle, Intellij

[rdehuyss]

Never mind, it has apparently been declared in 5.10.1... Three questions:

• is there a place where I can find the reasoning behind this?
• and would this change (if following semver and breaking) not validate a new major version change?
• as I used this feature a lot, how to solve this (instead of copying the @Disabled over and over)?

Thanks!

[rdehuyss]

Reopening as this really breaks a lot for me... Would you accept a PR for an annotation @DisabledAndInherited?

[sbrannen]

Never mind, it has apparently been declared in 5.10.1... Three questions:

What was declared in the release notes for 5.10.1 is that the documentation for the existing behavior was improved.

As far as I know, we did not intentionally change the behavior for @Disabled with regard to inheritance.

Are you saying your sample project demonstrates a change in behavior for @Disabled between 5.10.0 and 5.10.1?

[rdehuyss]

Hi @sbrannen! Thanks for getting back to me so quickly!

Are you saying your sample project demonstrates a change in behavior for @disabled between 5.10.0 and 5.10.1?

Yes

With 5.10.0, the JUnitTest3 is green, upgrading to 5.10.1 changes it to red.

[rdehuyss]

Relates to #3462

[rdehuyss]

Hi @sbrannen - where you able to verify the example project showing the change with @Disabled?

[marcphilipp]

Thanks for the reproducer! I can confirm that the behavior has indeed changed:

public abstract class AbstractJUnitTest {

    @Test
    protected void someTestMethod() {
        System.out.println("This should not run as disabled by AnstractJUnitTest2");
        throw new RuntimeException("Should not be thrown");
    }

    @Test
    protected void anotherTestMethod() {
        System.out.println("This should run");
    }
}

public abstract class AbstractJUnitTest2 extends AbstractJUnitTest {

    @Override
    @Disabled
    protected void someTestMethod() {
        super.someTestMethod();
    }
}

public class JUnitTest3 extends AbstractJUnitTest2{
}

In 5.10.0, JUnit3Test.someTestMethod is skipped, whereas 5.10.1 executes it. I think this is caused by 1752389.

[sbrannen]

If you rewrite AbstractJUnitTest2 as follows, someTestMethod() is properly disabled.

public abstract class AbstractJUnitTest2 extends AbstractJUnitTest {

	@Disabled
	@Test
	@Override
	protected void someTestMethod() {
		super.someTestMethod();
	}

}

If I recall correctly, we have never claimed in JUnit Jupiter that a @Test method can be overridden and still be considered a test method if the override does not redeclare @Test.

So, I consider that the expected behavior (or at lest the behavior we originally intended).

In light of that, I actually consider it a bug that the scenario without @Test worked before.