Skip to content

Commit 38ce86a

Browse files
juan-fernandezjuan-fernandez
authored
[test optimization] Fix mocha parallel mode with retries (#7768)
1 parent 39ae05a commit 38ce86a

File tree

7 files changed

+196
-6
lines changed

7 files changed

+196
-6
lines changed

‎integration-tests/ci-visibility/mocha-plugin-tests/retries-parallel-2.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict'
2+
3+
const assert = require('assert')
4+
let attempt = 0
5+
6+
describe('mocha-test-retries-parallel-2', function () {
7+
this.retries(2)
8+
9+
it('will fail twice then pass', () => {
10+
assert.strictEqual(attempt++, 2)
11+
})
12+
})

‎integration-tests/ci-visibility/mocha-plugin-tests/retries-parallel.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict'
2+
3+
const assert = require('assert')
4+
let attempt = 0
5+
6+
describe('mocha-test-retries-parallel', function () {
7+
this.retries(2)
8+
9+
it('will fail twice then pass', () => {
10+
assert.strictEqual(attempt++, 2)
11+
})
12+
})

‎integration-tests/ci-visibility/test-flaky-test-retries/eventually-passing-test-parallel-2.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
'use strict'
2+
3+
const assert = require('assert')
4+
let counter = 0
5+
6+
describe('test-flaky-test-retries-parallel-2', () => {
7+
it('can retry failed tests', () => {
8+
assert.strictEqual(++counter, 3)
9+
})
10+
})

‎integration-tests/ci-visibility/test-flaky-test-retries/eventually-passing-test-parallel.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
'use strict'
2+
3+
const assert = require('assert')
4+
let counter = 0
5+
6+
describe('test-flaky-test-retries-parallel', () => {
7+
it('can retry failed tests', () => {
8+
assert.strictEqual(++counter, 3)
9+
})
10+
})

‎integration-tests/mocha/mocha.spec.js‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,70 @@ describe(`mocha@${MOCHA_VERSION}`, function () {
13531353
})
13541354
})
13551355

