Philzen/rewrite-TestNG-to-JUnit5

Add `@TestInstance(TestInstance.Lifecycle.PER_CLASS)` where required

Philzen opened this issue · 0 comments

TestNG instantiates test classes once and then runs all contained test methods on the same instance, while JUnit by default instantiates a fresh object for each test method:

In order to allow individual test methods to be executed in isolation and to avoid unexpected side effects due to mutable test instance state, JUnit creates a new instance of each test class before executing each test method (see Definitions). This "per-method" test instance lifecycle is the default behavior in JUnit Jupiter and is analogous to all previous versions of JUnit.

This behavior can be changed to mimic that of TestNG by putting the @TestInstance(TestInstance.Lifecycle.PER_CLASS) annotation on the class-level.


https://github.com/MBoegers/migrate-testngtojupiter-rewrite/blob/main/src/main/java/io/github/mboegers/openrewrite/testngtojupiter/AddTestLifecyleToJUnitTests.java can serve as a nice starting point here.


Important: It is only applied to the scope it is annotating, that means every nested class will still be instantiated per-method unless also annotated with @TestInstance(TestInstance.Lifecycle.PER_CLASS). The annotation will be absolutely mandatory on a nested class if it contains @BeforeAll / @AfterAll annotations, otherwise any tests in that class won't be executed and the Junit runner will fail with:

org.junit.platform.commons.JUnitException: @BeforeAll method 'void org.philzen.oss.research.JupiterNestedBeforeAllTest$Topic.beforeAll()' must be static unless the test class is annotated with @testinstance(Lifecycle.PER_CLASS).


It may be considered to skip the adding the annotation onto a class if all of the following is true:

  • all methods in it are @Tests
  • there is no constructor

However, in certain fringe cases this may not be enough, as a (badly written, however possible) test could refer to some global state (not only static state in other classes, but also System properties, etc.). So it is probably the safest to apply the migration everywhere.