szpak / gradle-pitest-plugin

Gradle plugin for PIT Mutation Testing
http://gradle-pitest-plugin.solidsoft.info/
221 stars 58 forks source link

Auto add junit-platform-launcher to classpath #337

Closed hcoles closed 1 year ago

hcoles commented 1 year ago

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

https://github.com/hcoles/pitest/pull/1212

szpak commented 1 year ago

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).

hcoles commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Vampire commented 1 year ago

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 commented 1 year ago

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

Vampire commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

mfvanek commented 1 year ago

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

szpak commented 1 year ago

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.

szpak commented 1 year ago

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: https://github.com/szpak/gradle-pitest-plugin/pull/353#issuecomment-1728485874

szpak commented 1 year ago

Implemented in 1.15.0.