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

Auto add junit-platform-launcher to classpath #337

Closed
hcoles opened this issue May 18, 2023 · 13 comments
Closed

Auto add junit-platform-launcher to classpath #337

hcoles opened this issue May 18, 2023 · 13 comments
Labels
Milestone

Comments

@hcoles
Copy link

hcoles commented May 18, 2023

Version mismatches between the pitest-junit5-plugin and junit itself have been causing a constant stream of support requests, this will be resolved when pitest 1.14.0 is released, but will require a change to the gradle plugin.

Currently, junit-platform-launcher is included as a shaded dependency in the pitest-junit-plugin jar. This is a cludge to work around the fact that platform launcher doesn't appear on the normal test classpath for maven or for gradle. The maven surefire plugin pulls in the correct version of it automatically if it detects the junit 5 engine on the classpath, and gradle presumably does something similar.

The pitest maven plugin will start replicating the behaviour in 1.14.0, and the next release of the junit 5 plugin will therefore remove the shaded dependency.

The same functionality will be required in the gradle plugin (although the dependency could be added manually if this is painful to do).

See

hcoles/pitest#1212

@szpak
Copy link
Owner

szpak commented May 18, 2023

Do you expect to have the plugin (even) more clumsy with the new JUnit Platform versions? Or due to the more stable stable API (then internal implementation which currently can fail due to the mismatch of the shaded version and the other Platform component) it is expected to have greater compatibility?

It would be probably required to have some version checking and add the dependency only if pitest-junit5-plugin >=1.2.0 (or you plan to release 2.0.0?).

It's somehow related to some other idea to automatically take plugin version to match the JUnit Platform version used in the project provided by @nagkumar, which in general is sensible, but I don't see a good way to map the future versions of JUnit Platform to the exact versions of the plugin (without doing any runtime remote dependency resolution which would be slow, fragile and problematic).

@szpak szpak added the PIT label May 18, 2023
@hcoles
Copy link
Author

hcoles commented May 19, 2023

The junit api the plugin depends on seems to be pretty stable (the plugin compiles against platform for all versions from 1.10.0-M1 down to 1.2.0 without any change), but as we've seen there have been frequent breakages one step up (where the launcher api interacts with the rest of junit). So things ought to be much more stable with this change.

I'll release the plugin as 2.0.0 as it is a breaking change.

#334 made sense from a user point of view, but sounds like a maintainance nightmare. If the need to release the pitest junit plugin in sync with junit changes has been largely removed, hopefully the motivation behind #334 has been addressed.

@Vampire
Copy link
Contributor

Vampire commented May 19, 2023

A bit more information.
Actually, even with version 2.0.0 of the junit5 plugin, it will work as before for Gradle.
This is caused by an inconsistency with the Maven plugin that should probably be resolved alongside.
As fixing that will require this issue to be done too to continue working without user intervention, I'm not sure whether you want a separate issue or fix it together with this one.
Let me explain a bit further.

The Maven plugin only adds the plugin JAR itself to the SUT classpath, but not any of its declared dependencies.
This is for the plugin dependencies to not pollute the SUT classpath more than necessary.
Due to that, it is expected that PIT plugins shade and relocate all their dependencies into their JAR.
The platform launcher dependency is an exception, it was shaded unrelocated in the junit5 plugin as it wouldn't work properly otherwise, but then can cause the mentioned problems if the SUT uses a different platform version.
To fix this, now the Maven plugin will have junit5 specific code to add the platform launcher in the version the SUT uses for the JUnit platform.
With this change, the shaded dependency can be removed from the JUnit 5 plugin.
I assume that with that change the platform launcher dependency will appear as dependency in the junit5 plugins POM, if it is not changed to "provided".

The Gradle plugin behaves differently from the Maven plugin, in that it takes into account the declared dependencies of the PIT plugins.
This is the inconsistency that should probably be fixed to align the two integrations.
This would basically mean to make the pitest configuration non-transitive from what I remember from the Gradle plugin code.
With this change, the Gradle plugin will behave like the Maven plugin which is probably better for consistency.

With switching to non-transitive (or if the junit5 plugin declares the launcher as "provided" which would probably make sense), it would then break in Gradle as then the launcher is missing from the SUT classpath.
So after either switching to non-transitive and / or if the junit5 plugin declares the launcher as "provided" and stops shading it, this plugin needs to be resolved to stay compatible, finding the right launcher version to include to be compatible with the SUT.

@hcoles
Copy link
Author

hcoles commented May 19, 2023

