[test optimization] Change failed test replay to opt out rather than …

…opt in (#5283)
DataDog · Feb 20, 2025 · 928fb5d · 928fb5d
1 parent 619d99b
commit 928fb5d
Show file tree

Hide file tree

Showing 8 changed files with 36 additions and 49 deletions.
diff --git a/integration-tests/cucumber/cucumber.spec.js b/integration-tests/cucumber/cucumber.spec.js
@@ -1587,7 +1587,7 @@ versions.forEach(version => {
           })
           // Dynamic instrumentation only supported from >=8.0.0
           context('dynamic instrumentation', () => {
-            it('does not activate if DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED is not set', (done) => {
+            it('does not activate if DD_TEST_FAILED_TEST_REPLAY_ENABLED is set to false', (done) => {
               receiver.setSettings({
                 flaky_test_retries_enabled: true,
                 di_enabled: true
@@ -1621,7 +1621,10 @@ versions.forEach(version => {
                 './node_modules/.bin/cucumber-js ci-visibility/features-di/test-hit-breakpoint.feature --retry 1',
                 {
                   cwd,
-                  env: envVars,
+                  env: {
+                    ...envVars,
+                    DD_TEST_FAILED_TEST_REPLAY_ENABLED: 'false'
+                  },
                   stdio: 'pipe'
                 }
               )
@@ -1666,10 +1669,7 @@ versions.forEach(version => {
                 './node_modules/.bin/cucumber-js ci-visibility/features-di/test-hit-breakpoint.feature --retry 1',
                 {
                   cwd,
-                  env: {
-                    ...envVars,
-                    DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
-                  },
+                  env: envVars,
                   stdio: 'pipe'
                 }
               )
@@ -1748,10 +1748,7 @@ versions.forEach(version => {
                 './node_modules/.bin/cucumber-js ci-visibility/features-di/test-hit-breakpoint.feature --retry 1',
                 {
                   cwd,
-                  env: {
-                    ...envVars,
-                    DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
-                  },
+                  env: envVars,
                   stdio: 'pipe'
                 }
               )
@@ -1800,10 +1797,7 @@ versions.forEach(version => {
                 './node_modules/.bin/cucumber-js ci-visibility/features-di/test-not-hit-breakpoint.feature --retry 1',
                 {
                   cwd,
-                  env: {
-                    ...envVars,
-                    DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
-                  },
+                  env: envVars,
                   stdio: 'pipe'
                 }
               )

diff --git a/integration-tests/jest/jest.spec.js b/integration-tests/jest/jest.spec.js
@@ -509,7 +509,7 @@ describe('jest CommonJS', () => {
         }).catch(done)
     })
 
-    it('can work with Dynamic Instrumentation', (done) => {
+    it('can work with Failed Test Replay', (done) => {
       receiver.setSettings({
         flaky_test_retries_enabled: true,
         di_enabled: true
@@ -565,7 +565,6 @@ describe('jest CommonJS', () => {
           env: {
             ...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
           },
@@ -2537,7 +2536,7 @@ describe('jest CommonJS', () => {
   })
 
   context('dynamic instrumentation', () => {
-    it('does not activate dynamic instrumentation if DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED is not set', (done) => {
+    it('does not activate dynamic instrumentation if DD_TEST_FAILED_TEST_REPLAY_ENABLED is set to false', (done) => {
       receiver.setSettings({
         flaky_test_retries_enabled: true,
         di_enabled: true
@@ -2571,7 +2570,8 @@ describe('jest CommonJS', () => {
           env: {
             ...getCiVisAgentlessConfig(receiver.port),
             TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
-            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1',
+            DD_TEST_FAILED_TEST_REPLAY_ENABLED: 'false'
           },
           stdio: 'inherit'
         }
@@ -2618,7 +2618,6 @@ describe('jest CommonJS', () => {
           env: {
             ...getCiVisAgentlessConfig(receiver.port),
             TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
             DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
@@ -2703,7 +2702,6 @@ describe('jest CommonJS', () => {
           env: {
             ...getCiVisAgentlessConfig(receiver.port),
             TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
             DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
@@ -2753,7 +2751,6 @@ describe('jest CommonJS', () => {
           env: {
             ...getCiVisAgentlessConfig(receiver.port),
             TESTS_TO_RUN: 'dynamic-instrumentation/test-not-hit-breakpoint',
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
             DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
@@ -2793,7 +2790,6 @@ describe('jest CommonJS', () => {
           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'
           },

diff --git a/integration-tests/mocha/mocha.spec.js b/integration-tests/mocha/mocha.spec.js
@@ -2209,7 +2209,7 @@ describe('mocha CommonJS', function () {
   })
 
   context('dynamic instrumentation', () => {
-    it('does not activate dynamic instrumentation if DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED is not set', (done) => {
+    it('does not activate dynamic instrumentation if DD_TEST_FAILED_TEST_REPLAY_ENABLED is set to false', (done) => {
       receiver.setSettings({
         flaky_test_retries_enabled: true,
         di_enabled: true
@@ -2247,7 +2247,8 @@ describe('mocha CommonJS', function () {
             TESTS_TO_RUN: JSON.stringify([
               './dynamic-instrumentation/test-hit-breakpoint'
             ]),
-            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
+            DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1',
+            DD_TEST_FAILED_TEST_REPLAY_ENABLED: 'false'
           },
           stdio: 'inherit'
         }
@@ -2299,7 +2300,6 @@ describe('mocha CommonJS', function () {
             TESTS_TO_RUN: JSON.stringify([
               './dynamic-instrumentation/test-hit-breakpoint'
             ]),
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
             DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
@@ -2389,7 +2389,6 @@ describe('mocha CommonJS', function () {
             TESTS_TO_RUN: JSON.stringify([
               './dynamic-instrumentation/test-hit-breakpoint'
             ]),
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
             DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'
@@ -2443,7 +2442,6 @@ describe('mocha CommonJS', function () {
             TESTS_TO_RUN: JSON.stringify([
               './dynamic-instrumentation/test-not-hit-breakpoint'
             ]),
-            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
             DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
           },
           stdio: 'inherit'

diff --git a/integration-tests/vitest/vitest.spec.js b/integration-tests/vitest/vitest.spec.js
@@ -987,7 +987,7 @@ versions.forEach((version) => {
     // dynamic instrumentation only supported from >=2.0.0
     if (version === 'latest') {
       context('dynamic instrumentation', () => {
-        it('does not activate it if DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED is not set', (done) => {
+        it('does not activate it if DD_TEST_FAILED_TEST_REPLAY_ENABLED is set to false', (done) => {
           receiver.setSettings({
             flaky_test_retries_enabled: true,
             di_enabled: true
@@ -1025,7 +1025,8 @@ versions.forEach((version) => {
               env: {
                 ...getCiVisAgentlessConfig(receiver.port),
                 TEST_DIR: 'ci-visibility/vitest-tests/dynamic-instrumentation*',
-                NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init'
+                NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init',
+                DD_TEST_FAILED_TEST_REPLAY_ENABLED: 'false'
               },
               stdio: 'pipe'
             }
@@ -1075,8 +1076,7 @@ versions.forEach((version) => {
               env: {
                 ...getCiVisAgentlessConfig(receiver.port),
                 TEST_DIR: 'ci-visibility/vitest-tests/dynamic-instrumentation*',
-                NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init',
-                DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: '1'
+                NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init'
               },
               stdio: 'pipe'
             }
@@ -1162,8 +1162,7 @@ versions.forEach((version) => {
               env: {
                 ...getCiVisAgentlessConfig(receiver.port),
                 TEST_DIR: 'ci-visibility/vitest-tests/dynamic-instrumentation*',
-                NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init',
-                DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: '1'
+                NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init'
               },
               stdio: 'pipe'
             }
@@ -1217,8 +1216,7 @@ versions.forEach((version) => {
               env: {
                 ...getCiVisAgentlessConfig(receiver.port),
                 TEST_DIR: 'ci-visibility/vitest-tests/breakpoint-not-hit*',
-                NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init',
-                DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: '1'
+                NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init'
               },
               stdio: 'pipe'
             }

diff --git a/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js b/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js
@@ -70,7 +70,7 @@ class TestVisDynamicInstrumentation {
         env: {
           ...process.env,
           DD_TRACE_ENABLED: 0,
-          DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 0
+          DD_TEST_FAILED_TEST_REPLAY_ENABLED: 0
         },
         workerData: {
           config: config.serialize(),

diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js
@@ -1144,7 +1144,7 @@ class Config {
       DD_CIVISIBILITY_FLAKY_RETRY_COUNT,
       DD_TEST_SESSION_NAME,
       DD_AGENTLESS_LOG_SUBMISSION_ENABLED,
-      DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED,
+      DD_TEST_FAILED_TEST_REPLAY_ENABLED,
       DD_TEST_MANAGEMENT_ENABLED,
       DD_TEST_MANAGEMENT_ATTEMPT_TO_FIX_RETRIES
     } = process.env
@@ -1164,7 +1164,7 @@ class Config {
       this._setBoolean(calc, 'isManualApiEnabled', !isFalse(this._isCiVisibilityManualApiEnabled()))
       this._setString(calc, 'ciVisibilityTestSessionName', DD_TEST_SESSION_NAME)
       this._setBoolean(calc, 'ciVisAgentlessLogSubmissionEnabled', isTrue(DD_AGENTLESS_LOG_SUBMISSION_ENABLED))
-      this._setBoolean(calc, 'isTestDynamicInstrumentationEnabled', isTrue(DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED))
+      this._setBoolean(calc, 'isTestDynamicInstrumentationEnabled', !isFalse(DD_TEST_FAILED_TEST_REPLAY_ENABLED))
       this._setBoolean(calc, 'isServiceUserProvided', !!this._env.service)
       this._setBoolean(calc, 'isTestManagementEnabled', !isFalse(DD_TEST_MANAGEMENT_ENABLED))
       this._setValue(calc,

diff --git a/packages/dd-trace/src/plugins/ci_plugin.js b/packages/dd-trace/src/plugins/ci_plugin.js
@@ -202,15 +202,15 @@ module.exports = class CiPlugin extends Plugin {
   configure (config, shouldGetEnvironmentData = true) {
     super.configure(config)
 
+    if (!shouldGetEnvironmentData) {
+      return
+    }
+
     if (config.isTestDynamicInstrumentationEnabled && !this.di) {
       const testVisibilityDynamicInstrumentation = require('../ci-visibility/dynamic-instrumentation')
       this.di = testVisibilityDynamicInstrumentation
     }
 
-    if (!shouldGetEnvironmentData) {
-      return
-    }
-
     this.testEnvironmentMetadata = getTestEnvironmentMetadata(this.constructor.id, this.config)
 
     const {

diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js
@@ -2039,7 +2039,7 @@ describe('Config', () => {
       delete process.env.DD_CIVISIBILITY_FLAKY_RETRY_COUNT
       delete process.env.DD_TEST_SESSION_NAME
       delete process.env.JEST_WORKER_ID
-      delete process.env.DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED
+      delete process.env.DD_TEST_FAILED_TEST_REPLAY_ENABLED
       delete process.env.DD_AGENTLESS_LOG_SUBMISSION_ENABLED
       options = {}
     })
@@ -2138,15 +2138,16 @@ describe('Config', () => {
         const config = new Config(options)
         expect(config).to.have.property('ciVisAgentlessLogSubmissionEnabled', true)
       })
-      it('should not set isTestDynamicInstrumentationEnabled by default', () => {
-        const config = new Config(options)
-        expect(config).to.have.property('isTestDynamicInstrumentationEnabled', false)
-      })
-      it('should set isTestDynamicInstrumentationEnabled if DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED is passed', () => {
-        process.env.DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED = 'true'
+      it('should set isTestDynamicInstrumentationEnabled by default', () => {
         const config = new Config(options)
         expect(config).to.have.property('isTestDynamicInstrumentationEnabled', true)
       })
+      it('should set isTestDynamicInstrumentationEnabled to false if DD_TEST_FAILED_TEST_REPLAY_ENABLED is false',
+        () => {
+          process.env.DD_TEST_FAILED_TEST_REPLAY_ENABLED = 'false'
+          const config = new Config(options)
+          expect(config).to.have.property('isTestDynamicInstrumentationEnabled', false)
+        })
     })
     context('ci visibility mode is not enabled', () => {
       it('should not activate intelligent test runner or git metadata upload', () => {