Junit5: Detect annotations on overridden super type methods

Created on 18 Jul 2017 · 16Comments · Source: junit-team/junit5

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

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.

Jupiter Platform declined discovery programming model

Source

sormuras

👍2

All 16 comments

FYI: I have updated the title and description.

sbrannen on 18 Jul 2017

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 on 18 Jul 2017

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");
    }

}

sbrannen on 18 Jul 2017

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");
    }

}

sormuras on 18 Jul 2017

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 on 18 Jul 2017

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

sbrannen on 18 Jul 2017

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".

marcphilipp on 18 Jul 2017

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 on 19 Jul 2017

Added additional links to the _Related Discussions_ section thanks to a few tips from @marcphilipp.

sbrannen on 19 Jul 2017

@marcphilipp are there any plans to implement it?

remal on 25 Jul 2020

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

marcphilipp on 25 Jul 2020

@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).

remal on 10 Aug 2020

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).