From 866e571989667f88ba11901985145c3fb414e7ed Mon Sep 17 00:00:00 2001 From: Jaromir Obr Date: Fri, 30 Oct 2026 08:50:43 +0100 Subject: [PATCH 1/2] fix(step): measure step.duration from the first retry attempt (#5774) HelperStep.run() and FuncStep.run() overwrote startTime on every attempt, so a retried step reported only the duration of its last attempt. Keep the start time of the first attempt, as recordStep() already intends. Co-Authored-By: Claude Opus 5.5 --- lib/step/func.js | 2 +- lib/step/helper.js | 2 +- test/unit/actor_test.js | 16 ++++++++++++++++ test/unit/steps_test.js | 18 +++++++++++++++++- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/lib/step/func.js b/lib/step/func.js index 2007daa54..f1eca4c2b 100644 --- a/lib/step/func.js +++ b/lib/step/func.js @@ -21,7 +21,7 @@ class FuncStep extends BaseStep { // we wrap that function to track time and status // and disable it in dry run mode this.args = Array.prototype.slice.call(arguments) - this.startTime = +Date.now() + if (!this.startTime) this.startTime = +Date.now() if (store.dryRun) { this.setStatus('success') diff --git a/lib/step/helper.js b/lib/step/helper.js index a87148ac2..07f763ef1 100644 --- a/lib/step/helper.js +++ b/lib/step/helper.js @@ -16,7 +16,7 @@ class HelperStep extends Step { */ run() { this.args = Array.prototype.slice.call(arguments) - this.startTime = +Date.now() + if (!this.startTime) this.startTime = +Date.now() if (store.dryRun) { this.setStatus('success') diff --git a/test/unit/actor_test.js b/test/unit/actor_test.js index 9ab96c5d6..e4b654cba 100644 --- a/test/unit/actor_test.js +++ b/test/unit/actor_test.js @@ -163,6 +163,22 @@ describe('Actor', () => { return I.failAfter(2) }) + it('should measure duration of a retried step from its first attempt', () => { + recorder.start() + recorder.retry({ + retries: 2, + minTimeout: 100, + factor: 1, + when: () => true, + }) + let duration + event.dispatcher.addListener(event.step.finished, step => (duration = step.duration)) + + return I.failAfter(2).then(() => { + expect(duration).toBeGreaterThanOrEqual(190) + }) + }) + it('should print handle failed steps', () => { recorder.start() let listeners = 0 diff --git a/test/unit/steps_test.js b/test/unit/steps_test.js index c4f8f0fe9..ce05e46a0 100644 --- a/test/unit/steps_test.js +++ b/test/unit/steps_test.js @@ -1,5 +1,5 @@ import sinon from 'sinon' -import Step, { MetaStep } from '../../lib/step.js' +import Step, { MetaStep, FuncStep } from '../../lib/step.js' import event from '../../lib/event.js' import { secret } from '../../lib/secret.js' import { expect } from 'chai' @@ -90,6 +90,22 @@ describe('Steps', () => { expect(action.called) expect(step.status).is.equal('success') }) + + it('should keep start time of the first attempt when retried', () => { + step.startTime = 1000 + step.run() + expect(step.startTime).is.equal(1000) + }) + }) + }) + + describe('FuncStep', () => { + it('should keep start time of the first attempt when retried', () => { + const funcStep = new FuncStep('doSomething') + funcStep.setCallable(() => 'done') + funcStep.startTime = 1000 + expect(funcStep.run()).is.equal('done') + expect(funcStep.startTime).is.equal(1000) }) }) From 3f54f07470f10826d7c4810bb7b43c7712bebef2 Mon Sep 17 00:00:00 2001 From: Jaromir Obr Date: Fri, 30 Oct 2026 09:06:27 +0100 Subject: [PATCH 2/2] test(actor): use step-level retry so the retry rule doesn't leak The global recorder.retry() rule stayed registered after the test and retried errors in later test files (AsyncWrapper tests failed in CI). step.retry() removes its rule once the step finishes. Co-Authored-By: Claude Opus 5.5 --- test/unit/actor_test.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/unit/actor_test.js b/test/unit/actor_test.js index e4b654cba..76e8d17b2 100644 --- a/test/unit/actor_test.js +++ b/test/unit/actor_test.js @@ -165,16 +165,10 @@ describe('Actor', () => { it('should measure duration of a retried step from its first attempt', () => { recorder.start() - recorder.retry({ - retries: 2, - minTimeout: 100, - factor: 1, - when: () => true, - }) let duration event.dispatcher.addListener(event.step.finished, step => (duration = step.duration)) - return I.failAfter(2).then(() => { + return I.failAfter(2, step.retry({ retries: 2, minTimeout: 100, factor: 1 })).then(() => { expect(duration).toBeGreaterThanOrEqual(190) }) })