Allow @BeforeAll and @AfterAll methods to be non-static

[mfulton26]

Overview

#88 explains why @BeforeAll and @AfterAll methods currently must be static; however, the question arises again and again.

Related Issues

• Introduce first-class support for scenario tests #48
• Document that @BeforeAll & @AfterAll are unsupported in @Nested test classes #88
• Introduce configuration parameter for setting default test instance lifecycle semantics #905

Proposal

Reintroduce support for @TestInstance as was present in the JUnit 5 Prototype, thereby allowing developers and third party extension authors to make use of the feature as well. In other words, we do not want to limit use of this feature to JUnit Jupiter itself.

Open Issues

Special caution must be taken with regard to @Nested test classes in terms of the lifecycle of the test instance against which @BeforeAll and @AfterAll methods are executed.

Original Issue Description

Can support be added for @BeforeAll and @AfterAll methods in @Nested test classes?

For example:

class Tests {
    @Test
    void something() {
    }

    @BeforeAll(nested = SubTests.class)
    static void setUp() {
    }

    @Nested
    class SubTests {

        // ...

Deliverables

• Reintroduce support for @TestInstance as was present in the JUnit 5 prototype.
  • Make @TestInstance an @Inherited annotation to support lifecycle inheritance semantics within a test class hierarchy.
• Add support for @TestInstance for @Nested test classes.
  • Ensure proper behavior for @Nested test classes in terms of the lifecycle of the test instance against which @BeforeAll and @AfterAll methods are executed.
• Allow @BeforeAll and @AfterAll methods to be non-static.
• Move the getTestInstance() method from TestExtensionContext to ExtensionContext.
  • Ensure that the proper test instance is supplied to container-level extensions via the ExtensionContext.
  • Ensure that the proper test instance is supplied to TestInstancePostProcessor extensions via the ExtensionContext as well.
  • Ensure that the proper test instance is supplied to TestTemplateContainerExtensionContext extensions via the ExtensionContext as well.
• Add documentation to the User Guide for the new features.
• Update existing documentation in the User Guide which states that @BeforeAll and @AfterAll methods must be static.
• Update existing JavaDoc which states that @BeforeAll and @AfterAll methods must be static.
• Update the release notes in the User Guide describing the new features.
• Update the release notes in the User Guide describing breaking changes.

[sbrannen]

The short answer is "no".

For further details, see #48, #88, and especially this comment: #88 (comment).

However, I will leave this issue open since the question is asked so frequently on its own unrelated to scenario tests.

[sbrannen]

FYI: I have updated this issue's title and added an Overview section to its description.

[mfulton26]

I see. Thanks!

[sdeleuze]

@sbrannen Allowing @BeforeAll and @AfterAll to be non-static would make usage of JUnit 5 much more pleasant with Kotlin where we currently have to use crappy code (even if you can blame Kotlin for that, but I believe they really did the best they could in finding a good tradeoff between good Java interrop and clean design) to make it work.

As described here, currently we have to create a companion object with method annotated with @JvmStatic to make it works.

Enabling that would allow Junit 5 usage much more simple, with idiomatic Kotlin code without requiring usage of a Kotlin specific library for testing.

[sbrannen]

Hi @sdeleuze,

Nice to see a fellow core Spring committer chiming in here! 😉

As for your request... you're preaching to the choir (at least when addressing me). In the JUnit Lambda prototype @bechte and I implemented support for @BeforeAll and @AfterAll that only required such methods to be static if the test class was configured with "test instance per method" semantics.

I am personally totally in favor of bringing back that support. In fact, it is simply a requirement for proper support for Scenario Tests (see #48 for details). So it will have to be pulled back into the framework in one form or other, and that should happen in the M5 time frame.

On a different topic, what do you think about using Kotlin's spek testing framework? Is that a viable option for you? Or are you more focused on using the Spring TestContext Framework with JUnit Jupiter support for testing your Kotlin codebase?

[sbrannen]

@junit-team/junit-lambda what do you guys think about reinstating the @TestInstance annotation for M4 since we'll need it (or something equivalent) for scenario tests in M5?

[sdeleuze]

My pleasure to be here :-)

I think both could make sense depending on the use case. For functional Spring + Kotlin applications like the one I develop on https://github.com/mix-it/mixit, I think Spek could be a better fit (we are going to use it shortly, it is on our TODO), while for regular Spring Boot annotation based applications, being able to take advantage of Spring TestContext Framework with JUnit Jupiter support would make sense for a lot of developers I think.

[sbrannen]

Slated for M4 now in order to ease adoption for Kotlin users.

[sbrannen]

Disclaimer: slated for M4, but might get shifted forward to M5 again. 😉

[sdeleuze]

That would be awesome in M4, but I notice the disclaimer ;-) Kotlin support is now one of the major themes of Spring Framework 5 as announced in my recent blog post and this issue is the only really annoying issue with Kotlin + Junit 5 I am aware of, so fingers crossed ;-)

[marcphilipp]

what do you guys think about reinstating the @TestInstance annotation for M4 since we'll need it (or something equivalent) for scenario tests in M5?

I'm ok with reinstating it. We need to make sure to validate that test methods don't add instance level extensions like TestInstancePostProcessor then, though.

[sbrannen]

I'm ok with reinstating it. We need to make sure to validate that test methods don't add instance level extensions like TestInstancePostProcessor then, though.

I'm not sure about that: sometimes you may want dependencies to be re-injected between test methods. So it's really up to the extension.

For example, with TestNG (which supports scenario-style tests), the Spring TestContext Framework re-injects dependencies if the previously executed test method was annotated with @DirtiesContext since the state of the previously injected dependencies is no longer valid.

[marcphilipp]

Point taken.

[sbrannen]

FYI: I have added Proposal and Open Issues sections to this issue's description.

[sbrannen]

This issue has been addressed in master in 502fc31.

[kcooney]

Sorry to be late to the discussion. One option that I have considered in the past is to allow @BeforeAll methods take in a Store (the "class" Store). That Store could be used to store data needed for all test methods (like connection parameters to a locally created database, or the port of a locally running HTTP server). We could pass the same Store instances to any @AfterAll methods that take in a Store. A test method that takes in a Store would get a unique store for each test instance, but those stores would have their "parent" Srore being the one passed to the "class" Store

Since extensions have access to the Store already, this would allow for great reuse while still limiting accidental reuse of state between test instances.

(yes I realize for Kotlin we wouldn't want to require static methods; just suggesting a different solution for the general problem of scenario tests).

[sbrannen]

Sorry to be late to the discussion.

Yesss... rather late to the closed discussion. 😉

If you'd like the team to consider your idea about sharing the Store with test code, please open a new issue.

Cheers!