Detect annotations on overridden super type methods

[sormuras]

Overview

In JUnit 4, if @Test is declared on an abstract method in a superclass, then any concrete implementation of the method is not required to redeclare the @Test annotation. However, JUnit Jupiter currently does not support implicitly inherited annotations on overridden methods.

This is an unintentional difference between JUnit 4 and JUnit Jupiter.

Related Discussions

• https://stackoverflow.com/questions/45162254/why-do-junit-5-tests-not-inherit-test-annotation-from-abstract-classes
• @Ignore not taking effect when used on an overriden test method junit4#695
• bug in ignore an overriding method junit4#468
• stefanbirkner/junit@2607752

Deliverables

• Tweak internal annotation utilities to find implicitly inherited annotations on overridden methods.
• Tweak internal annotation utilities to find implicitly inherited repeated annotations on overridden methods.

[sbrannen]

FYI: I have updated the title and description.

[sbrannen]

Works in JUnit 4

import org.junit.Before;
import org.junit.Test;

public abstract class AbstractTests {

	@Before
	public abstract void before();

	@Test
	public abstract void test();

}

public class FooTests extends AbstractTests {

	@Override
	// @Before
	public void before() {
		System.err.println("before");
	}

	@Override
	// @Test
	public void test() {
		System.err.println("test");
	}

}

[sbrannen]

Does not work in JUnit Jupiter 5.0 M5

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public abstract class AbstractTests {

	@BeforeEach
	public abstract void before();

	@Test
	public abstract void test();

}

public class FooTests extends AbstractTests {

	@Override
	// @BeforeEach
	public void before() {
		System.err.println("before");
	}

	@Override
	// @Test
	public void test() {
		System.err.println("test");
	}

}

[sormuras]

Does not work in JUnit Jupiter 5.0 M5

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

interface InterfaceTests {

	@BeforeEach
	void before();

	@Test
	void test();

}

public class BarTests implements InterfaceTests {

	@Override
	// @BeforeEach
	public void before() {
		System.err.println("before");
	}

	@Override
	// @Test
	public void test() {
		System.err.println("test");
	}

}

[sbrannen]

My preliminary analysis of this functionality in JUnit 4 leads me to believe that the behavior is perhaps accidental, especially in light of explicit documentation within JUnit 4's BlockJUnit4ClassRunner such as the following.

    /**
     * Returns the methods that run tests. Default implementation returns all
     * methods annotated with {@code @Test} on this class and superclasses that
     * are not overridden.
     */
    protected List<FrameworkMethod> computeTestMethods() {
        return getTestClass().getAnnotatedMethods(Test.class);
    }

A round of debugging led me to the fact that the following is actually what JUnit 4 tracks as the "test method" in such cases.

[public void org.junit.platform.AbstractTests.test()]

In summary: it appears that JUnit 4 in fact does not have any special reflective code in place to support "implicit inheritance" of annotations of methods. Rather, it only works coincidentally since JUnit 4 erroneously (?) tracks annotated abstract methods. If a concrete method (that overrides an annotated abstract method) is not annotated, it simply gets ignored since it is in fact "not a test method". Then the execution of the abstract method just happens to work by chance (?) since the methods have the same name.

In light of these findings, I am removing the "bug" label, adding the "team discussion" label, and moving this issue to 5.0 RC1.

[sbrannen]

Added Platform label since such a change will (adversely?) affect any lookups for annotations on methods across all use cases.

[marcphilipp]

We also need to consider cases where the subclass adds an annotation, e.g.

public class BaseTests {
	@Test
	public void test() {/*...*/}
}

public class SubTests extends BaseTests {
	@Override
	@Disabled
	public void test() {/*...*/}
}

This would be equivalent to:

public class SubTests {
	@Test
	@Disabled
	public void test() {/*...*/}
}

This becomes even more interesting, if the subclass declares annotations that the base class declares as well.

public class BaseTests {
	@Test
	@DisplayName("base")
	@Tag("base")
	public void test() {/*...*/}
}

public class SubTests extends BaseTests {
	@Override
	@DisplayName("sub")
	@Tag("sub")
	public void test() {/*...*/}
}

In this case the display name should be "sub" but it should/could have both tags, so it would be equivalent to:

public class SubTests {
	@Test
	@DisplayName("sub")
	@Tag("base")
	@Tag("sub")
	public void test() {/*...*/}
}

To override a repeatable annotation completely, you could use its container annotation:

public class SubTests extends BaseTests {
	@Override
	@DisplayName("sub")
	@Tags({@Tag("sub")})
	public void test() {/*...*/}
}

which would then yield only the tag "sub".

[sbrannen]

We also need to consider cases where the subclass adds an annotation, e.g.

Yep, that opens up a whole new can of worms.

Not so sure that I'm a fan of this approach unless we keep it super simple.

[sbrannen]

Added additional links to the Related Discussions section thanks to a few tips from @marcphilipp.

[remal]

@marcphilipp are there any plans to implement it?

[marcphilipp]

@remal No, it's currently not planned to be implemented. What's your concrete use case?

[remal]

@marcphilipp the use case is quite simple: I have several implementations of an interface and I'd like to use a base abstract test class (with abstract methods annotated with @Test) as a contract (concrete tests should test at least listed scenarios).

[rpost]

We also need to consider cases where the subclass adds an annotation, e.g.

Yep, that opens up a whole new can of worms.

Not so sure that I'm a fan of this approach unless we keep it super simple.

Imo silently skipping overridden annotated method is the worst solution. It should be either checked for such case and rejected with clear error message or supported as in junit4 (the latter is my personal preference: @BeforeEach methods are inherited from superclasses as long as they are not overridden. - this behaviour is very surprising).

[marcphilipp]

Team decision: Keep the current behavior to avoid a breaking change and considerable added complexity.

[mjustin]

Is this behavior documented somewhere (and should it be)?

[marcphilipp]

Yes, it's documented on all method-level annotations, e.g.

@BeforeEach methods are inherited from superclasses as long as they are not overridden.

[sbrannen]

Closely related to:

• Align method and field search algorithms with Java semantics #3553