Re: issue with junit 5 integration

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

Re: issue with junit 5 integration

Olivier Lamy
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

Dan Tran
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

Tibor Digana
(Sorry, I had to re-send my email from apache.org)
Romain, that link in Resolver means that our Provider has transitive
dependencies. This Surefire's Platform Provider + it's dependencies. Not
the JUnit5's provider.
Adding engine with another version makes sense to me in current situation.
It's similar to what we are doing when we add junit:junit:4.12.
My question is whether the Engine should have scope=provided in Surefire's
Provider in the future and the user is forced to add it anyway in his POM.

On Wed, Sep 5, 2018 at 9:16 AM Romain Manni-Bucau <[hidden email]>
wrote:

> 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 Dan Tran
Le mer. 5 sept. 2018 18:20, Tibor Digana
<[hidden email]> a écrit :

> Romain, that link in Resolver means that our Provider has transitive
> dependencies. This Surefire's Platform Provider + it's dependencies. Not
> the JUnit5's provider.
>

Yes but the platform and engine are transitive, right?


Adding engine with another version makes sense to me in current situation.
> It's similar to what we are doing when we add junit:junit:4.12.
> My question is whether the Engine should have scope=provided in Surefire's
> Provider in the future and the user is forced to add it anyway in his POM.
>

I wouldnt force but ensure it is made easy or even transparent based on the
api - platform and engines are discoverable for now, can become tricky for
the platform later but let's tackle it when it arrives.


> On Wed, Sep 5, 2018 at 9:16 AM Romain Manni-Bucau <[hidden email]>
> wrote:
>
> > 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
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
>
> --
> Cheers
> Tibor
>
Reply | Threaded
Open this post in threaded view
|

Re: issue with junit 5 integration

Tibor Digana
>> Yes but the platform and engine are transitive, right?
Yes. We can grab the classpath from the forked jvm via Jconsole and we
should see these artifacts.

@Christian
what happens when you just add new Engine with another version to the POM's
dependencies or plugin's dependencies.
The old version of JUnit5 Engine is overridden?

We should decide which way is not error prone from the users perspective.
WDYT?

On Wed, Sep 5, 2018 at 6:23 PM Romain Manni-Bucau <[hidden email]>
wrote:

> Le mer. 5 sept. 2018 18:20, Tibor Digana
> <[hidden email]> a écrit :
>
> > Romain, that link in Resolver means that our Provider has transitive
> > dependencies. This Surefire's Platform Provider + it's dependencies. Not
> > the JUnit5's provider.
> >
>
> Yes but the platform and engine are transitive, right?
>
>
> Adding engine with another version makes sense to me in current situation.
> > It's similar to what we are doing when we add junit:junit:4.12.
> > My question is whether the Engine should have scope=provided in
> Surefire's
> > Provider in the future and the user is forced to add it anyway in his
> POM.
> >
>
> I wouldnt force but ensure it is made easy or even transparent based on the
> api - platform and engines are discoverable for now, can become tricky for
> the platform later but let's tackle it when it arrives.
>
>
> > On Wed, Sep 5, 2018 at 9:16 AM Romain Manni-Bucau <[hidden email]
> >
> > wrote:
> >
> > > 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
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> >
> > --
> > Cheers
> > Tibor
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: issue with junit 5 integration

Christian Stein
On Wed, Sep 5, 2018 at 6:49 PM Tibor Digana <[hidden email]> wrote:

> >> Yes but the platform and engine are transitive, right?
> Yes. We can grab the classpath from the forked jvm via Jconsole and we
> should see these artifacts.
>
> @Christian
> what happens when you just add new Engine with another version to the POM's
> dependencies or plugin's dependencies.
> The old version of JUnit5 Engine is overridden?
>
> We should decide which way is not error prone from the users perspective.
> WDYT?
>
>
What happens in Surefire today needs some investigations... I was sure
yesterday
and earlier today, but had to partly revert changes in our sample projects
to actually
force Surefire to find and execute tests. See [1] for the changes I made. I
fear that
now both versions, 1.2.0 and 1.3.0, do land on the classpath now...


[1]
https://github.com/junit-team/junit5-samples/commit/6c81972c691a7d055d895953eda5cefead8dc541


There are two debug statements from Surefire I don't understand. They look
like:

...
[DEBUG] test(compact) classpath:
  test-classes
  classes
  junit-jupiter-api-5.3.0.jar
  apiguardian-api-1.0.0.jar
  opentest4j-1.1.0.jar
  junit-platform-commons-1.3.0.jar
  junit-jupiter-params-5.3.0.jar
  junit-jupiter-engine-5.3.0.jar
  junit-platform-engine-1.3.0.jar
----...
T E S T S
----...
[DEBUG] boot(compact) classpath:
  surefire-booter-2.22.0.jar
  surefire-api-2.22.0.jar
  surefire-logger-api-2.22.0.jar
  test-classes
  classes
  junit-jupiter-api-5.3.0.jar
  apiguardian-api-1.0.0.jar
  opentest4j-1.1.0.jar
  junit-platform-commons-1.3.0.jar
  junit-jupiter-params-5.3.0.jar
  junit-jupiter-engine-5.3.0.jar
  junit-platform-engine-1.3.0.jar
  surefire-junit-platform-2.22.0.jar
  junit-platform-launcher-1.2.0.jar
  junit-platform-engine-1.2.0.jar
  junit-platform-commons-1.2.0.jar
...

The first one "test" looks good, the second one "boot" doesn't...

You may reproduce it by cloning "
https://github.com/junit-team/junit5-samples"
and running "mvn clean test -X" in "junit5-jupiter-starter-maven".

Now to what should happen:

1. User-defined artifacts always win.
2. Platform and Engines are different products a need to agree on a minimal
Platform version.
3. Platform versions are backwards compatible.
4. The highest Platform version "in the stack" wins.
5. The presence of an API may imply an associated Engine artifact and
version. (Jupiter API -> 99% Jupiter Engine)
6. The presence of an API may imply running a Surefire provider -- but also
could
    handled by the JUnit Platform (provider). E.g. seeing TestNG artifact
could mean
    that the user wants the JUnit Platform with TestNGine [2] to find and
run tests.

The list is far from complete.

Cheers,
Christian

[2]