Migrate test suite to JUnit5

[slarse]

We currently have a little bit of JUnit5 (such as VariableTest), but mostly JUnit4 in the test suite. I think it's worth our while to start migrating entirely to JUnit5. Off the top of my head, here are some benefits that come with JUnit5:

• Much better support for parameterized tests
• Better support for temporary directories
• Better support for conditional test execution
• The awesome assertThrows assertion
  • Edit: Actually, this was backported to JUnit 4.13, so it's a moot point

This is also very easy to do incrementally, one test class at a time, making it a very good first issue.

Why not keep both?

While we can keep both JUnit4 and JUnit5, and one can even write test cases of both flavors in the same test class, it's just messy to do so due to all of the assertions and annotations having the same simple names. You can also be pretty confident that if a contributor comes into a test class and sees only JUnit4 tests, that contributor will write JUnit4 tests as well. The longer we wait, the more work it will be to leave JUnit4 behind!

How do I know if a test class is using JUnit4?

The easiest way to tell is to look for imports from the JUnit4 API, the most telling one being import org.junit.Test. At the moment of writing, there are 189 such imports in the test suite.

$ grep -r 'import org.junit.Test' src/test/java | wc -l
189

How do I know if a test class is using JUnit5?

The easiest way to tell is to look for imports from the JUnit5 API, the most telling one being import org.junit.jupiter.api.Test. At the moment of writing, there are 3 such imports in the test suite.

$ grep -r 'import org.junit.jupiter.api.Test' src/test/java | wc -l
3

How can I convert from JUnit4 to JUnit5?

See #3921 for an example.

Some IDEs offer auto-conversion, such as IntelliJ IDEA. But in general, remove any org.junit import that is not on the form org.junit.jupiter (e.g. import org.junit.Test should be removed), and then fix any compile or test errors by importing the appropriate classes (e.g. import org.junit.jupiter.api.Test) and methods from from the org.junit.jupiter package. It's really rather straightforward.

To check that you've removed all JUnit4 stuff from a test class, you can run the following in a bash-like terminal:

$ grep org.junit <path to the test class> | grep -v jupiter

If it prints no output, you're done!

Note about class/method visibility: While JUnit4 mandated public test classes and methods, JUnit5 allows any visibility but private. However, it's best if you leave the visibility as-is to minimize the diff.

To round off, here are a couple of nice articles about moving from JUnit4 to 5:

• Oracle blog
• Baeldung article

Scope of a single PR?

Convert one test class per PR. No more, and no less!

Objections?

If anyone is opposed to this, please raise your voice :). I don't see any harm in migrating test classes, even if we never finish up entirely. 189 also sounds like a big number, but as it's semi-automatic, it typically doesn't take more than a few minutes to migrate a test class (of course, there are exceptions, especially if there's heavy use of @Rule).

[Rohitesh-Kumar-Jain]

Hi @slarse,

Thanks for opening this issue, this seems a really good task that I can start in this community bonding phase.

[slarse]

Hi @Rohitesh-Kumar-Jain,

Absolutely! I hope the instructions should be sufficient, but let me know if you need any help.

Something that I think I forgot to write in the OP is that you should do one test class per PR, to keep the PRs focused and easy to review :)

[monperrus]

Some IDEs offer auto-conversion, such as IntelliJ IDEA.

Super useful transformation, thanks for the link

[Rohitesh-Kumar-Jain]

Hi @slarse,

I have one question what is spoon-pom is for, and what is pom is for.

I am actually trying to replace @Rule with @ExtendWith in this Class, for that, I believe I need to add this dependency

<dependency>
    <groupId>org.mockito</groupId>
    <artifactId>mockito-junit-jupiter</artifactId>
    <version>3.10.0</version>
    <scope>test</scope>
</dependency>

Apart from this, there are other pom files as well, what are they for, and in which pom file(s) should I be adding my dependencies while working on the Spoon core tests?

[slarse]

Hi @Rohitesh-Kumar-Jain,

spoon-pom is the parent pom-file for the Spoon modules. If you have a look in e.g. the spoon-core pom file, you'll see that it declares the spoon-pom/pom.xml file as its parent.

We typically put test dependencies into spoon-pom/pom.xml so they are available to all inheriting pom-files.

[MartinWitt]

With #4570 merged the last JUnit 4 usage in spoon-core is migrated. See https://github.com/search?q=org.junit.Test%3B+repo%3AINRIA%2Fspoon+path%3A%2Fsrc%2F+language%3AJava+language%3AJava&type=Code&ref=advsearch&l=Java&l=Java.

[slarse]

Nicely done @MartinWitt . Closing.

[MartinWitt]

Oh the closing was fast.

Shall we migrate all spoon subprojects like spoon-decompiler too?

In my opinion, yes for consistency (and JUnit5 is simply better), but I'm not sure about the ownership model for those subprojects. @monperrus, wdyt?

This question is still open. Otherwise, we can't remove the JUnit 4 dependency easy.

[slarse]

Point.

The question is however not open, the answer is yes migrate them. IMO we can do one subproject per PR.

[slarse]

So, this issue then closes with the removal of the JUnit4 dependency in Spoon!

[monperrus]

I'm not sure about the ownership model for those subprojects. @monperrus, wdyt?

I'd say there is no ownership, it is purely best-effort open-source. If you care, you PR.

And yes, agree, it'd be great to port the submodules, one project per PR. Good news, they have light test suites.

[slarse]

Good news, they have light test suites.

The one situation when that's good news :)

[monperrus]

closing thanks to the gritty work of @MartinWitt!