Cannot use this plugin in tandem with Kotlin Multiplatform Tests #116

Activity

1. eriwen

   added

   bugSomething isn't workingSomething isn't working

   on Sep 28, 2021
2. abelkov commented on Oct 8, 2021

   

   abelkov

   on Oct 8, 2021

   More actions

   Mirror issue on Kotlin side: https://youtrack.jetbrains.com/issue/KT-49155
3. pshevche

   added

   enhancementNew feature or requestNew feature or request

   and removed

   bugSomething isn't workingSomething isn't working

   on Apr 24, 2024
4. osipxd commented on Oct 11, 2024

   

   osipxd

   on Oct 11, 2024

   Last edited by osipxd

   More actions

   Found a workaround:

   tasks.withType<KotlinJvmTest>().configureEach {
       applyTestRetryCompatibilityWorkaround()
   }
   
   /**
    * Test retry plugin is incompatible with test tasks that override the `createTestExecuter` method.
    * This includes the [KotlinJvmTest] task which wraps the test executor with its own wrapper.
    *
    * This workaround heavily relies on the internal implementation details of the test-retry plugin and KGP.
    *
    * The test retry plugin adds a `doFirst` action, which:
    * - Retrieves the test executer using `createTestExecuter` (KGP returns wrapped test executer here)
    * - Wraps it with `RetryTestExecuter`
    * - Sets the executer using `setTestExecuter`
    *
    * In the `doLast` action, it expects that `createTestExecuter` returns the previously created `RetryTestExecuter` instance.
    * However, KGP wraps every result of `createTestExecutor` with its own wrapper, resulting in the following nesting:
    *   KotlinJvmTarget$Executer(RetryTestExecuter(KotlinJvmTarget$Executer(DefaultTestExecuter)))
    *
    * KGP wraps the executer only if `targetName` is present, as it is needed to add the target name suffix to the test name.
    * The workaround sets `targetName` to `null` after the first KGP wrapper is created,
    * so `createTestExecuter` returns the previously created executer:
    *   RetryTestExecuter(KotlinJvmTarget$Executer(DefaultTestExecuter))
    *
    * Issue: https://github.com/gradle/test-retry-gradle-plugin/issues/116 (KT-49155)
    */
   private fun KotlinJvmTest.applyTestRetryCompatibilityWorkaround() {
       if (targetName == null) return
   
       val executeTestsActionIndex = taskActions.indexOfLast { it.displayName == "Execute executeTests" }
       check(executeTestsActionIndex != -1) { "Action executeTests not found" }
   
       // Add the workaround action and then move it to the correct position right before tests execution.
       doFirst("workaround for compatibility with testRetry") {
           targetName = null
       }
       val injectedAction = taskActions.removeFirst()
       taskActions.add(executeTestsActionIndex, injectedAction)
   
       // Restore targetName value at the end as other plugins might rely on it.
       // For example, kover uses it to find test tasks by target name
       doLast("restore targetName") { targetName = originalTargetName }
   }

   I hope it doesn't break any stuff related to task inputs as targetName is marked as @Input
5. osipxd commented on Oct 14, 2024

   

   osipxd

   on Oct 14, 2024

   Last edited by osipxd

   More actions

   In my opinion, the problem is in createTestExecuter method semantics violation. Method name implies creation of a new instance on each call, while its implementation in the Test task looks like this:

   protected TestExecuter<JvmTestExecutionSpec> createTestExecuter() {
       if (testExecuter == null) {
           return new DefaultTestExecuter(...);
       } else {
           return testExecuter;
       }
   }

   What could be done on a KGP side (or any other plugin's overriding an executer) is to check if the super.createTestExecuter returns the default implementation:

   override fun createTestExecuter(): TestExecuter<JvmTestExecutionSpec> {
       val executer = super.createTestExecuter()
   
       return if (executer is DefaultTestExecuter) {
           // Wrap the executer or create a new one
       } else {
           // Executer was set by test-retry plugin, return it "as is"
           executer 
       }
   }

   Though it would be even better to resolve this problem in Gradle. Probably by introducing a new method getTestExecuter like this:

   protected final TestExecuter<JvmTestExecutionSpec> getTestExecuter() {
       if (testExecuter == null) {
           return createTestExecuter();
       } else {
           return testExecuter;
       }
   }
   
   protected TestExecuter<JvmTestExecutionSpec> createTestExecuter() {
       return new DefaultTestExecuter(...);
   }

   Third-party plugins will be able to override createTestExecuter while getTestExecuter will return the executor set by test-retry plugin. The only problem is that this solution requires changes to the Gradle codebase, so probably this issue should be moved there.