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

Provide dedicated Spring Boot starter for JUnit 5 #14716

Closed
odrotbohm opened this issue Oct 8, 2018 · 27 comments
Closed

Provide dedicated Spring Boot starter for JUnit 5 #14716

odrotbohm opened this issue Oct 8, 2018 · 27 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@odrotbohm
Copy link
Member

odrotbohm commented Oct 8, 2018

When trying to port a Spring Boot project that has previously used JUnit 4 to JUnit 5 one needs to add quite a few additional dependencies to basically get back to the very same functionality that one got with the standard test starter. This is due to the fact that in JUnit 5, a lot of functionality that had been included in JUnit itself was extracted into extensions that now live in third party projects that require explicit inclusion.

Most notabily it are the following additional libraries:

  • org.junit.jupiter:junit-jupiter-params for parameterized tests
  • org.mockito:mockito-junit-jupiter for @Mock
  • org.hamcrest:hamcrest-junit for assertThat(…)

Especially the latter is quite nasty as it in turn depends on JUnit 4.12, so that an explicit exclude is needed.

<dependency>
  <groupId>org.springframework.boot</groupId>
  <artifactId>spring-boot-starter-test</artifactId>
  <scope>test</scope>
  <exclusions>
    <exclusion>
      <groupId>junit</groupId>
      <artifactId>junit</artifactId>
    </exclusion>
  </exclusions>
</dependency>

<dependency>
  <groupId>org.junit.jupiter</groupId>
  <artifactId>junit-jupiter-engine</artifactId>
  <scope>test</scope>
</dependency>

<dependency>
  <groupId>org.junit.jupiter</groupId>
  <artifactId>junit-jupiter-params</artifactId>
  <scope>test</scope>
</dependency>

<!-- 
  As indicated by Marc below this could be replaced by
  hamcrest-core to avoid the exclusion
-->
<dependency>
  <groupId>org.hamcrest</groupId>
  <artifactId>hamcrest-junit</artifactId>
  <version>2.0.0.0</version>
  <scope>test</scope>
  <exclusions>
    <exclusion>
      <groupId>junit</groupId>
      <artifactId>junit</artifactId>
    </exclusion>
  </exclusions>
</dependency>

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

Be sure to read the note on that list below.

It would be cool if that could be reduced to only declare a spring-boot-starter-test-junit5 (name tbd) instead of spring-boot-starter-test.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 8, 2018
@wilkinsona
Copy link
Member

wilkinsona commented Oct 8, 2018

@olivergierke and I discussed this on Slack last week. A couple of thoughts from me from that discussion:

If we wanted to provide a starter for JUnit Jupiter, I guess we’d split some of spring-boot-starter-test out into spring-boot-starter-junit-4 and then have a spring-boot-starter-junit-jupiter that it could be swapped out for. That’d be similar to what we do for the embedded containers and spring-boot-starter-web

It’s a tricky one. An extra starter would probably have made your life easier but it adds complexity to Boot to solve a problem that’s really in JUnit. It’ll be fine when we flip everyone over to JUnit Jupiter, but I’m not sure things are ready for that just yet.

One additional thought:

The example above makes things look one dependency worse then they actually are. junit-jupiter-api is a transitive dependency of junit-jupiter-engine so need not be declared explicitly.

@wilkinsona
Copy link
Member

I concluded with the following:

It’s probably worth opening an issue to see what the rest of the team thinks
I suspect they’ll be against it, but the point you made about the params dependency and the mockito dependency is a good one

@odrotbohm
Copy link
Member Author

Thanks for the API JAR catch, I've removed that one. Regarding the naming, I thought that aligning with …-junit4 and …-junit5 is probably better than something …-jupiter as that's leaking some internal code name into the user's projects. I.e. users a re rather thinking in "I want to upgrade to JUnit 5" rather than "I want to migrate to the Jupiter engine".

@snicoll
Copy link
Member

snicoll commented Oct 8, 2018

While I understand there is slightly more work than expected to use JUnit Jupiter, I am not keen to branch our current starter (or any starter for that matter). I agree with @wilkinsona that this adds complexity here for a problem that's in JUnit.

@odrotbohm
Copy link
Member Author

odrotbohm commented Oct 8, 2018

… that this adds complexity here for a problem that's in JUnit.

