Skip to content

Commit 0d49ecf

Browse files
juan-fernandezjuan-fernandez
authored
[test optimization] Fix ATR + DI issues with jest (#5136)
1 parent dcf3c7e commit 0d49ecf

7 files changed

Lines changed: 68 additions & 42 deletions

File tree

‎integration-tests/ci-visibility/dynamic-instrumentation/is-jest.js‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎integration-tests/ci-visibility/dynamic-instrumentation/test-hit-breakpoint.js‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,16 @@
11
/* eslint-disable */
22
const sum = require('./dependency')
3-
const isJest = require('./is-jest')
43
const { expect } = require('chai')
54

6-
// TODO: instead of retrying through jest, this should be retried with auto test retries
7-
if (isJest()) {
8-
jest.retryTimes(1)
9-
}
10-
5+
let count = 0
116
describe('dynamic-instrumentation', () => {
127
it('retries with DI', function () {
13-
if (this.retries) {
14-
this.retries(1)
8+
if (process.env.TEST_SHOULD_PASS_AFTER_RETRY && count++ === 1) {
9+
// Passes after a retry if TEST_SHOULD_PASS_AFTER_RETRY is passed
10+
expect(sum(1, 3)).to.equal(4)
11+
} else {
12+
expect(sum(11, 3)).to.equal(14)
1513
}
16-
expect(sum(11, 3)).to.equal(14)
1714
})
1815

1916
it('is not retried', () => {

‎integration-tests/ci-visibility/dynamic-instrumentation/test-not-hit-breakpoint.js‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,10 @@
11
/* eslint-disable */
22
const sum = require('./dependency')
3-
const isJest = require('./is-jest')
43
const { expect } = require('chai')
54

6-
// TODO: instead of retrying through jest, this should be retried with auto test retries
7-
if (isJest()) {
8-
jest.retryTimes(1)
9-
}
10-
115
let count = 0
126
describe('dynamic-instrumentation', () => {
137
it('retries with DI', function () {
14-
if (this.retries) {
15-
this.retries(1)
16-
}
178
const willFail = count++ === 0
189
if (willFail) {
1910
expect(sum(11, 3)).to.equal(14) // only throws the first time

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

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ describe('jest CommonJS', () => {
518518
const retriedTests = tests.filter(test => test.meta[TEST_IS_RETRY] === 'true')
519519

520520
assert.equal(retriedTests.length, 2)
521-
const [retriedTest] = retriedTests
521+
const retriedTest = retriedTests.find(test => test.meta[TEST_SUITE].includes('test-hit-breakpoint.js'))
522522

523523
assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true')
524524

@@ -560,6 +560,7 @@ describe('jest CommonJS', () => {
560560
...getCiVisAgentlessConfig(receiver.port),
561561
TESTS_TO_RUN: 'dynamic-instrumentation/test-',
562562
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
563+
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1',
563564
RUN_IN_PARALLEL: true
564565
},
565566
stdio: 'inherit'
@@ -2518,7 +2519,8 @@ describe('jest CommonJS', () => {
25182519
cwd,
25192520
env: {
25202521
...getCiVisAgentlessConfig(receiver.port),
2521-
TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint'
2522+
TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
2523+
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
25222524
},
25232525
stdio: 'inherit'
25242526
}
@@ -2565,7 +2567,8 @@ describe('jest CommonJS', () => {
25652567
env: {
25662568
...getCiVisAgentlessConfig(receiver.port),
25672569
TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
2568-
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
2570+
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
2571+
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
25692572
},
25702573
stdio: 'inherit'
25712574
}
@@ -2649,7 +2652,8 @@ describe('jest CommonJS', () => {
26492652
env: {
26502653
...getCiVisAgentlessConfig(receiver.port),
26512654
TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
2652-
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
2655+
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
2656+
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
26532657
},
26542658
stdio: 'inherit'
26552659
}
@@ -2698,7 +2702,8 @@ describe('jest CommonJS', () => {
26982702
env: {
26992703
...getCiVisAgentlessConfig(receiver.port),
27002704
TESTS_TO_RUN: 'dynamic-instrumentation/test-not-hit-breakpoint',
2701-
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
2705+
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
2706+
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
27022707
},
27032708
stdio: 'inherit'
27042709
}
@@ -2711,6 +2716,44 @@ describe('jest CommonJS', () => {
27112716
}).catch(done)
27122717
})
27132718
})
2719+
2720+
it('does not wait for breakpoint for a passed test', (done) => {
2721+
receiver.setSettings({
2722+
flaky_test_retries_enabled: true,
2723+
di_enabled: true
2724+
})
2725+
2726+
const eventsPromise = receiver
2727+
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/citestcycle'), (payloads) => {
2728+
const events = payloads.flatMap(({ payload }) => payload.events)
2729+
2730+
const tests = events.filter(event => event.type === 'test').map(event => event.content)
2731+
const retriedTests = tests.filter(test => test.meta[TEST_IS_RETRY] === 'true')
2732+
2733+
assert.equal(retriedTests.length, 1)
2734+
const [retriedTest] = retriedTests
2735+
// Duration is in nanoseconds, so 200 * 1e6 is 200ms
2736+
assert.equal(retriedTest.duration < 200 * 1e6, true)
2737+
})
2738+
2739+
childProcess = exec(runTestsWithCoverageCommand,
2740+
{
2741+
cwd,
2742+
env: {
2743+
...getCiVisAgentlessConfig(receiver.port),
2744+
TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
2745+
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
2746+
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1',
2747+
TEST_SHOULD_PASS_AFTER_RETRY: '1'
2748+
},
2749+
stdio: 'inherit'
2750+
}
2751+
)
2752+
2753+
childProcess.on('exit', () => {
2754+
eventsPromise.then(() => done()).catch(done)
2755+
})
2756+
})
27142757
})
27152758

27162759
// This happens when using office-addin-mock

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2188,7 +2188,8 @@ describe('mocha CommonJS', function () {
21882188
...getCiVisAgentlessConfig(receiver.port),
21892189
TESTS_TO_RUN: JSON.stringify([
21902190
'./dynamic-instrumentation/test-hit-breakpoint'
2191-
])
2191+
]),
2192+
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
21922193
},
21932194
stdio: 'inherit'
21942195
}
@@ -2240,7 +2241,8 @@ describe('mocha CommonJS', function () {
22402241
TESTS_TO_RUN: JSON.stringify([
22412242
'./dynamic-instrumentation/test-hit-breakpoint'
22422243
]),
2243-
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
2244+
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
2245+
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
22442246
},
22452247
stdio: 'inherit'
22462248
}
@@ -2329,7 +2331,8 @@ describe('mocha CommonJS', function () {
23292331
TESTS_TO_RUN: JSON.stringify([
23302332
'./dynamic-instrumentation/test-hit-breakpoint'
23312333
]),
2332-
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
2334+
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
2335+
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
23332336
},
23342337
stdio: 'inherit'
23352338
}
@@ -2382,7 +2385,8 @@ describe('mocha CommonJS', function () {
23822385
TESTS_TO_RUN: JSON.stringify([
23832386
'./dynamic-instrumentation/test-not-hit-breakpoint'
23842387
]),
2385-
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
2388+
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
2389+
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
23862390
},
23872391
stdio: 'inherit'
23882392
}

‎packages/datadog-instrumentations/src/jest.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {
303303
const numRetries = this.global[RETRY_TIMES]
304304
const numTestExecutions = event.test?.invocations
305305
const willBeRetried = numRetries > 0 && numTestExecutions - 1 < numRetries
306-
const mightHitBreakpoint = this.isDiEnabled && numTestExecutions >= 1
306+
const mightHitBreakpoint = this.isDiEnabled && numTestExecutions >= 2
307307

308308
const asyncResource = asyncResources.get(event.test)
309309

@@ -319,7 +319,7 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {
319319

320320
// After finishing it might take a bit for the snapshot to be handled.
321321
// This means that tests retried with DI are BREAKPOINT_HIT_GRACE_PERIOD_MS slower at least.
322-
if (mightHitBreakpoint) {
322+
if (status === 'fail' && mightHitBreakpoint) {
323323
await new Promise(resolve => {
324324
setTimeout(() => {
325325
resolve()

‎packages/dd-trace/src/plugins/util/test.js‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -691,14 +691,12 @@ function getFileAndLineNumberFromError (error, repositoryRoot) {
691691
return []
692692
}
693693

694-
// The error.stack property in TestingLibraryElementError includes the message, which results in redundant information
695694
function getFormattedError (error, repositoryRoot) {
696-
if (error.name !== 'TestingLibraryElementError') {
697-
return error
698-
}
699-
const { stack } = error
700695
const newError = new Error(error.message)
701-
newError.stack = stack.split('\n').filter(line => line.includes(repositoryRoot)).join('\n')
696+
if (error.stack) {
697+
newError.stack = error.stack.split('\n').filter(line => line.includes(repositoryRoot)).join('\n')
698+
}
699+
newError.name = error.name
702700

703701
return newError
704702
}

0 commit comments

Comments
 (0)