Junit5: TestInstance.Lifecycle on enclosing class not inherited by @Nested test class

[seanf]

JUnit version:
master branch, commit 16ac9ac4de (git describe: r5.3.0-RC1-47-g16ac9ac4de).

Expected behaviour:

• Configured Lifecycle.PER_CLASS is inherited in Nested test class from the outer class.
• @Nested class cannot override enclosing class's Lifecycle
• Ref: https://github.com/junit-team/junit5/issues/1566#issuecomment-416966563

Actual behaviour:

• @Nested test class has Lifecycle.PER_METHOD
• @Nested test class can override enclosing class's Lifecycle

Test case:

import java.util.Optional;

import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.extension.BeforeEachCallback;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.TestInstanceFactory;
import org.junit.jupiter.api.extension.TestInstanceFactoryContext;
import org.junit.jupiter.api.extension.TestInstantiationException;

import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
import static org.junit.platform.commons.util.ReflectionUtils.newInstance;

/**
 * @author Sean Flanigan <a href="mailto:[email protected]">[email protected]</a>
 */

@ExtendWith(BeforeEachLogger.class)
// if you enable this, it show that InnerTestCase really is constructed twice:
//@ExtendWith(CustomTestInstanceFactory.class)
@TestInstance(PER_CLASS)
class OuterTestCase {

    @Test
    void outerTest() {
        System.out.println("executing outerTest");
    }

    @Nested
    // Unless you specify PER_CLASS again, the InnerTestCase will be
    // constructed more than once, despite the PER_CLASS on OuterTestCase
//    @TestInstance(PER_CLASS)
    class InnerTestCase {

        @Test
        void innerTest1() {
            System.out.println("executing innerTest1");
        }

        @Nested
        class InnerInnerTestCase {

            @Test
            void innerTest2() {
                System.out.println("executing innerTest2");
            }
        }
    }
}

class BeforeEachLogger implements BeforeEachCallback {
    @Override
    public void beforeEach(ExtensionContext context) throws Exception {
        System.out.println("beforeEach() called for: " + context.getRequiredTestMethod());
        // It APPEARS that getTestInstanceLifecycle is always present in beforeEach
        // but this seems to be undocumented.
        System.out.println("beforeEach() lifecycle: " + context.getTestInstanceLifecycle().get());
    }
}

class CustomTestInstanceFactory implements TestInstanceFactory {
    @Override
    public Object createTestInstance(
            TestInstanceFactoryContext factoryContext,
            ExtensionContext extensionContext)
            throws TestInstantiationException {
        try {
            Optional<Object> outerInstance = factoryContext.getOuterInstance();
            Class<?> testClass = factoryContext.getTestClass();
            if (outerInstance.isPresent()) {
                System.out.println("createTestInstance() called for inner class: " + testClass.getSimpleName());
                return newInstance(testClass, outerInstance.get());
            }
            else {
                System.out.println("createTestInstance() called for outer class: " + testClass.getSimpleName());
                return newInstance(testClass);
            }
        }
        catch (Exception e) {
            throw new TestInstantiationException(e.getMessage(), e);
        }
    }
}

[seanf]

Is the Optional ExtensionContext.getTestInstanceLifecycle() guaranteed to be "present" in beforeEach? Is this documented anywhere?

EDIT: The same question goes for all the callbacks: BeforeEachCallback, AfterEachCallback, BeforeAllCallback, AfterAllCallback, BeforeTestExecutionCallback, AfterTestExecutionCallback, TestInstancePostProcessor. From my experiments, ExtensionContext.getTestInstanceLifecycle() is present for each of these callbacks (even when using a default Lifecycle), but none of the Javadocs say so.

[sbrannen]

As of this moment in time, ExtensionContext.getTestInstanceLifecycle() should not be _empty_ except in the root ExtensionContext.

If we were to introduce additional nodes in the execution hierarchy (e.g., for packages), ExtensionContext.getTestInstanceLifecycle() would almost certainly be _empty_ in those cases as well.

[sbrannen]

Is this documented anywhere?

No, I don't believe so.

[seanf]

Thanks @sbrannen. So for the existing callbacks I listed, is it safe to assume that the Lifecycle will always be present?

[sbrannen]