Hm, isn't basically every Spring Boot starter an attempt to solve that problem for some third party library? I.e. what else than an attempt to solve exactly that problem are starters in the first place?

How do you mean branch? Wouldn't it be a new starter (i.e. a single additional POM file) containing just a slightly different setup of JUnit libraries? It's not that new major versions of JUnit pop up every other week, is it?

If you think it's too complex, fine. I just thought it felt weird to have to declare 5 explicit dependencies instead of 1, one of which even needs an explict version number if I wanted to use the current version of the most commonly used Java testing framework. That's quite at odds with the rest of the Spring Boot dependency management experience.

@snicoll
Copy link
Member

snicoll commented Oct 8, 2018

How do you mean branch?

What I mean is having a "v4" and "v5" starter of the same feature. Regardless of how we name the new one, it'll become irrelevant once we decide to switch the current starter to JUnit 5. That's an odd situation I'd like to avoid if we can.

@wilkinsona
Copy link
Member

wilkinsona commented Oct 8, 2018

I just thought it felt weird to have to declare 5 explicit dependencies instead of 1, one of which even needs an explict version number if I wanted to use the current version of the most commonly used Java testing framework.

It may be the current version as in the latest, but it isn't the most widely used, even for greenfield development. I'm pretty sure that honour still belongs to JUnit 4.

That's quite at odds with the rest of the Spring Boot dependency management experience.

Our goal is to make common use cases as easy as possible without preventing more complex or less typical scenarios. In this situation the common use case is to use JUnit 4 and the less typical scenario is the use of JUnit 5. As you've demonstrated above, using JUnit 5 is possible. It's more complex than you'd like but that's almost entirely due to JUnit 5's structure and the rest of the ecosystem arguably not being JUnit 5-ready just yet.

The only way that I can think of to simplify the situation for those who want to use JUnit 5 is to introduce another starter. That could either be two separate top-level starters, or it could be a lower-level starter that's swapped in much as we do with spring-boot-starter-undertow. No matter which option we choose, we've added complexity for users who are happily using JUnit 4 today. I think the question becomes one of balance. Do we think it's worth making things more complex for those who are happy with the status quo to make things easier for those who want to use JUnit 5? My opinion is that we haven't yet reached the tipping point where the extra complexity is justified.

@odrotbohm
Copy link
Member Author

I'd argue this is a chicken and egg problem. If we only provide spring-boot-starter-test, that's of course what people use. If there was a starter working with JUnit 5, wouldn't that significantly increase the chance that projects that get started on Boot 2.x would use just that? How do we even measure "JUnit 5-ready"? Currently there's not even a way for us to track that, is there?

It feels like the team is not eager to add support for JUnit 5 right now. Let's use this ticket to see whether other parties are interested and gather further feedback.

@wilkinsona
Copy link
Member

How do we even measure "JUnit 5-ready"? Currently there's not even a way for us to track that, is there?

I think there is. It's things like IDE support (largely there now, but will require adoption of new IDE versions) and build system support (likewise), in addition to things like Hamcrest not assuming the use of JUnit 4.

@asarkar
Copy link

asarkar commented Oct 8, 2018

I’ve come across the same problem (Boot lacking OOTB JUnit 5 support), with one difference. I used AssertJ, not Hamcrest, so I think not automatically bringing in Hamcrest and leaving that choice to the user might be a good thing. The way I see it, neither Hamcrest nor AssertJ are necessary to write the tests; in fact, JUnit 5 has some new assertions by itself

@marcphilipp
Copy link

Regarding Hamcrest: I consider hamcrest-junit an experiment that never quite took off. Thus, depending on hamcrest-core (which includes MatcherAssert.assertThat()) should be enough and simpler because it does not depend on JUnit.

@filiphr
Copy link
Contributor

filiphr commented Oct 8, 2018

It may be the current version as in the latest, but it isn't the most widely used, even for greenfield development. I'm pretty sure that honour still belongs to JUnit 4.

I am not entirely certain that the honour belongs to JUnit 4. All recent projects that I have recently started are on JUnit 5 (with all the hoops one needs to do to support that in Spring Boot).

I would agree with @olivergierke that having a spring-boot-starter-junit5-test (name not really important) would make it really easy for people to use Spring Boot and JUnit 5. At least we won't need to add mockito, params and the engine as a dependency. There are already 3 different starters for web (tomcat, jetty and undertow), so why not do the same here.