@Vampire 2.0.0 will change the platform dependency to be provided, so it will not appear as a dependency.

@Vampire
Copy link
Contributor

Vampire commented May 19, 2023

Ok, yeah, then this change will be necessary anyway.
But still it would be more consistent with the Maven plugin to also switch to non-transitive. :-)

@hcoles
Copy link
Author

hcoles commented May 19, 2023

Having thought this through, I think I'll release a 1.2.0 of the junit 5 plugin with the dependency still declared. This will be breaking for maven users, and non breaking for gradle users.

Version 2.0.0 will be released with the dependency no longer declared once the gradle plugin has been updated.

Net result is

  • All versions of junit platform between 1.2.0 and 1.10.0-M1 immediately work out of the box for maven users
  • All versions of junit platform between 1.2.0 and 1.10.0-M1 work for gradle users if they manually override the platform-launcher version when it is mismatched
  • Works out of the box for everyone once gradle plugin is updated

@Vampire
Copy link
Contributor

Vampire commented May 19, 2023

Actually, depending on how the Gradle build is written, it even works already with provided too.

If you use

tasks.test {
    useJUnitPlatform()
}

dependencies {
    testImplementation("org.spockframework:spock-core:2.3-groovy-4.0")
}

it does not work.

But if you instead use

testing {
    suites {
        named<JvmTestSuite>("test") {
            useSpock("2.3-groovy-4.0")
            // or useJUnitJupiter(...) or useKotlinTest(...)
        }
    }
}

then Gradle already adds the org.junit.platform:junit-platform-launcher in the correct version to testRuntimeOnly and it works.

So the change for this issue could also just be a documentation change to require using the test suites DSL or otherwise include the launcher manually as testRuntimeOnly dependency.

@mfvanek
Copy link
Contributor

mfvanek commented May 30, 2023

Just for the history and to complete the picture...
After switching to

    junit5PluginVersion.set("1.2.0")
    pitestVersion.set("1.14.1")

I've faced with an error

10:10:43 PM PIT >> SEVERE : Coverage generator Minion exited abnormally due to UNKNOWN_ERROR
Exception in thread "main" org.pitest.util.PitError: Coverage generation minion exited abnormally! (UNKNOWN_ERROR)

due to

10:10:43 PM PIT >> FINE : MINION : Caused by: java.lang.ClassNotFoundException: org.junit.platform.launcher.core.LauncherFactory
10:10:43 PM PIT >> FINE : MINION : 	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
10:10:43 PM PIT >> FINE : MINION : 	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
10:10:43 PM PIT >> FINE : MINION : 	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)

I've fixed it with adding

    testRuntimeOnly("org.junit.platform:junit-platform-launcher") {
        because("required for pitest")
    }

@szpak
Copy link
Owner

szpak commented May 30, 2023

@mfvanek You can check also using pitest instead of testRuntimeOnly to make its availabitlity even more narrow (although, testRuntimeOnly should not break anything)

@mfvanek
Copy link
Contributor

mfvanek commented May 30, 2023

@szpak
It was the first thing I tried to do but this didn't work :(

@szpak
Copy link
Owner

szpak commented May 30, 2023

It was the first thing I tried to do but this didn't work :(

Thanks for letting me know. Adding a mechanism for junit-platform-launcher addition, I will examine it.

raoulvdberge added a commit to refinedmods/refinedarchitect that referenced this issue Aug 22, 2023
@szpak szpak added this to the 1.14.0 milestone Sep 1, 2023
szpak added a commit that referenced this issue Sep 20, 2023
... for JUnit Platform projects to avoid "UNKNOWN_ERROR" or:
"NoClassDefFoundError: org.junit.platform.launcher.core.LauncherFactory" with PIT 1.14.0+
(with pitest-junit-plugin 1.2.0+).

That dependency is no longer shared. More details:
#337
szpak added a commit that referenced this issue Sep 20, 2023
... for JUnit Platform projects to avoid "UNKNOWN_ERROR" or:
"NoClassDefFoundError: org.junit.platform.launcher.core.LauncherFactory" with PIT 1.14.0+
(with pitest-junit-plugin 1.2.0+).

That dependency is no longer shaded. More details:
#337
@szpak
Copy link
Owner

szpak commented Sep 20, 2023

For people facing this problem in real projects. You may it it a try with the new SNAPSHOT version trying to fix the problem (in functional tests it works). For details: #353 (comment)

@szpak
Copy link
Owner

szpak commented Sep 28, 2023

Implemented in 1.15.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants