From a28dc9b14576c014eddbd618405005b286bc857c Mon Sep 17 00:00:00 2001 From: Tristan Zander Date: Sat, 11 May 2024 08:08:34 -0400 Subject: [PATCH] fix: Ensure parallel rules aren't bypassed on retries (#2404) --- CHANGELOG.md | 1 + features/parallel.feature | 49 +++++++++++++++ features/step_definitions/parallel_steps.ts | 67 +++++++++++++++++++++ package.json | 1 + src/runtime/parallel/coordinator.ts | 19 +++--- 5 files changed, 129 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7689c5bd..9f97d8597 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/features/parallel.feature b/features/parallel.feature index 7cef38f97..5bd8ed5b1 100644 --- a/features/parallel.feature +++ b/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 diff --git a/features/step_definitions/parallel_steps.ts b/features/step_definitions/parallel_steps.ts index eda500330..69778c5b6 100644 --- a/features/step_definitions/parallel_steps.ts +++ b/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', + ]) + } +) diff --git a/package.json b/package.json index 4d2b439d2..80252ee84 100644 --- a/package.json +++ b/package.json @@ -164,6 +164,7 @@ "Tom V ", "Tomer Ben-Rachel ", "Tristan Dunn ", + "Tristan Zander ", "unknown ", "Valerio Innocenti Sedili ", "Vasily Shelkov ", diff --git a/src/runtime/parallel/coordinator.ts b/src/runtime/parallel/coordinator.ts index 274e0fdb1..a72c58329 100644 --- a/src/runtime/parallel/coordinator.ts +++ b/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 + } } }