1356+
onlyLatestIt('correctly reports retries in parallel mode', async () => {
1357+
const eventsPromise = receiver
1358+
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/citestcycle'), (payloads) => {
1359+
const events = payloads.flatMap(({ payload }) => payload.events)
1360+
const tests = events.filter(event => event.type === 'test').map(event => event.content)
1361+
1362+
// Verify we are actually in parallel mode
1363+
const sessionEvent = events.find(event => event.type === 'test_session_end').content
1364+
assert.strictEqual(sessionEvent.meta[MOCHA_IS_PARALLEL], 'true')
1365+
1366+
// Each file has 1 test that fails twice then passes on the 3rd attempt (3 events per file)
1367+
assert.strictEqual(tests.length, 6)
1368+
1369+
// 2 failed attempts per file = 4 total
1370+
const failedTests = tests.filter(test => test.meta[TEST_STATUS] === 'fail')
1371+
assert.strictEqual(failedTests.length, 4)
1372+
1373+
// The 3rd attempt passes in each file = 2 total
1374+
const passingTests = tests.filter(test => test.meta[TEST_STATUS] === 'pass')
1375+
assert.strictEqual(passingTests.length, 2)
1376+
1377+
// First attempt of each test is not a retry, subsequent ones are (2 retries * 2 files = 4)
1378+
const retries = tests.filter(test => test.meta[TEST_IS_RETRY] === 'true')
1379+
assert.strictEqual(retries.length, 4)
1380+
1381+
// Native --retries (not ATR), so retry reason should be 'external'
1382+
retries.forEach(test => {
1383+
assert.strictEqual(test.meta[TEST_RETRY_REASON], TEST_RETRY_REASON_TYPES.ext)
1384+
})
1385+
1386+
// Verify the two files ran in separate worker processes
1387+
const testsBySuite = {}
1388+
for (const test of tests) {
1389+
const suiteName = test.meta[TEST_SUITE]
1390+
if (!testsBySuite[suiteName]) {
1391+
testsBySuite[suiteName] = test
1392+
}
1393+
}
1394+
const testFromEachWorker = Object.values(testsBySuite)
1395+
assert.strictEqual(testFromEachWorker.length, 2)
1396+
const runtimeIds = testFromEachWorker.map(test => test.meta['runtime-id'])
1397+
assert.ok(runtimeIds[0])
1398+
assert.ok(runtimeIds[1])
1399+
assert.notStrictEqual(runtimeIds[0], runtimeIds[1],
1400+
'Tests from different files should have different runtime-ids (separate worker processes)'
1401+
)
1402+
})
1403+
1404+
childProcess = exec(
1405+
'node node_modules/mocha/bin/mocha' +
1406+
' --parallel --jobs 2 --retries 2' +
1407+
' ./ci-visibility/mocha-plugin-tests/retries-parallel.js' +
1408+
' ./ci-visibility/mocha-plugin-tests/retries-parallel-2.js',
1409+
{
1410+
cwd,
1411+
env: getCiVisAgentlessConfig(receiver.port),
1412+
}
1413+
)
1414+
await Promise.all([
1415+
eventsPromise,
1416+
once(childProcess, 'exit'),
1417+
])
1418+
})
1419+
13561420
it('does not blow up when workerpool is used outside of a test', (done) => {
13571421
childProcess = exec('node ./ci-visibility/run-workerpool.js', {
13581422
cwd,
@@ -3218,6 +3282,74 @@ describe(`mocha@${MOCHA_VERSION}`, function () {
32183282

32193283
await Promise.all([once(childProcess, 'exit'), eventsPromise])
32203284
})
3285+
3286+
onlyLatestIt('retries failed tests automatically in parallel mode', async () => {
3287+
receiver.setSettings({
3288+
flaky_test_retries_enabled: true,
3289+
early_flake_detection: {
3290+
enabled: false,
3291+
},
3292+
})
3293+
3294+
const eventsPromise = receiver
3295+
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/citestcycle'), (payloads) => {
3296+
const events = payloads.flatMap(({ payload }) => payload.events)
3297+
const tests = events.filter(event => event.type === 'test').map(event => event.content)
3298+
3299+
// Verify we are in parallel mode
3300+
const sessionEvent = events.find(event => event.type === 'test_session_end').content
3301+
assert.strictEqual(sessionEvent.meta[MOCHA_IS_PARALLEL], 'true')
3302+
3303+
// Each file has 1 test that fails twice then passes (3 events per file, 6 total)
3304+
assert.strictEqual(tests.length, 6)
3305+
3306+
const failedAttempts = tests.filter(test => test.meta[TEST_STATUS] === 'fail')
3307+
assert.strictEqual(failedAttempts.length, 4)
3308+
3309+
const passedAttempts = tests.filter(test => test.meta[TEST_STATUS] === 'pass')
3310+
assert.strictEqual(passedAttempts.length, 2)
3311+
3312+
// Retries should be tagged with ATR reason
3313+
const atrRetries = tests.filter(
3314+
test => test.meta[TEST_RETRY_REASON] === TEST_RETRY_REASON_TYPES.atr
3315+
)
3316+
assert.strictEqual(atrRetries.length, 4)
3317+
3318+
passedAttempts.forEach(test => {
3319+
assert.strictEqual(test.meta[TEST_IS_RETRY], 'true')
3320+
assert.strictEqual(test.meta[TEST_RETRY_REASON], TEST_RETRY_REASON_TYPES.atr)
3321+
})
3322+
3323+
// Verify tests ran in separate worker processes
3324+
const testsBySuite = {}
3325+
for (const test of tests) {
3326+
const suiteName = test.meta[TEST_SUITE]
3327+
if (!testsBySuite[suiteName]) {
3328+
testsBySuite[suiteName] = test
3329+
}
3330+
}
3331+
const testFromEachWorker = Object.values(testsBySuite)
3332+
assert.strictEqual(testFromEachWorker.length, 2)
3333+
const runtimeIds = testFromEachWorker.map(test => test.meta['runtime-id'])
3334+
assert.ok(runtimeIds[0])
3335+
assert.ok(runtimeIds[1])
3336+
assert.notStrictEqual(runtimeIds[0], runtimeIds[1])
3337+
})
3338+
3339+
childProcess = exec(
3340+
'node node_modules/mocha/bin/mocha --parallel --jobs 2' +
3341+
' ./ci-visibility/test-flaky-test-retries/eventually-passing-test-parallel.js' +
3342+
' ./ci-visibility/test-flaky-test-retries/eventually-passing-test-parallel-2.js',
3343+
{
3344+
cwd,
3345+
env: getCiVisAgentlessConfig(receiver.port),
3346+
}
3347+
)
3348+
await Promise.all([
3349+
eventsPromise,
3350+
once(childProcess, 'exit'),
3351+
])
3352+
})
32213353
})
32223354