I think there is. It's things like IDE support (largely there now, but will require adoption of new IDE versions) and build system support (likewise), in addition to things like Hamcrest not assuming the use of JUnit 4.

How can you measure JUnit 5 adoption for Spring Boot? The IDE support is already there and I think that users are already on the new IDE versions. IntelliJ has support since 2016.2 which is more than 2 years now and eclipse since Oxygen 1.a which is more than a year now. Since the 2.22.0 version of the maven-surefire-plugin there is ootb support for JUnit 5 in maven, and Gradle has native support since 4.6.

What exactly is the complexity for the Spring Boot team for this?

@philwebb
Copy link
Member

philwebb commented Oct 9, 2018

I'm personally not keen to add an additional JUnit 5 starter because I think at some point (probably 2.2) we'll be looking to upgrade the existing test starter to JUnit 5 (probably whilst also trying to support JUnit 4 based tests). Once we do that, it would be nice if we didn't also have the additional job of deprecating a spring-boot-starter-jupiter dependency.

Looking again at the original issues that @olivergierke raised, he wants to add the following:

junit-jupiter-params

I'm quite surprised this one is a module at all, given that it appears to be very commonly used. I wonder if the JUnit team have considered publishing a POM themselves that pulls in junit-jupiter-api, junit-jupiter-engine and junit-jupiter-params. It seems like this trio is a supper common combination (perhaps @sbrannen knows)

org.mockito:mockito-junit-jupiter

This jar is super light so we could add it to the existing starter and exclude junit-jupiter-api.

org.hamcrest:hamcrest-junit

As @marcphilipp mentioned above, using MatcherAssert from hamcrest-core is a better option here and I don't think we will add a hamcrest-junit managed dependency. I also tend to recommend AssertJ these days if asked.

@philwebb
Copy link
Member

philwebb commented Oct 9, 2018

I've raised junit-team/junit5#1629. If that's accepted and we pull in mockito-junit-jupiter most of the pain would go away.

@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Oct 9, 2018
@sormuras
Copy link

sormuras commented Oct 9, 2018

junit-jupiter-params

See junit-team/junit5#858 for why we didn't merge junit-jupiter-params into junit-jupiter-api. In short: it's an optional API which is still considered being experimental.

junit-jupiter-engine

  • Test compile time dependency: junit-jupiter-api (and junit-jupiter-params, if you want it)
  • Test runtime time dependency: junit-jupiter-engine

Maven doesn't provide such scopes ootb, and Surefire is not smart enough, yet, to auto-resolve the matching engine here. This junit-platform-maven-plugin does already do the auto-resolve "magic".

See this picture for an architecture overview.

@wilkinsona
Copy link
Member

For those reacting with a 👍, can you please comment with suggestions on how to deal with the added complexity? We need to consider both the desire to use JUnit 5 and a desire not to make things harder or more complex for those who want to stick with JUnit 4. We also need to factor in Phil's concern above. Without an answer to those, I can't see a way forward other than waiting until we flip over to JUnit 5 by default.

@wilkinsona
Copy link
Member

What exactly is the complexity for the Spring Boot team for this?

It's not really complexity for the Boot team but complexity for users. For example, if we offered spring-boot-starter-test and spring-boot-starter-test-junit5, a user now needs to be pick between the two. If they want to choose JUnit 4, it's not immediately obvious which one to use. That's a step backwards for JUnit 4 users. There's then also the extra complexity of migrating from a new JUnit 5-specific starter back onto spring-boot-starter-test once it's using JUnit 5 by default. This is the concern that Phil raised above.

If moving spring-boot-starter-test to JUnit 5 is a realistic option in the Spring Boot 2.2 time frame (roughly mid-2019), adding a JUnit 5-specific starter at this late stage of 2.1 only for it to become redundant in 2.2 is quite hard to justify. It's made harder by it being quite late in the 2.1 cycle so time for feedback is limited. I'm leaning towards doing nothing in 2.1 and then moving spring-boot-stater-test to JUnit 5 early in 2.2 so that we have plenty of time for feedback and to learn if it's working for enough people to be the new default.

@sormuras
Copy link

sormuras commented Oct 9, 2018

I'm leaning towards doing nothing in 2.1 and then moving spring-boot-stater-test to JUnit 5 early in 2.2 so that we have plenty of time for feedback and to learn if it's working for enough people to be the new default.

