Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade spring boot starter test to JUnit 5 #14736

Closed
philwebb opened this issue Oct 9, 2018 · 16 comments
Closed

Upgrade spring boot starter test to JUnit 5 #14736

philwebb opened this issue Oct 9, 2018 · 16 comments
Assignees
Labels
theme: testing Issues related to testing type: task A general task
Milestone

Comments

@philwebb
Copy link
Member

philwebb commented Oct 9, 2018

We're getting a lot of users asking for a dedicated JUnit 5 starter POM, but I'd rather we investigate how we can upgrade our existing starter to JUnit 5.

We need to consider the following use-cases:

  • For green field applications it would be nice to use only JUnit 5. It will be confusing if there are multiple @Test imports.

  • For existing applications it would be nice if they can migrate to JUnit 5 in their own time. An upgraded application should continue to work. It's likely we'll need to support this type of hybrid application for some time.

  • It's possible that some users might want to remain on JUnit 4 only. We'll need to consider if this is something we want to support, or if it's something they should do themselves.

@philwebb
Copy link
Member Author

philwebb commented Oct 9, 2018

See also #6402 (which this one might end up being a duplicate of)

@wilkinsona
Copy link
Member

wilkinsona commented Oct 9, 2018

Following advice from the Gradle team, we've raised the minimum supported version of Gradle to 4.4 in Boot 2.1. Support for JUnit 5 was added to Gradle in 4.6 so moving to JUnit 5 by default will have a knock-on effect of requiring us to raise the minimum supported version of Gradle. I suspect that'll be fine post 2.1.

@wilkinsona
Copy link
Member

wilkinsona commented Nov 28, 2018

Eclipse does not yet fully support JUnit 5 out of the box. To use it with 2018-09 you either have to install a patch or also have JUnit 4 on the classpath. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=538956 for details. The problem should be fixed in 2018-12.

@sbrannen
Copy link
Member

Support for JUnit 5 was added to Gradle in 4.6 so moving to JUnit 5 by default will have a knock-on effect of requiring us to raise the minimum supported version of Gradle.

Please note as well that there have been bug fixes in Gradle's support for JUnit 5 in subsequent Gradle releases as well. So, Gradle 4.6 would be the bare minimum.

@mbhave mbhave modified the milestones: 2.x, 2.2.x Dec 4, 2018
mbhave added a commit to mbhave/spring-boot that referenced this issue Dec 5, 2018
mbhave added a commit to mbhave/spring-boot that referenced this issue Dec 6, 2018
@wilkinsona
Copy link
Member

Unfortunately, it appears that the combination of Surefire, JUnit's Vintage Engine, and Spring Framework's test framework is not yet ready for prime time.

When using the Vintage Engine, any logging performed by the test runner during the creation of the test description corrupts Surefire's communication link between the main Maven JVM and the JVM that's running the tests. This can be identified by Surefire logging a warning similar to the following:

[WARNING] Corrupted STDOUT by directly writing to native stream in forked JVM 1. See FAQ web page and the dump file …/target/surefire-reports/2018-12-12T09-56-36_709-jvmRun1.dumpstream

I believe this is a bug in Surefire. It appears that when the Vintage Engine is in use, runners are called earlier and at a time when Surefire is unable to cope with output to System.out. I have opened SUREFIRE-1614.

Spring Framework's test framework regularly logs at info level during test description creation. The logging in AbstractContextLoader.generateDefaultLocations(Class<?>) is one example. This means that corruption of the communications like and the warning being output will be a common problem if we switch the starter to the Vintage Engine and encourage a gentle migration from the JUnit 4 API to the JUnit Jupiter API.

A workaround is to use a logback-test.xml or log4j2-test.xml to tune the logging configuration so that nothing is written to System.out before Surefire is able to cope with such output. Unfortunately, this requires a certain amount of trial and error as exactly what will be logged and when depends on the specific runners that are being used and the test configuration.