32233355
it('takes into account untested files if "all" is passed to nyc', (done) => {

‎packages/datadog-instrumentations/src/mocha/main.js‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -315,10 +315,10 @@ function getExecutionConfiguration (runner, isParallel, frameworkVersion, onFini
315315
config.isTestManagementTestsEnabled = libraryConfig.isTestManagementEnabled
316316
config.testManagementAttemptToFixRetries = libraryConfig.testManagementAttemptToFixRetries
317317
config.isImpactedTestsEnabled = libraryConfig.isImpactedTestsEnabled
318-
// ITR and auto test retries are not supported in parallel mode yet
318+
// ITR is not supported in parallel mode yet
319319
config.isSuitesSkippingEnabled = !isParallel && libraryConfig.isSuitesSkippingEnabled
320-
config.isFlakyTestRetriesEnabled = !isParallel && libraryConfig.isFlakyTestRetriesEnabled
321-
config.flakyTestRetriesCount = !isParallel && libraryConfig.flakyTestRetriesCount
320+
config.isFlakyTestRetriesEnabled = libraryConfig.isFlakyTestRetriesEnabled
321+
config.flakyTestRetriesCount = libraryConfig.flakyTestRetriesCount
322322

323323
if (config.isKnownTestsEnabled) {
324324
ctx.onDone = onReceivedKnownTests
@@ -663,7 +663,8 @@ addHook({
663663
if (!testFinishCh.hasSubscribers ||
664664
(!config.isKnownTestsEnabled &&
665665
!config.isTestManagementTestsEnabled &&
666-
!config.isImpactedTestsEnabled)) {
666+
!config.isImpactedTestsEnabled &&
667+
!config.isFlakyTestRetriesEnabled)) {
667668
return run.apply(this, arguments)
668669
}
669670

@@ -709,6 +710,11 @@ addHook({
709710
newWorkerArgs._ddModifiedFiles = config.modifiedFiles || {}
710711
}
711712

713+
if (config.isFlakyTestRetriesEnabled) {
714+
newWorkerArgs._ddIsFlakyTestRetriesEnabled = true
715+
newWorkerArgs._ddFlakyTestRetriesCount = config.flakyTestRetriesCount
716+
}
717+
712718
// We pass the known tests for the test file to the worker
713719
const testFileResult = await run.apply(
714720
this,

‎packages/datadog-instrumentations/src/mocha/worker.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const {
1010
getOnHookEndHandler,
1111
getOnFailHandler,
1212
getOnPendingHandler,
13+
getOnTestRetryHandler,
1314
getRunTestsWrapper,
1415
} = require('./utils')
1516
require('./common')
@@ -48,6 +49,12 @@ addHook({
4849
delete this.options._ddIsTestManagementTestsEnabled
4950
delete this.options._ddTestManagementTests
5051
}
52+
if (this.options._ddIsFlakyTestRetriesEnabled) {
53+
config.isFlakyTestRetriesEnabled = true
54+
config.flakyTestRetriesCount = this.options._ddFlakyTestRetriesCount
55+
delete this.options._ddIsFlakyTestRetriesEnabled
56+
delete this.options._ddFlakyTestRetriesCount
57+
}
5158
return run.apply(this, arguments)
5259
})
5360

@@ -74,6 +81,8 @@ addHook({
7481

7582
this.on('test end', getOnTestEndHandler(config))
7683

84+
this.on('retry', getOnTestRetryHandler(config))
85+
7786
// If the hook passes, 'hook end' will be emitted. Otherwise, 'fail' will be emitted
7887
this.on('hook end', getOnHookEndHandler())
7988

@@ -92,5 +101,4 @@ addHook({
92101
name: 'mocha',
93102
versions: ['>=5.2.0'],
94103
file: 'lib/runnable.js',
95-
}, runnableWrapper)
96-
// TODO: parallel mode does not support flaky test retries, so no library config is passed.
104+
}, (runnablePackage) => runnableWrapper(runnablePackage, config))

0 commit comments

Comments
 (0)