👍

Nice side-effect: it gives build tool authors more time to improve their JUnit 5 integration.

@filiphr
Copy link
Contributor

filiphr commented Oct 9, 2018

@philwebb said:

I'm personally not keen to add an additional JUnit 5 starter because I think at some point (probably 2.2) we'll be looking to upgrade the existing test starter to JUnit 5 (probably whilst also trying to support JUnit 4 based tests).

If you are considering adding JUnit 5 test starter. I would definitely not mix it with JUnit 4. This will also lead to people needing to exclude JUnit 4. If people want to use JUnit 4 (with JUnit 5) they just need to pull the junit-vintage-engine dependency.

Regarding the concern for the extra complexity for users. I don't think it would be confusing. JUnit 4 has been the default engine in spring-boot-starter-test from the start. So early adopters can pick to use the new spring-boot-starter-test-junit5 if they choose to do so.

Regarding the 2.2 if you want to switch the default then you will write that in the migration notes and you can add spring-boot-starter-test-junit4 for people that still want to use JUnit 4. They will have to change dependency. As for deprecating the spring-boot-starter-test-junit5 you can use Maven relocation, not sure how this would work for Gradle.

@odrotbohm
Copy link
Member Author

It's not really complexity for the Boot team but complexity for users. For example, if we offered spring-boot-starter-test and spring-boot-starter-test-junit5, a user now needs to be pick between the two. If they want to choose JUnit 4, it's not immediately obvious which one to use. That's a step backwards for JUnit 4 users.

What if a spring-boot-starter-test-junit4 was created and would either delegate to …-test or get delegated to by it? Current users wouldn't even have to bother, as they could just use the dependencies they've always used. People explicitly wanting to lock into JUnit 4 could do that, JUnit 5 users, could do that, too. We could even deprecate …-test. It's not the first time we would've removed a starter or changed its name in a minor release (I am not even suggesting we do that for 2.1 or 2.2). …-test is a bit of leaky abstraction anyway, as you now realize, you can't really change anything about it without the risk of breaking anything. I'd very much prefer to have a dedicated …-test-junit5 in my project, as it makes obvious what fundamental tech stack my tests run on. It's basically in line with …-data-jpa, …-data-mongodb etc. except the testing space was dominated by JUnit 4 before, which lead to the creation of that canoical …-test starter. Maybe it's time to adapt our setup to the situation that we have a second significant "competitor", just like we have support for different template engines?

There's then also the extra complexity of migrating from a new JUnit 5-specific starter back onto spring-boot-starter-test once it's using JUnit 5 by default. This is the concern that Phil raised above.

If moving spring-boot-starter-test to JUnit 5 is a realistic option in the Spring Boot 2.2 time frame (roughly mid-2019), adding a JUnit 5-specific starter at this late stage of 2.1 only for it to become redundant in 2.2 is quite hard to justify. It's made harder by it being quite late in the 2.1 cycle so time for feedback is limited. I'm leaning towards doing nothing in 2.1 and then moving spring-boot-stater-test to JUnit 5 early in 2.2 so that we have plenty of time for feedback and to learn if it's working for enough people to be the new default.

Are you saying you consider to upgrade the …-test starter to a new major version of JUnit in a minor Boot release. That comes as a surprise as it basically means that people will have to rewrite or at least reorganize (fix imports etc.) their tests significantly when upgrading a minor Boot version.

If we accept your statement that JUnit 4 is still the predominant version used in Boot projects, why would that change before Boot has more first-class support for 5? Why produce that cliff for ourselves if we could just switch to support both generations (effectively two different testing frameworks) so that users can switch at their own pace and us being able to deprecate and phase out the JUnit 4 support at some point.

@odrotbohm
Copy link
Member Author

odrotbohm commented Oct 9, 2018

I wanted to leave one important note: The list of dependencies I listed was caused by very specific upgrade requirements: I wanted to get a JUnit 4, Hamcrest based project to run on JUnit 5. I accepted a few package import changes but anything but that. I am not suggesting that this is proposed content for a new starter. I am also not suggesting to move …-test to JUnit 5.

