issue with junit 5 integration

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

issue with junit 5 integration

Romain Manni-Bucau
Hi guys,

Jira seems down so sending a mail.

I wanted to upgrade Meecrowave to JUnit 5.3 since it is out but I realized
the way surefire provider was developped for JUnit 5 was forcing the
junit-platform-engine even adding it manually in the test dependencies or
plugin dependencies.

Side note: I didn't investigated other providers but I guess it is the
exact same but the API breakage are happening less often.

I therefore created a PR to fix that ->
https://github.com/apache/maven-surefire/pull/193

Note: I didn't upgrade the JUnit 5 version in the same release but it
should probably be done too in another commit/PR.

I wonder if you have release plans which could include this. In terms of
issues I have in mind the other thing about JUnit 5 which would be great to
add is the support for display names instead of using the class+test names
in the logs and reports but this is less mandatory than previous one which
fails with a NoSuchMethod error when using vintage engine.

Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
Reply | Threaded
Open this post in threaded view
|

Re: issue with junit 5 integration

Romain Manni-Bucau
jupiter ;)

junit 5 is not jupiter but platform+ engine*s* so it must detect the full
stack and not just the default. A first step can be to detect
platform+jupiter+engine but I guess we will get spock, cucumber etc engine
quickly so being generic can be worth it. In my case I have vintage-engine
- cause i have junit4 and junit5 extensions and it is broken cause vintage
uses platform method of the 1.3.0 and the surefire provider forces 1.2.0.

In my proposal, the project dependencies (likely scope test on user side or
compile for engine/extension writers) is used and the plugin can
override/force some dependencies if needed. Alternative surefire could get
a specific config for that, not sure it does worth it.

Hope it is clearer.

Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>


Le mer. 5 sept. 2018 à 07:41, Dan Tran <[hidden email]> a écrit :

