Junit5: 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

• #48
• #88
• #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

• [x] Reintroduce support for @TestInstance as was present in the JUnit 5 prototype.
  
  
  
  • [x] Make @TestInstance an @Inherited annotation to support lifecycle inheritance semantics within a test class hierarchy.
  
  
• [x] Add support for @TestInstance for @Nested test classes.
  
  
  
  • [x] Ensure proper behavior for @Nested test classes in terms of the lifecycle of the test instance against which @BeforeAll and @AfterAll methods are executed.
  
  
• [x] Allow @BeforeAll and @AfterAll methods to be non-static.
• [x] Move the getTestInstance() method from TestExtensionContext to ExtensionContext.
  
  
  
  • [x] Ensure that the proper test instance is supplied to container-level extensions via the ExtensionContext.
  
  
  • [x] Ensure that the proper test instance is supplied to TestInstancePostProcessor extensions via the ExtensionContext as well.
  
  
  • [x] Ensure that the proper test instance is supplied to TestTemplateContainerExtensionContext extensions via the ExtensionContext as well.
  
  
• [x] Add documentation to the User Guide for the new features.
• [x] Update existing documentation in the User Guide which states that @BeforeAll and @AfterAll methods must be static.
• [x] Update existing JavaDoc which states that @BeforeAll and @AfterAll methods must be static.
• [x] Update the release notes in the User Guide describing the new features.
• [x] 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: https://github.com/junit-team/junit5/issues/88#issuecomment-231587363.

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]

_in progress_

[sbrannen]

FYI: introduced a _Deliverables_ section for this issue.

[marcphilipp]

@sbrannen Any news on this one? Is it still _in progress_?

[sbrannen]

Yep. Still in progress.

[sbrannen]

FYI: current work on this issue can be see in the following branch.

https://github.com/junit-team/junit5/commits/issues/419-test-instance-lifecycle

[sbrannen]

Note that deliverables 1 and 2 have already been addressed in the aforementioned branch.

[sbrannen]

FYI: the aforementioned branch is now feature complete.

The _Deliverables_ have been updated accordingly.

[sbrannen]

