[test optimization] Fix ATR + DI issues with jest (#5136)

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

@@ -1,19 +1,16 @@
 /* eslint-disable */
 const sum = require('./dependency')
-const isJest = require('./is-jest')
 const { expect } = require('chai')
 
-// TODO: instead of retrying through jest, this should be retried with auto test retries
-if (isJest()) {
-  jest.retryTimes(1)
-}
-
+let count = 0
 describe('dynamic-instrumentation', () => {
   it('retries with DI', function () {
-    if (this.retries) {
-      this.retries(1)
+    if (process.env.TEST_SHOULD_PASS_AFTER_RETRY && count++ === 1) {
+      // Passes after a retry if TEST_SHOULD_PASS_AFTER_RETRY is passed
+      expect(sum(1, 3)).to.equal(4)
+    } else {
+      expect(sum(11, 3)).to.equal(14)
     }
-    expect(sum(11, 3)).to.equal(14)
   })
 
   it('is not retried', () => {

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

@@ -1,19 +1,10 @@
 /* eslint-disable */
 const sum = require('./dependency')
-const isJest = require('./is-jest')
 const { expect } = require('chai')
 
-// TODO: instead of retrying through jest, this should be retried with auto test retries
-if (isJest()) {
-  jest.retryTimes(1)
-}
-
 let count = 0
 describe('dynamic-instrumentation', () => {
   it('retries with DI', function () {
-    if (this.retries) {
-      this.retries(1)
-    }
     const willFail = count++ === 0
     if (willFail) {
       expect(sum(11, 3)).to.equal(14) // only throws the first time

integration-tests/jest/jest.spec.js

@@ -518,7 +518,7 @@ describe('jest CommonJS', () => {
           const retriedTests = tests.filter(test => test.meta[TEST_IS_RETRY] === 'true')
 
           assert.equal(retriedTests.length, 2)
-          const [retriedTest] = retriedTests
+          const retriedTest = retriedTests.find(test => test.meta[TEST_SUITE].includes('test-hit-breakpoint.js'))
 
           assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true')
 
@@ -560,6 +560,7 @@ describe('jest CommonJS', () => {
             ...getCiVisAgentlessConfig(receiver.port),
             TESTS_TO_RUN: 'dynamic-instrumentation/test-',
             DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1',
             RUN_IN_PARALLEL: true
           },
           stdio: 'inherit'
@@ -2518,7 +2519,8 @@ describe('jest CommonJS', () => {
           cwd,
           env: {
             ...getCiVisAgentlessConfig(receiver.port),
-            TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint'
+            TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
         }
@@ -2565,7 +2567,8 @@ describe('jest CommonJS', () => {
           env: {
             ...getCiVisAgentlessConfig(receiver.port),
             TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
+            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
         }
@@ -2649,7 +2652,8 @@ describe('jest CommonJS', () => {
           env: {
             ...getCiVisAgentlessConfig(receiver.port),
             TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
+            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
         }
@@ -2698,7 +2702,8 @@ describe('jest CommonJS', () => {
           env: {
             ...getCiVisAgentlessConfig(receiver.port),
             TESTS_TO_RUN: 'dynamic-instrumentation/test-not-hit-breakpoint',
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
+            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
         }
@@ -2711,6 +2716,44 @@ describe('jest CommonJS', () => {
         }).catch(done)
       })
     })
+
+    it('does not wait for breakpoint for a passed test', (done) => {
+      receiver.setSettings({
+        flaky_test_retries_enabled: true,
+        di_enabled: true
+      })
+
+      const eventsPromise = receiver
+        .gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/citestcycle'), (payloads) => {
+          const events = payloads.flatMap(({ payload }) => payload.events)
+
+          const tests = events.filter(event => event.type === 'test').map(event => event.content)
+          const retriedTests = tests.filter(test => test.meta[TEST_IS_RETRY] === 'true')
+
+          assert.equal(retriedTests.length, 1)
+          const [retriedTest] = retriedTests
+          // Duration is in nanoseconds, so 200 * 1e6 is 200ms
+          assert.equal(retriedTest.duration < 200 * 1e6, true)
+        })
+
+      childProcess = exec(runTestsWithCoverageCommand,
+        {
+          cwd,
+          env: {
+            ...getCiVisAgentlessConfig(receiver.port),
+            TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
+            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1',
+            TEST_SHOULD_PASS_AFTER_RETRY: '1'
+          },
+          stdio: 'inherit'
+        }
+      )
+
+      childProcess.on('exit', () => {
+        eventsPromise.then(() => done()).catch(done)
+      })
+    })
   })
 
   // This happens when using office-addin-mock

integration-tests/mocha/mocha.spec.js

@@ -2188,7 +2188,8 @@ describe('mocha CommonJS', function () {
             ...getCiVisAgentlessConfig(receiver.port),
             TESTS_TO_RUN: JSON.stringify([
               './dynamic-instrumentation/test-hit-breakpoint'
-            ])
+            ]),
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
         }
@@ -2240,7 +2241,8 @@ describe('mocha CommonJS', function () {
             TESTS_TO_RUN: JSON.stringify([
               './dynamic-instrumentation/test-hit-breakpoint'
             ]),
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
+            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
         }
@@ -2329,7 +2331,8 @@ describe('mocha CommonJS', function () {
             TESTS_TO_RUN: JSON.stringify([
               './dynamic-instrumentation/test-hit-breakpoint'
             ]),
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
+            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
         }
@@ -2382,7 +2385,8 @@ describe('mocha CommonJS', function () {
             TESTS_TO_RUN: JSON.stringify([
               './dynamic-instrumentation/test-not-hit-breakpoint'
             ]),
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
+            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
         }

packages/datadog-instrumentations/src/jest.js

@@ -303,7 +303,7 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {
         const numRetries = this.global[RETRY_TIMES]
         const numTestExecutions = event.test?.invocations
         const willBeRetried = numRetries > 0 && numTestExecutions - 1 < numRetries
-        const mightHitBreakpoint = this.isDiEnabled && numTestExecutions >= 1
+        const mightHitBreakpoint = this.isDiEnabled && numTestExecutions >= 2
 
         const asyncResource = asyncResources.get(event.test)
 
@@ -319,7 +319,7 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {
 
         // After finishing it might take a bit for the snapshot to be handled.
         // This means that tests retried with DI are BREAKPOINT_HIT_GRACE_PERIOD_MS slower at least.
-        if (mightHitBreakpoint) {
+        if (status === 'fail' && mightHitBreakpoint) {
           await new Promise(resolve => {
             setTimeout(() => {
               resolve()

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

@@ -691,14 +691,12 @@ function getFileAndLineNumberFromError (error, repositoryRoot) {
   return []
 }
 
-// The error.stack property in TestingLibraryElementError includes the message, which results in redundant information
 function getFormattedError (error, repositoryRoot) {
-  if (error.name !== 'TestingLibraryElementError') {
-    return error
-  }
-  const { stack } = error
   const newError = new Error(error.message)
-  newError.stack = stack.split('\n').filter(line => line.includes(repositoryRoot)).join('\n')
+  if (error.stack) {
+    newError.stack = error.stack.split('\n').filter(line => line.includes(repositoryRoot)).join('\n')
+  }
+  newError.name = error.name
 
   return newError
 }