> Odd, I am under impression  surefire auto detect  junit-jupiter-engine  at
> runtime
>
> am I missing something?
>
> -D
>
> On Tue, Sep 4, 2018 at 10:08 PM Romain Manni-Bucau <[hidden email]>
> wrote:
>
> > It does _NOT_ work and as mentionned you can test on meecrowave trying to
> > upgrade the version in junit module.
> >
> > The test classpath build ignores project and plugin dependencies. It
> faked
> > working until 5.3.0-RC1 cause no breakage was visible.
> >
> > When testing, dont forget to use jupiter, vintage engines + platform
> stack,
> > otherwise code is compatible with 1.2 which gets loaded. The doc ignores
> > the platform and vintage engine and guess it was not tested at all
> checking
> > the code ;). Not a big deal since it is a "first" release but we should
> be
> > ablz to get it fixed quickly.
> >
> >
> > Le mar. 4 sept. 2018 23:51, Olivier Lamy <[hidden email]> a écrit :
> >
> > > Ok perso I don't mind (it just need to be documented)
> > > But the issue is: users are used to simply upgrade their junit
> > dependency.
> > >
> > >
> > > On Wed, 5 Sep 2018 at 07:37, Christian Stein <[hidden email]>
> wrote:
> > >
> > > > No, it works with Surefire 2.22.0 and JUnit 5.3.
> > > >
> > > > Just add (or move) the test-runtime dependencies to the Surefire
> plugin
> > > > element:
> > > >
> > > > <build>
> > > >    <plugins>
> > > >       <!-- JUnit 5 requires Surefire version 2.22.0 or higher -->
> > > >       <plugin>
> > > >          <artifactId>maven-surefire-plugin</artifactId>
> > > >          <version>2.22.0</version>
> > > >          <dependencies>
> > > >                    <dependency>
> > > >                        <groupId>org.junit.jupiter</groupId>
> > > >                        <artifactId>junit-jupiter-engine</artifactId>
> > > >                        <version>5.3.0</version>
> > > >                    </dependency>
> > > >          </dependencies>
> > > >       </plugin>
> > > >    </plugins>
> > > > </build>
> > > >
> > > >
> > > > Just checking our sample builds over at JUnit 5. Which do _NOT_ do
> this
> > > at
> > > > the moment.
> > > >
> > > >
> > > >
> > > > On Tue, Sep 4, 2018 at 11:32 PM Olivier Lamy <[hidden email]>
> wrote:
> > > >
> > > > > Hi
> > > > > very embarrassing issue which probably worth a quick release!
> > > > > Can you create a jira?
> > > > > As junit 5.3.0 has just been released,  I might be happy to cut
> > 2.22.1
> > > > very
> > > > > quickly with only this fix.
> > > > > others wdyt?
> > > > >
> > > > >
> > > > > On Wed, 5 Sep 2018 at 06:46, Romain Manni-Bucau <
> > [hidden email]
> > > >
> > > > > wrote:
> > > > >
> > > > > > Hi guys,
> > > > > >
> > > > > > Jira seems down so sending a mail.
> > > > > >
> > > > > > I wanted to upgrade Meecrowave to JUnit 5.3 since it is out but I
> > > > > realized
> > > > > > the way surefire provider was developped for JUnit 5 was forcing
> > the
> > > > > > junit-platform-engine even adding it manually in the test
> > > dependencies
> > > > or
> > > > > > plugin dependencies.
> > > > > >
> > > > > > Side note: I didn't investigated other providers but I guess it
> is
> > > the
> > > > > > exact same but the API breakage are happening less often.
> > > > > >
> > > > > > I therefore created a PR to fix that ->
> > > > > > https://github.com/apache/maven-surefire/pull/193
> > > > > >
> > > > > > Note: I didn't upgrade the JUnit 5 version in the same release
> but
> > it
> > > > > > should probably be done too in another commit/PR.
> > > > > >
> > > > > > I wonder if you have release plans which could include this. In
> > terms
> > > > of
> > > > > > issues I have in mind the other thing about JUnit 5 which would
> be
> > > > great
> > > > > to
> > > > > > add is the support for display names instead of using the
> > class+test
> > > > > names
> > > > > > in the logs and reports but this is less mandatory than previous
> > one
> > > > > which
> > > > > > fails with a NoSuchMethod error when using vintage engine.
> > > > > >
> > > > > > Romain Manni-Bucau
> > > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > > > <http://rmannibucau.wordpress.com> | Github <
> > > > > > https://github.com/rmannibucau> |
> > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > > > > > <
> > > > > >
> > > > >
> > > >
> > >
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Olivier Lamy
> > > > > http://twitter.com/olamy | http://linkedin.com/in/olamy
> > > > >
> > > >
> > >
> > >
> > > --
> > > Olivier Lamy
> > > http://twitter.com/olamy | http://linkedin.com/in/olamy
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: issue with junit 5 integration

Romain Manni-Bucau
Me too but
https://github.com/apache/maven-surefire/blob/d88ce541f3ba78a12422bdfa35c98cfb5783f9ea/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/SurefireDependencyResolver.java#L142
only handles a single artifact whereas junit5 relies on N > 1 artifacts for
its stack.

Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>


Le mer. 5 sept. 2018 à 08:23, Dan Tran <[hidden email]> a écrit :