So for the existing callbacks I listed, is it safe to assume that the Lifecycle will always be present?

Yes

[sbrannen]

... well... unless you invoke extensionContext.getRoot().getTestInstanceLifecycle() which should always _empty_. 😉

[sbrannen]

Please note that the "actual behavior" you have described is documented in the class-level JavaDoc for @TestInstance:

If @TestInstance is not explicitly declared on a test class or on a test interface implemented by a test class, the lifecycle mode will implicitly default to PER_METHOD. Note, however, that an explicit lifecycle mode is _inherited_ within a test class hierarchy.

[sbrannen]

In addition, as stated in https://github.com/junit-team/junit5/issues/1566#issuecomment-416996200:

A @Nested test class can be configured with its own lifecycle mode which may differ from that of an enclosing test class.

A @Nested test class cannot change the lifecycle mode of an enclosing test class.

The general rule is that each test class (whether top-level, nested, or a subclass) decides for itself what its lifecycle mode should be. The only form of _inheritance_ with regard to lifecycle modes is that subclasses inherit the mode from their parent class by default.

[sbrannen]

With regard to what you consider the _expected behavior_:

Configured Lifecycle.PER_CLASS is inherited in Nested test class from the outer class.

I can understand that you might expect that since the lifecycle is inherited within a test class hierarchy.

The difference here is that we are talking about a nested class structure, which technically (in terms of the Java language) has no form of inheritance.

Having said, however, it would be possible to introduce a new flag -- analogous to the existing junit.jupiter.testinstance.lifecycle.default configuration parameter -- that would allow a project to _turn on_ inheritance for the lifecycle within nested class structures.

Please note that we cannot do that by default, since that would be a breaking change.

@Nested class cannot override enclosing class's Lifecycle

I cannot imagine that we would ever want to prevent a nested class from declaring its own lifecycle, since that would be unnecessarily restrictive.

[seanf]

I see, I misunderstood https://github.com/junit-team/junit5/issues/1566#issuecomment-416966563, specifically the line "A @Nested test class cannot change the @TestInstance lifecycle mode declared for an enclosing class." That just means the enclosing class will keep its lifecycle, not that the nested class will have the same lifecycle itself.

Thanks for the explanation.

[sbrannen]

That just means the enclosing class will keep its lifecycle, not that the nested class will have the same lifecycle itself.

Right.

And I see you closed this issue. OK.

[seanf]

I cannot imagine that we would ever want to prevent a nested class from declaring its own lifecycle, since that would be unnecessarily restrictive.

Quite true, I was thinking it would be. I should have realised that TestInstance.Lifecycle has been around since 5.0, and would be pretty stable by now.

Having said, however, it would be possible to introduce a new flag -- analogous to the existing junit.jupiter.testinstance.lifecycle.default configuration parameter -- that would allow a project to turn on inheritance for the lifecycle within nested class structures.

It seems like the sort of thing that would be better controlled at the class level, say with an annotation on the outer class which enables "nest-inheritance" of Lifecycle. I think a bunch of tests which depend on this sort of behaviour will break pretty badly if a global properties file is changed. Seems like it would be better as a local decision with local consequences.

[seanf]

And I see you closed this issue. OK.

Well, if I'm wrong about the intended behaviour, I'm wrong.

In my situation, having the lifecycle change in the nested tests is kind of a nuisance though.

[sbrannen]

It seems like the sort of thing that would be better controlled at the class level, say with an annotation on the outer class which enables "nest-inheritance" of Lifecycle

Yeah, you're probably right: localizing it would likely be better.

[sbrannen]

In my situation, having the lifecycle change in the nested tests is kind of a nuisance though.

I understand that.

This is "one of those features that can't please everyone". The team had to pick one or the other. C'est la vie.

[sbrannen]

It seems like the sort of thing that would be better controlled at the class level, say with an annotation on the outer class which enables "nest-inheritance" of Lifecycle

Feel free to open an issue dedicated to that proposal, and we can see if the idea gains traction within the community.

Cheers

[seanf]

@sbrannen How about a new title: Allow enclosing test class to force @Nested classes to use same Lifecycle?

[sbrannen]

I'd prefer a separate issue. See previous comment. 😉