@wilkinsona wilkinsona added for: team-attention An issue we'd like other members of the team to review status: blocked An issue that's blocked on an external project change and removed for: team-attention An issue we'd like other members of the team to review labels Dec 12, 2018
@sbrannen
Copy link
Member

I believe this is a bug in Surefire. It appears that when the Vintage Engine is in use, runners are called earlier and at a time when Surefire is unable to cope with output to System.out. I have opened SUREFIRE-1614.

Thanks for the great detective work, @wilkinsona, and for raising the issue with Surefire!

@wilkinsona
Copy link
Member

wilkinsona commented Jan 25, 2019

Unlike JUnit 4, JUnit 5 does not depend on Hamcrest. As part of moving the starter to JUnit 5, we should take the opportunity to consider if we want to remove Hamcrest (see #15789 for background which I've closed in favour of this issue).

@dupski
Copy link

dupski commented Feb 14, 2019

It looks like apache/maven-surefire#209 has been merged now.

Should the "blocked" tag be removed now @wilkinsona ? I just started a new spring boot project and was surprised that JUnit 4 is still the default.

@wilkinsona
Copy link
Member

wilkinsona commented Feb 14, 2019

We’re still blocked, unfortunately. The fix is only available in Surefire 3.0 which, as is fitting for a new major release, contains some breaking changes. Ideally, we wouldn’t upgrade to it in a minor release of Boot and we’d like the changes to be backported to Surefire 2.x instead. Any news on that, @snicoll?

@wilkinsona
Copy link
Member

Another problem with Surefire: https://issues.apache.org/jira/projects/SUREFIRE/issues/SUREFIRE-1546. @DisplayName doesn't work at the moment. It's fixed in 3.0.0-M4 but it has not yet been released. It looks increasingly like a move to JUnit 5 is going to require us to jump to Surefire 3.0 as well.

@wilkinsona
Copy link
Member

The current schedule for Surefire 3.0 has M4 starting on 24/12/18 and scheduled for release on 24/02/19. There are then two more milestones planned before 3.0 GA. Those three releases have no dates at the moment. Taking a bit of a stab in the dark and assuming that M5 and M6 have a similar duration to M4, it's pretty tight for 3.0 have reached GA before we RC at the beginning of July.

@Tibor17
Copy link

Tibor17 commented Feb 24, 2019

@philwebb
We are developing Surefire in ASF.
Here is too much noise around Surefire. Can you tell me what exactly you need or expect from us in Surefire?

@snicoll
Copy link
Member

snicoll commented Feb 25, 2019

@Tibor17 thank you for reaching out. I've started a thread on maven-dev and I am happy to follow up with the team over there.

mbhave added a commit to mbhave/spring-boot that referenced this issue May 7, 2019
snicoll pushed a commit to snicoll/spring-boot that referenced this issue May 7, 2019
snicoll pushed a commit to snicoll/spring-boot that referenced this issue May 7, 2019
mbhave added a commit to mbhave/spring-boot that referenced this issue May 8, 2019
@snicoll snicoll removed the status: blocked An issue that's blocked on an external project change label May 8, 2019
@snicoll snicoll closed this as completed in d9f339a May 8, 2019
snicoll added a commit that referenced this issue May 8, 2019
@snicoll snicoll modified the milestones: 2.2.x, 2.2.0.M3 May 8, 2019
@snicoll snicoll changed the title Investigate JUnit 5 starter upgrade Upgrade spring boot starter test to JUnit 5 May 8, 2019
@snicoll
Copy link
Member

snicoll commented May 8, 2019

spring-boot-starter-test now brings JUnit Jupiter with the vintage engine. JUnit 4 tests are still executed and it is possible to use the new JUnit Jupiter API. The plan is that we will remove the vintage engine at some point to direct users at JUnit 5 only.

@nightswimmings

This comment has been minimized.

@philwebb

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: testing Issues related to testing type: task A general task
Projects
None yet
Development

No branches or pull requests

8 participants