Skip to content

Commit 3b781c4

Browse files
authored
Merge pull request #5606 from smalia2001/fix/retry-failed-step-accumulation
fix: prevent retry config accumulation
2 parents f958b75 + 23b2bfe commit 3b781c4

9 files changed

Lines changed: 126 additions & 5 deletions

‎lib/helper/Playwright.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,8 @@ class Playwright extends Helper {
584584
// Clear popup state to ensure clean state for each test
585585
popupStore.clear()
586586

587-
recorder.retry({
587+
// Configure retry for this test; will clean up after test completes
588+
this._retryConfig = {
588589
retries: test?.opts?.conditionalRetries || 3,
589590
when: err => {
590591
if (!err || typeof err.message !== 'string') {
@@ -593,7 +594,8 @@ class Playwright extends Helper {
593594
// ignore context errors
594595
return err.message.includes('context')
595596
},
596-
})
597+
}
598+
recorder.retry(this._retryConfig)
597599

598600
// Start browser if needed (initial start or browser restart strategy)
599601
if (!this.isRunning && !this.options.manualStart) await this._startBrowser()
@@ -698,6 +700,12 @@ class Playwright extends Helper {
698700
}
699701

700702
async _after() {
703+
// Clean up our retry config to prevent accumulation
704+
if (this._retryConfig) {
705+
recorder.retries = recorder.retries.filter(r => r !== this._retryConfig)
706+
this._retryConfig = null
707+
}
708+
701709
if (!this.isRunning) return
702710

703711
// Clear popup state to prevent leakage between tests

‎lib/helper/Puppeteer.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,8 @@ class Puppeteer extends Helper {
354354
async _before(test) {
355355
this.sessionPages = {}
356356
this.currentRunningTest = test
357-
recorder.retry({
357+
// Configure retry for this test; will clean up after test completes
358+
this._retryConfig = {
358359
retries: test?.opts?.conditionalRetries || 3,
359360
when: err => {
360361
if (!err || typeof err.message !== 'string') {
@@ -363,13 +364,20 @@ class Puppeteer extends Helper {
363364
// ignore context errors
364365
return err.message.includes('context')
365366
},
366-
})
367+
}
368+
recorder.retry(this._retryConfig)
367369
if (this.options.restart && !this.options.manualStart) return this._startBrowser()
368370
if (!this.isRunning && !this.options.manualStart) return this._startBrowser()
369371
return this.browser
370372
}
371373

372374
async _after() {
375+
// Clean up our retry config to prevent accumulation
376+
if (this._retryConfig) {
377+
recorder.retries = recorder.retries.filter(r => r !== this._retryConfig)
378+
this._retryConfig = null
379+
}
380+
373381
if (!this.isRunning) return
374382

375383
// Clear popup state to prevent leakage between tests

‎lib/plugin/retryFailedStep.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,9 @@ export default function (config) {
151151
test.opts.stepRetryPriority = stepRetryPriority
152152

153153
debug('applying retries = %d for test %s', config.retries, test.title)
154-
recorder.retry(config)
154+
if (!recorder.retries.find(r => r === config)) {
155+
recorder.retry(config)
156+
}
155157
})
156158

157159
event.dispatcher.on(event.test.started, test => {
@@ -170,4 +172,8 @@ export default function (config) {
170172
test.opts.conditionalRetries = test.opts.conditionalRetries || config.retries
171173
}
172174
})
175+
176+
event.dispatcher.on(event.test.after, () => {
177+
recorder.retries = recorder.retries.filter(r => r !== config)
178+
})
173179
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
export const config = {
2+
tests: './retry_accumulation_test.js',
3+
output: './output',
4+
helpers: {
5+
Playwright: {
6+
url: 'http://localhost:8000',
7+
manualStart: true,
8+
},
9+
AccumulationHelper: {
10+
require: './helper.accumulation.js',
11+
},
12+
},
13+
plugins: {
14+
retryFailedStep: {
15+
enabled: true,
16+
retries: 2,
17+
},
18+
},
19+
bootstrap: null,
20+
mocha: {},
21+
name: 'retryAccumulation',
22+
};
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
export const config = {
2+
tests: './retry_multiple_scenarios_test.js',
3+
output: './output',
4+
helpers: {
5+
Playwright: {
6+
url: 'http://localhost:8000',
7+
show: false,
8+
restart: false,
9+
},
10+
AccumulationHelper: {
11+
require: './helper.accumulation.js',
12+
},
13+
},
14+
plugins: {
15+
retryFailedStep: {
16+
enabled: true,
17+
retries: 2,
18+
},
19+
},
20+
bootstrap: null,
21+
mocha: {},
22+
name: 'retryMultipleScenarios',
23+
};
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import Helper from '../../../../../lib/helper.js';
2+
3+
class AccumulationHelper extends Helper {
4+
_before() {
5+
this._failCount = 0;
6+
}
7+
8+
failingStep() {
9+
this._failCount++;
10+
if (this._failCount <= 2) {
11+
throw new Error('failing step - retry expected');
12+
}
13+
}
14+
}
15+
16+
export default AccumulationHelper;
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Feature('Retry Config Accumulation Test');
2+
3+
Scenario('first scenario', async ({ I }) => {
4+
I.failingStep();
5+
});
6+
7+
Scenario('second scenario', async ({ I }) => {
8+
I.failingStep();
9+
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
Feature('Retry FailedStep - Multiple Consequent Scenarios');
2+
3+
Scenario('first scenario', async ({ I }) => {
4+
I.failingStep();
5+
});
6+
7+
Scenario('second scenario', async ({ I }) => {
8+
I.failingStep();
9+
});
10+
11+
Scenario('third scenario', async ({ I }) => {
12+
I.failingStep();
13+
});

‎test/runner/retry_hooks_test.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,20 @@ describe('CodeceptJS Retry Hooks', function () {
6666
done()
6767
})
6868
})
69+
70+
it('should prevent retry config accumulation across tests', done => {
71+
exec(config_run_config('codecept.retry.accumulation.conf.js', ''), (err, stdout) => {
72+
debug_this_test && console.log(stdout)
73+
expect(stdout).toContain('2 passed')
74+
done()
75+
})
76+
})
77+
78+
it('should retryFailedStep on multiple consequent scenarios', done => {
79+
exec(config_run_config('codecept.retry.multipleScenarios.conf.js', ''), (err, stdout) => {
80+
debug_this_test && console.log(stdout)
81+
expect(stdout).toContain('3 passed')
82+
done()
83+
})
84+
})
6985
})

0 commit comments

Comments
 (0)