@sdeleuze, can you please confirm that annotating Kotlin test classes with @TestInstance(Lifecycle.PER_CLASS) will address your concerns (at least theoretically since it's difficult for you to verify against the branch)?

[sdeleuze]

Thanks for working on this I was not thinking this was so much work!

Is it true that @BeforeAll and @AfterAll on non-static methods currently require @TestInstance(Lifecycle.PER_CLASS) annotation on the class as shown in this test?

If the anwser is yes, is it doable to remove this limitation? Maybe a test instance with the default behavior could be created for running @BeforeAll and @AfterAll annotated methods like it is done for @Test annotated method.

I think that's what users not aware of JUnit internals would expect.

[sbrannen]

Thanks for working on this I was not thinking this was so much work!

Yeah, I wasn't really either... until I started digging deeper!

It was much easier to implement in the JUnit Lambda prototype. 😉

[sbrannen]

Is it true that @BeforeAll and @AfterAll on non-static methods currently require @TestInstance(Lifecycle.PER_CLASS) annotation on the class as shown in this test?

Yes, that is correct.

The default behavior is Lifecycle.PER_METHOD... in order to align with _presumed_ expectations of developers accustomed to the behavior in previous versions of JUnit, although based on my rather extensive experience... 95% of the developers in this world never ever realize that JUnit 4 instantiates the test class for each test method.

In fact, in the JUnit Lambda prototype I originally implemented "per class" semantics by default (as in TestNG and other testing frameworks). But... I got voted down by the majority of the JUnit Lambda team and had to reverse the semantics. 😞

If the answer is yes, is it doable to remove this limitation?

Not really. There has to be _default_ behavior with regard to test instance lifecycle (disregarding @BeforeAll and @AfterAll methods). In the end it's one or the other: per-method vs. per-class.

Maybe a test instance with the default behavior could be created for running @BeforeAll and @AfterAll annotated methods like it is done for @Test annotated method.

I would not condone that approach since it would make it impossible to share instance state, which is a prerequisite of features such as scenario tests. Plus I think such an approach would be even less intuitive for developers.

I think that's what users not aware of JUnit internals would expect.

If you are claiming that "per class" semantics should be the default, then I agree. 😉

But... it's not too late in the game: we could still switch to "per class" semantics as the default for JUnit Jupiter. That would break with the semantics from JUnit 1 through JUnit 4, but I could live with that. Now it's just up to the community to convince the rest of the JUnit 5 Team if the community truly wants "per class" test instance lifecycle be default. 😇

[marcphilipp]

I feel very strongly about this, almost to a point where I'd say this is not even up for debate.

Using a separate test instance for each test method avoids so many problems developers, especially inexperienced ones, would otherwise run into. In particular, it avoids state problems by giving each test a clean slate, thereby decoupling them so you do not end up depending on their order etc.

I understand that you'd like this to be the default behavior for tests in Kotlin. Is there a way to detect a Class was written in Kotlin using the Reflection API?

[sbrannen]

I feel very strongly about this, almost to a point where I'd say this is not even up for debate.

I suspected that might be the case. 😉

I understand that you'd like this to be the default behavior for tests in Kotlin. Is there a way to detect a Class was written in Kotlin using the Reflection API?

Even if it's technically possible to change the default test instance lifecycle semantics based on the programming language in use, would it really be a good idea to have different behavior in Java vs. Groovy vs. Kotlin...?

[marcphilipp]

Even if it's technically possible to change the default test instance lifecycle semantics based on the programming language in use, would it really be a good idea to have different behavior in Java vs. Groovy vs. Kotlin...?

I think it would be a clear rule with a sound justification in the case of Kotlin.

[sormuras]

I understand that you'd like this to be the default behavior for tests in Kotlin. Is there a way to detect a Class was written in Kotlin using the Reflection API?

Even if it's technically possible to change the default test instance lifecycle semantics based on the programming language in use, would it really be a good idea to have different behavior in Java vs. Groovy vs. Kotlin...?

I don't think so. It should be up to the end-user to define a lifecycle behaviour explicitly w/o some magic within the framework.

In general I share @marcphilipp 's point of view regarding the lifecycle behaviour: Lifecycle.PER_METHOD should stay the default one.

[sbrannen]

Brainstorming...

Another option would be to introduce a configuration flag (e.g., system property) that allows teams to set the default test instance lifecycle semantics on a per-project basis.

That would at least make it less cumbersome to adopt per-class semantics within a team without forcing developers to copy-n-paste @TestInstance(Lifecycle.PER_CLASS) across the code base.

It would remain an opt-in decision but considerably less intrusive with the configuration flag.

Thoughts?

[sdeleuze]

Interesting discussion ... Indeed I admit I was not aware that JUnit creates a new test instance for each test method.

I am not aware of JUnit internals but I don't totally understand what prevent so strongly to support @BeforeAll and @AfterAll with the default behavior of one instance per method? Sorry if you already explained it but that's not clear to me. Could you give me more hints about the blocking points (since this solution would be better than a switch IMO)?

[marcphilipp]

If I understand you correctly (please correct me if I'm wrong), you're asking what prevents us from supporting non-static @BeforeAll/@AfterAll methods with instance per method semantics.

Well, to invoke a non-static method we need an instance of the test class. Since a @BeforeAll/@AfterAll method must only be invoked once, what instance(s) should it be invoked with?

[sbrannen]

Well, to invoke a non-static method we need an instance of the test class. Since a @BeforeAll/@AfterAll method must only be invoked once, what instance(s) should it be invoked with?

I think he means that we could create a throw-away instance (or two) and just invoke the @BeforeAll/@AfterAll methods against that/those instance(s).

@sdeleuze, please correct me if I'm wrong with that assumption.

[sbrannen]

I think he means that we could create a throw-away instance (or two) and just invoke the @BeforeAll/@AfterAll methods against that/those instance(s).

But again, if my assumption is correct, I would not condone that approach since the test methods would then have no means to access any state managed by @BeforeAll/@AfterAll methods (e.g., something like a _server_ that is started in a @BeforeAll method). The only way to share such _global_ state in such a scenario would be to store it in static fields, but then it wouldn't make much sense to have the @BeforeAll/@AfterAll methods be non-static. It's a Catch 22. 😉

[sbrannen]

BTW, when I said:

Another option would be to introduce a configuration flag (e.g., system property) that allows teams to set the default test instance lifecycle semantics on a per-project basis.

... I was not proposing that such a change be incorporated in this issue. Rather, that would be a topic for M6.

[sbrannen]

New issue created: https://github.com/junit-team/junit5/issues/905

[sdeleuze]

@sbrannen Yes, this is what I had in mind, but you are right: it seems what I would like ideally is @TestInstance(Lifecycle.PER_CLASS) by default. If this is not doable in JUnit 5, a configuration flag as proposed in #905 is maybe the best trade-off we could find.

[sbrannen]

Update: the _User Guide_ (in the branch) now contains documentation on the new features associated with test instance lifecycle.

[sbrannen]

This issue has been addressed in master in 502fc31f9dffcd494607cb5d17de422d5202ca65.

[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!