> " surefire provider forces 1.2.0" this is not good :-)  i thought surefire
> auto detect first one available in classpath
>
> On Tue, Sep 4, 2018 at 11:06 PM Romain Manni-Bucau <[hidden email]>
> wrote:
>
> > jupiter ;)
> >
> > junit 5 is not jupiter but platform+ engine*s* so it must detect the full
> > stack and not just the default. A first step can be to detect
> > platform+jupiter+engine but I guess we will get spock, cucumber etc
> engine
> > quickly so being generic can be worth it. In my case I have
> vintage-engine
> > - cause i have junit4 and junit5 extensions and it is broken cause
> vintage
> > uses platform method of the 1.3.0 and the surefire provider forces 1.2.0.
> >
> > In my proposal, the project dependencies (likely scope test on user side
> or
> > compile for engine/extension writers) is used and the plugin can
> > override/force some dependencies if needed. Alternative surefire could
> get
> > a specific config for that, not sure it does worth it.
> >
> > Hope it is clearer.
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <https://rmannibucau.metawerx.net/> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <
> > https://github.com/rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > <
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > >
> >
> >
> > Le mer. 5 sept. 2018 à 07:41, Dan Tran <[hidden email]> a écrit :
> >
> > > Odd, I am under impression  surefire auto detect  junit-jupiter-engine
> > at
> > > runtime
> > >
> > > am I missing something?
> > >
> > > -D
> > >
> > > On Tue, Sep 4, 2018 at 10:08 PM Romain Manni-Bucau <
> > [hidden email]>
> > > wrote:
> > >
> > > > It does _NOT_ work and as mentionned you can test on meecrowave
> trying
> > to
> > > > upgrade the version in junit module.
> > > >
> > > > The test classpath build ignores project and plugin dependencies. It
> > > faked
> > > > working until 5.3.0-RC1 cause no breakage was visible.
> > > >
> > > > When testing, dont forget to use jupiter, vintage engines + platform
> > > stack,
> > > > otherwise code is compatible with 1.2 which gets loaded. The doc
> > ignores
> > > > the platform and vintage engine and guess it was not tested at all
> > > checking
> > > > the code ;). Not a big deal since it is a "first" release but we
> should
> > > be
> > > > ablz to get it fixed quickly.
> > > >
> > > >
> > > > Le mar. 4 sept. 2018 23:51, Olivier Lamy <[hidden email]> a écrit
> :
> > > >
> > > > > Ok perso I don't mind (it just need to be documented)
> > > > > But the issue is: users are used to simply upgrade their junit
> > > > dependency.
> > > > >
> > > > >
> > > > > On Wed, 5 Sep 2018 at 07:37, Christian Stein <[hidden email]>
> > > wrote:
> > > > >
> > > > > > No, it works with Surefire 2.22.0 and JUnit 5.3.
> > > > > >
> > > > > > Just add (or move) the test-runtime dependencies to the Surefire
> > > plugin
> > > > > > element:
> > > > > >
> > > > > > <build>
> > > > > >    <plugins>
> > > > > >       <!-- JUnit 5 requires Surefire version 2.22.0 or higher -->
> > > > > >       <plugin>
> > > > > >          <artifactId>maven-surefire-plugin</artifactId>
> > > > > >          <version>2.22.0</version>
> > > > > >          <dependencies>
> > > > > >                    <dependency>
> > > > > >                        <groupId>org.junit.jupiter</groupId>
> > > > > >
> > <artifactId>junit-jupiter-engine</artifactId>
> > > > > >                        <version>5.3.0</version>
> > > > > >                    </dependency>
> > > > > >          </dependencies>
> > > > > >       </plugin>
> > > > > >    </plugins>
> > > > > > </build>
> > > > > >
> > > > > >
> > > > > > Just checking our sample builds over at JUnit 5. Which do _NOT_
> do
> > > this
> > > > > at
> > > > > > the moment.
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Tue, Sep 4, 2018 at 11:32 PM Olivier Lamy <[hidden email]>
> > > wrote:
> > > > > >
> > > > > > > Hi
> > > > > > > very embarrassing issue which probably worth a quick release!
> > > > > > > Can you create a jira?
> > > > > > > As junit 5.3.0 has just been released,  I might be happy to cut
> > > > 2.22.1
> > > > > > very
> > > > > > > quickly with only this fix.
> > > > > > > others wdyt?
> > > > > > >
> > > > > > >
> > > > > > > On Wed, 5 Sep 2018 at 06:46, Romain Manni-Bucau <
> > > > [hidden email]
> > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi guys,
> > > > > > > >
> > > > > > > > Jira seems down so sending a mail.
> > > > > > > >
> > > > > > > > I wanted to upgrade Meecrowave to JUnit 5.3 since it is out
> > but I
> > > > > > > realized
> > > > > > > > the way surefire provider was developped for JUnit 5 was
> > forcing
> > > > the
> > > > > > > > junit-platform-engine even adding it manually in the test
> > > > > dependencies
> > > > > > or
> > > > > > > > plugin dependencies.
> > > > > > > >
> > > > > > > > Side note: I didn't investigated other providers but I guess
> it
> > > is
> > > > > the
> > > > > > > > exact same but the API breakage are happening less often.
> > > > > > > >
> > > > > > > > I therefore created a PR to fix that ->
> > > > > > > > https://github.com/apache/maven-surefire/pull/193
> > > > > > > >
> > > > > > > > Note: I didn't upgrade the JUnit 5 version in the same
> release
> > > but
> > > > it
> > > > > > > > should probably be done too in another commit/PR.
> > > > > > > >
> > > > > > > > I wonder if you have release plans which could include this.
> In
> > > > terms
> > > > > > of
> > > > > > > > issues I have in mind the other thing about JUnit 5 which
> would
> > > be
> > > > > > great
> > > > > > > to
> > > > > > > > add is the support for display names instead of using the
> > > > class+test
> > > > > > > names
> > > > > > > > in the logs and reports but this is less mandatory than
> > previous
> > > > one
> > > > > > > which
> > > > > > > > fails with a NoSuchMethod error when using vintage engine.
> > > > > > > >
> > > > > > > > Romain Manni-Bucau
> > > > > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > > > > > <http://rmannibucau.wordpress.com> | Github <
> > > > > > > > https://github.com/rmannibucau> |
> > > > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > > > > > > > <
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Olivier Lamy
> > > > > > > http://twitter.com/olamy | http://linkedin.com/in/olamy
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Olivier Lamy
> > > > > http://twitter.com/olamy | http://linkedin.com/in/olamy
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: issue with junit 5 integration