IMO, the new starter should only contain dependencies that you'd use in an idiomatic JUnit 5 setup. Just like …-test has now an opinion on that, the new starter should have as well. While I generally like the idea of moving bits of that responsibility back to JUnit, unlike @wilkinsona and @philwebb I don't think that's actually solving the problem. JUnit 5 is heavily designed around the notion of extensions and we will see a lot of implementations that become very popular outside of the core JUnit project. If JUnit started to create gathering POMs for those that'd create a cyclic project dependency.

@wilkinsona
Copy link
Member

Are you saying you consider to upgrade the …-test starter to a new major version of JUnit in a minor Boot release. That comes as a surprise as it basically means that people will have to rewrite or at least reorganize (fix imports etc.) their tests significantly when upgrading a minor Boot version.

No. That's not the proposal. This is what @philwebb said above:

I think at some point (probably 2.2) we'll be looking to upgrade the existing test starter to JUnit 5 (probably whilst also trying to support JUnit 4 based tests)

Perhaps that's not possible. We don't know yet, but we need to know the answer before anything can be done here.

I wanted to leave one important note: The list of dependencies I listed was caused by very specific upgrade requirements: I wanted to get a JUnit 4, Hamcrest based project to run on JUnit 5.

Unfortunately, that makes it a poor example for the benefits that the starter would bring as it overstates the benefits. With Spring Boot 2.0.x, I can write and run JUnit tests in Eclipse (Photon) with the following dependencies in my project's pom:

<dependency>
	<groupId>org.springframework.boot</groupId>
	<artifactId>spring-boot-starter-test</artifactId>
	<exclusions>
		<exclusion>
			<groupId>junit</groupId>
			<artifactId>junit</artifactId>
		</exclusion>
	</exclusions>
	<scope>test</scope>
</dependency>
<dependency>
	<groupId>org.junit.jupiter</groupId>
	<artifactId>junit-jupiter-engine</artifactId>
	<scope>test</scope>
</dependency>

@odrotbohm
Copy link
Member Author

Yes, you can. However you wouldn't get the same rough featureset you got on JUnit 4. It's mostly the parameterized tests extension and Mockito. And yes, if I find myself missing these annotations I can go browse the JUnit documentation, find the pointers to the artifacts and add those artifacts to my POM. That's still significantly more work than just using a dedicated starter that brings those around. To summarize it's:

<dependency>
  <groupId>org.springframework.boot</groupId>
  <artifactId>spring-boot-starter-test</artifactId>
  <scope>test</scope>
  <exclusions>
    <exclusion>
      <groupId>junit</groupId>
      <artifactId>junit</artifactId>
    </exclusion>
  </exclusions>
</dependency>

<dependency>
  <groupId>org.junit.jupiter</groupId>
  <artifactId>junit-jupiter-engine</artifactId>
  <scope>test</scope>
</dependency>

<dependency>
  <groupId>org.junit.jupiter</groupId>
  <artifactId>junit-jupiter-params</artifactId>
  <scope>test</scope>
</dependency>

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

VS.

<dependency>
  <groupId>org.springframework.boot</groupId>
  <artifactId>spring-boot-starter-test-…</artifactId>
  <scope>test</scope>
</dependency>

@philwebb
Copy link
Member

philwebb commented Oct 9, 2018

I'm taking an executive decision here that we're not going to add a JUnit 5 starter at this time. I've raised #14736 instead to look at what's going to be involved with upgrading applications. I think we need to answer that question before we can consider anything else. I've also labeled a few related issues with theme: testing to help us track things.

Thanks for the input that everyone has given on this issue, it will be very helpful when we look at #14736. Depending on the outcome of #14736 we may also end up reopening this one, but I really want to answer the upgrade question before jumping into a new starter.

@philwebb philwebb closed this as completed Oct 9, 2018
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed for: team-attention An issue we'd like other members of the team to review labels Oct 9, 2018
@nightswimmings
Copy link

nightswimmings commented Dec 1, 2019

IMO, the new versions of starter should get rid of JUnit4. For a user that migrates to newer versions of Boot, there is an expectation on having to update/fix things, but the newcomer wants the latest, not the vintage, transparently. Anyway, the Spring Boot ecosystem mostly consists of developers eager to embrace change, we should aim for the ultimate standard in doubt. Just my 2c as user

@sergey-morenets
Copy link

@nightswimmings Yes, it's odd to me as well that Spring Boot Starter Test 2.3.0 still contains JUnit 4 dependency

@philwebb
Copy link
Member

I've opened #21625 to consider when we should remove JUnit 4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests