fix: Ensure parallel rules aren't bypassed on retries (#2404)

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 Please see [CONTRIBUTING.md](./CONTRIBUTING.md) on how to contribute to Cucumber.
 
 ## [Unreleased]
+- Ensure that parallel workers remain in-progress during retries. [#2404](https://github.com/cucumber/cucumber-js/pull/2404) 
 
 ## [10.6.0] - 2024-04-25
 ### Added

features/parallel.feature

@@ -77,3 +77,52 @@ Feature: Running scenarios in parallel
     When I run cucumber-js with `--parallel 2`
     Then it passes
     And `testCaseStarted` envelope has `workerId`
+
+  Scenario: running in parallel respects `parallelCanAssign` rules on retried scenarios
+    Given a file named "features/step_definitions/cucumber_steps.js" with:
+      """
+      const {When, setParallelCanAssign} = require('@cucumber/cucumber')
+
+      When(/^I wait slowly$/, function(callback) {
+        setTimeout(callback, 200 * 2)
+      })
+      
+      When(/^I wait quickly$/, function(callback) {
+        setTimeout(callback, 200 * 1)
+      })
+
+      let counter = 0;
+      When(/^I fail and have to retry$/, function() {
+        counter += 1;
+        if (counter < 4) {
+          throw new Error('Failed as expected')
+        }
+      })
+
+      setParallelCanAssign(function(pickleInQuestion, picklesInProgress) {
+        const runningCount = picklesInProgress.length;
+        const picklesInProgressAllInParallel = picklesInProgress.every(p => p.tags.find(t => t.name === '@parallel') !== undefined);
+        const shouldRunInParallel = pickleInQuestion.tags.find((t) => t.name === '@parallel') !== undefined;
+
+        return ((!shouldRunInParallel && runningCount < 1) || shouldRunInParallel) && picklesInProgressAllInParallel;
+      })
+      """
+    And a file named "features/a.feature" with:
+      """
+      Feature: Testing parallelism with retries
+        @parallel
+        Scenario: fail_parallel
+          When I wait quickly
+          And I fail and have to retry
+
+        @parallel
+        Scenario: pass_parallel
+          When I wait slowly
+        
+        Scenario: pass_sync
+          When I wait quickly
+      """
+    When I run cucumber-js with `--parallel 2 --retry 3`
+    Then it passes
+    And the first two scenarios run in parallel while the last runs sequentially
+    And the scenario 'fail_parallel' retried 3 times

features/step_definitions/parallel_steps.ts

@@ -38,6 +38,54 @@ function getSetsOfPicklesRunningAtTheSameTime(
   return result
 }
 
+/**
+ * Returns any failed {@link message.TestCaseFinished} events that failed and will be retried.
+ * @param envelopes The total envelopes for the run.
+ * @param scenarioName The name of the scenario to gether events for.
+ * @returns The events that indicate a particular step failed and was retried.
+ */
+function getRetriesForScenario(
+  envelopes: messages.Envelope[],
+  scenarioName: string
+) {
+  const scenarioEnvelope = envelopes.find(
+    (envelope) => envelope.pickle?.name === scenarioName
+  )
+
+  if (scenarioEnvelope === undefined) {
+    throw new Error('Could not find scenario: ' + scenarioEnvelope)
+  }
+
+  const scenarioPickle = scenarioEnvelope.pickle!
+  const testCase = envelopes.find(
+    (env) => env.testCase?.pickleId === scenarioPickle.id
+  )?.testCase
+
+  if (testCase === undefined) {
+    throw new Error('Could not find test case for scenario: ' + scenarioName)
+  }
+
+  const scenarioCasesStarted = envelopes.filter(
+    (envelope) => envelope.testCaseStarted?.testCaseId === testCase.id
+  )
+  const testStartedIds = scenarioCasesStarted.map(
+    (started) => started.testCaseStarted.id
+  )
+  const scenarioCasesFinished = envelopes.filter((envelope) => {
+    if (envelope.testCaseFinished?.testCaseStartedId) {
+      return testStartedIds.includes(
+        envelope.testCaseFinished.testCaseStartedId
+      )
+    }
+    return false
+  })
+
+  return scenarioCasesFinished.filter(
+    (testCaseFinished) =>
+      testCaseFinished.testCaseFinished.willBeRetried === true
+  )
+}
+
 Then('no pickles run at the same time', function (this: World) {
   const actualSets = getSetsOfPicklesRunningAtTheSameTime(
     this.lastRun.envelopes
@@ -63,3 +111,22 @@ Then('`testCaseStarted` envelope has `workerId`', function (this: World) {
 
   expect(testCaseStartedEnvelope.testCaseStarted).to.ownProperty('workerId')
 })
+
+Then(
+  'the scenario {string} retried {int} times',
+  function (this: World, scenarioName: string, retryCount: number) {
+    const retried = getRetriesForScenario(this.lastRun.envelopes, scenarioName)
+    expect(retried).to.have.lengthOf(retryCount)
+  }
+)
+
+Then(
+  'the first two scenarios run in parallel while the last runs sequentially',
+  function (this: World) {
+    const sets = getSetsOfPicklesRunningAtTheSameTime(this.lastRun.envelopes)
+
+    expect(Array.from(new Set(sets).values())).to.eql([
+      'fail_parallel, pass_parallel',
+    ])
+  }
+)

package.json

@@ -164,6 +164,7 @@
     "Tom V <tom@toc.com>",
     "Tomer Ben-Rachel <tomerpacific@gmail.com>",
     "Tristan Dunn <tristanzdunn@gmail.com>",
+    "Tristan Zander <tristan.zander@ncino.com>",
     "unknown <jharlin@NormanDev2.telogical.com>",
     "Valerio Innocenti Sedili <valerio.innocenti.ext@yoox.com>",
     "Vasily Shelkov <vasilydshelkov@gmail.com>",

src/runtime/parallel/coordinator.ts

@@ -98,8 +98,7 @@ export default class Coordinator implements IRuntime {
       const envelope = message.jsonEnvelope
       this.eventBroadcaster.emit('envelope', envelope)
       if (doesHaveValue(envelope.testCaseFinished)) {
-        delete this.inProgressPickles[worker.id]
-        this.parseTestCaseResult(envelope.testCaseFinished)
+        this.parseTestCaseResult(envelope.testCaseFinished, worker.id)
       }
     } else {
       throw new Error(
@@ -187,15 +186,19 @@ export default class Coordinator implements IRuntime {
     }
   }
 
-  parseTestCaseResult(testCaseFinished: messages.TestCaseFinished): void {
+  parseTestCaseResult(
+    testCaseFinished: messages.TestCaseFinished,
+    workerId: string
+  ): void {
     const { worstTestStepResult } = this.eventDataCollector.getTestCaseAttempt(
       testCaseFinished.testCaseStartedId
     )
-    if (
-      !testCaseFinished.willBeRetried &&
-      shouldCauseFailure(worstTestStepResult.status, this.options)
-    ) {
-      this.success = false
+    if (!testCaseFinished.willBeRetried) {
+      delete this.inProgressPickles[workerId]
+
+      if (shouldCauseFailure(worstTestStepResult.status, this.options)) {
+        this.success = false
+      }
     }
   }