Romain Manni-Bucau
In reply to this post by Romain Manni-Bucau
@Stephane: you can clone https://github.com/apache/meecrowave/tree/trunk
and change the version in the junit pom (
https://github.com/apache/meecrowave/blob/trunk/meecrowave-junit/pom.xml),
it will fail then

Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>


Le mer. 5 sept. 2018 à 10:07, Stephane Nicoll <[hidden email]> a
écrit :

> How do I reproduce this problem concretely? Having to specify a dependency
> in surefire is not an option for us as we want the JUnit5 provider to be
> detected automatically.
>
> I've upgraded a Spring Boot project to use 5.3.0 and I don't see any
> problem.
>
> Thanks,
> S.
>
>
> On Tue, Sep 4, 2018 at 10:46 PM Romain Manni-Bucau <[hidden email]>
> wrote:
>
> > Hi guys,
> >
> > Jira seems down so sending a mail.
> >
> > I wanted to upgrade Meecrowave to JUnit 5.3 since it is out but I
> realized
> > the way surefire provider was developped for JUnit 5 was forcing the
> > junit-platform-engine even adding it manually in the test dependencies or
> > plugin dependencies.
> >
> > Side note: I didn't investigated other providers but I guess it is the
> > exact same but the API breakage are happening less often.
> >
> > I therefore created a PR to fix that ->
> > https://github.com/apache/maven-surefire/pull/193
> >
> > Note: I didn't upgrade the JUnit 5 version in the same release but it
> > should probably be done too in another commit/PR.
> >
> > I wonder if you have release plans which could include this. In terms of
> > issues I have in mind the other thing about JUnit 5 which would be great
> to
> > add is the support for display names instead of using the class+test
> names
> > in the logs and reports but this is less mandatory than previous one
> which
> > fails with a NoSuchMethod error when using vintage engine.
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <https://rmannibucau.metawerx.net/> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <
> > https://github.com/rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > <
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: issue with junit 5 integration

Romain Manni-Bucau
From what I saw in the code the surefire provider artifact is resolved to
build the classpath ignoring the project configuration (it is just an
artifact resolution) and therefore it ignores the project overrides without
my patch.

Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>


Le mer. 5 sept. 2018 à 10:14, Stephane Nicoll <[hidden email]> a
écrit :

> I am more interested to understand what is particular in that project that
> exhibits an issue we don't seem to be facing. That thread is phrased in
> such a way that it is a general problem and I'd like a confirmation of that
> as it may impact Spring Boot (amongst other things)
>
> Thanks,
> S.
>
>
> On Wed, Sep 5, 2018 at 10:13 AM Romain Manni-Bucau <[hidden email]>
> wrote:
>
> > @Stephane: you can clone https://github.com/apache/meecrowave/tree/trunk
> > and change the version in the junit pom (
> > https://github.com/apache/meecrowave/blob/trunk/meecrowave-junit/pom.xml
> ),
> > it will fail then
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <https://rmannibucau.metawerx.net/> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <
> > https://github.com/rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > <
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > >
> >
> >
> > Le mer. 5 sept. 2018 à 10:07, Stephane Nicoll <[hidden email]
> >
> > a
> > écrit :
> >
> > > How do I reproduce this problem concretely? Having to specify a
> > dependency
> > > in surefire is not an option for us as we want the JUnit5 provider to
> be
> > > detected automatically.
> > >
> > > I've upgraded a Spring Boot project to use 5.3.0 and I don't see any
> > > problem.
> > >
> > > Thanks,
> > > S.
> > >
> > >
> > > On Tue, Sep 4, 2018 at 10:46 PM Romain Manni-Bucau <
> > [hidden email]>
> > > wrote:
> > >
> > > > Hi guys,
> > > >
> > > > Jira seems down so sending a mail.
> > > >
> > > > I wanted to upgrade Meecrowave to JUnit 5.3 since it is out but I
> > > realized
> > > > the way surefire provider was developped for JUnit 5 was forcing the
> > > > junit-platform-engine even adding it manually in the test
> dependencies
> > or
> > > > plugin dependencies.
> > > >
> > > > Side note: I didn't investigated other providers but I guess it is
> the
> > > > exact same but the API breakage are happening less often.
> > > >
> > > > I therefore created a PR to fix that ->
> > > > https://github.com/apache/maven-surefire/pull/193
> > > >
> > > > Note: I didn't upgrade the JUnit 5 version in the same release but it
> > > > should probably be done too in another commit/PR.
> > > >
> > > > I wonder if you have release plans which could include this. In terms
> > of
> > > > issues I have in mind the other thing about JUnit 5 which would be
> > great
> > > to
> > > > add is the support for display names instead of using the class+test
> > > names
> > > > in the logs and reports but this is less mandatory than previous one
> > > which
> > > > fails with a NoSuchMethod error when using vintage engine.
> > > >
> > > > Romain Manni-Bucau
> > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > <http://rmannibucau.wordpress.com> | Github <
> > > > https://github.com/rmannibucau> |
> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > > > <
> > > >
> > >
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: issue with junit 5 integration

Christian Stein
On Wed, Sep 5, 2018 at 10:22 AM Romain Manni-Bucau <[hidden email]>
wrote:

> From what I saw in the code the surefire provider artifact is resolved to
> ...


So, specifying that artifact in your setup (explicit or implicit) will lead
to the correct version
to be loaded. See the integration tests in Surefire, JUnit 5 sample
projects, Spring Boot, et al.

... therefore it ignores the project overrides without my patch.


Which is an improvement! And Surefire needs such a logic. I did something
similar here:
https://github.com/sormuras/junit-platform-maven-plugin/blob/05b7e3ae521ccb7f71d00ccd532523a99a9b6dfc/src/main/java/de/sormuras/junit/platform/maven/plugin/Resolver.java#L87-L116

So, my plan is:

1. Check meecrowave configuration if it is possible to get it running with
2.22.0 and 5.3
2. Improve Surefire