[test optimization] Better nomenclature usage in impacted tests logic

integration-tests/cucumber/cucumber.spec.js

@@ -2707,8 +2707,7 @@ versions.forEach(version => {
             }
           })
 
-      const runImpactedTest = (
-        done,
+      const runImpactedTest = async (
         { isModified, isEfd, isParallel, isNew },
         extraEnvVars = {}
       ) => {
@@ -2731,45 +2730,46 @@ versions.forEach(version => {
           }
         )
 
-        childProcess.on('exit', (code) => {
-          testAssertionsPromise.then(done).catch(done)
-        })
+        await Promise.all([
+          once(childProcess, 'exit'),
+          testAssertionsPromise
+        ])
       }
 
       context('test is not new', () => {
-        it('should be detected as impacted', (done) => {
+        it('should be detected as impacted', async () => {
           receiver.setSettings({ impacted_tests_enabled: true })
 
-          runImpactedTest(done, { isModified: true })
+          await runImpactedTest({ isModified: true })
         })
 
-        it('should not be detected as impacted if disabled', (done) => {
+        it('should not be detected as impacted if disabled', async () => {
           receiver.setSettings({ impacted_tests_enabled: false })
 
-          runImpactedTest(done, { isModified: false })
+          await runImpactedTest({ isModified: false })
         })
 
         it('should not be detected as impacted if DD_CIVISIBILITY_IMPACTED_TESTS_DETECTION_ENABLED is false',
-          (done) => {
+          async () => {
             receiver.setSettings({ impacted_tests_enabled: true })
 
-            runImpactedTest(done,
+            await runImpactedTest(
               { isModified: false },
               { DD_CIVISIBILITY_IMPACTED_TESTS_DETECTION_ENABLED: '0' }
             )
           })
 
         if (version !== '7.0.0') {
-          it('can detect impacted tests in parallel mode', (done) => {
+          it('can detect impacted tests in parallel mode', async () => {
             receiver.setSettings({ impacted_tests_enabled: true })
 
-            runImpactedTest(done, { isModified: true, isParallel: true })
+            await runImpactedTest({ isModified: true, isParallel: true })
           })
         }
       })
 
       context('test is new', () => {
-        it('should be retried and marked both as new and modified', (done) => {
+        it('should be retried and marked both as new and modified', async () => {
           receiver.setKnownTests({
             cucumber: {}
           })
@@ -2784,7 +2784,7 @@ versions.forEach(version => {
             },
             known_tests_enabled: true
           })
-          runImpactedTest(done, { isModified: true, isEfd: true, isNew: true })
+          await runImpactedTest({ isModified: true, isEfd: true, isNew: true })
         })
       })
     })

packages/datadog-instrumentations/src/cucumber.js

@@ -25,7 +25,7 @@ const skippableSuitesCh = channel('ci:cucumber:test-suite:skippable')
 const sessionStartCh = channel('ci:cucumber:session:start')
 const sessionFinishCh = channel('ci:cucumber:session:finish')
 const testManagementTestsCh = channel('ci:cucumber:test-management-tests')
-const impactedTestsCh = channel('ci:cucumber:modified-tests')
+const modifiedFilesCh = channel('ci:cucumber:modified-files')
 const isModifiedCh = channel('ci:cucumber:is-modified-test')
 
 const workerReportTraceCh = channel('ci:cucumber:worker-report:trace')
@@ -80,7 +80,7 @@ let isTestManagementTestsEnabled = false
 let isImpactedTestsEnabled = false
 let testManagementAttemptToFixRetries = 0
 let testManagementTests = {}
-let modifiedTests = {}
+let modifiedFiles = {}
 let numTestRetries = 0
 let knownTests = {}
 let skippedSuites = []
@@ -537,9 +537,9 @@ function getWrappedStart (start, frameworkVersion, isParallel = false, isCoordin
     }
 
     if (isImpactedTestsEnabled) {
-      const impactedTestsResponse = await getChannelPromise(impactedTestsCh)
+      const impactedTestsResponse = await getChannelPromise(modifiedFilesCh)
       if (!impactedTestsResponse.err) {
-        modifiedTests = impactedTestsResponse.modifiedTests
+        modifiedFiles = impactedTestsResponse.modifiedFiles
       }
     }
 
@@ -655,7 +655,7 @@ function getWrappedRunTestCase (runTestCaseFunction, isNewerCucumberVersion = fa
       isModifiedCh.publish({
         scenarios,
         testFileAbsolutePath: gherkinDocument.uri,
-        modifiedTests,
+        modifiedFiles,
         stepIds,
         stepDefinitions: this.supportCodeLibrary.stepDefinitions,
         setIsModified
@@ -1000,7 +1000,7 @@ addHook({
 
     if (isImpactedTestsEnabled) {
       this.options.worldParameters._ddImpactedTestsEnabled = isImpactedTestsEnabled
-      this.options.worldParameters._ddModifiedTests = modifiedTests
+      this.options.worldParameters._ddModifiedFiles = modifiedFiles
     }
 
     return startWorker.apply(this, arguments)
@@ -1036,7 +1036,7 @@ addHook({
       }
       isImpactedTestsEnabled = !!this.options.worldParameters._ddImpactedTestsEnabled
       if (isImpactedTestsEnabled) {
-        modifiedTests = this.options.worldParameters._ddModifiedTests
+        modifiedFiles = this.options.worldParameters._ddModifiedFiles
       }
     }
   )

packages/datadog-instrumentations/src/jest.js

@@ -46,7 +46,7 @@ const skippableSuitesCh = channel('ci:jest:test-suite:skippable')
 const libraryConfigurationCh = channel('ci:jest:library-configuration')
 const knownTestsCh = channel('ci:jest:known-tests')
 const testManagementTestsCh = channel('ci:jest:test-management-tests')
-const impactedTestsCh = channel('ci:jest:modified-tests')
+const modifiedFilesCh = channel('ci:jest:modified-files')
 
 const itrSkippedSuitesCh = channel('ci:jest:itr:skipped-suites')
 
@@ -78,7 +78,7 @@ let isTestManagementTestsEnabled = false
 let testManagementTests = {}
 let testManagementAttemptToFixRetries = 0
 let isImpactedTestsEnabled = false
-let modifiedTests = {}
+let modifiedFiles = {}
 
 const testContexts = new WeakMap()
 const originalTestFns = new WeakMap()
@@ -197,10 +197,8 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {
 
       if (this.isImpactedTestsEnabled) {
         try {
-          const hasImpactedTests = Object.keys(modifiedTests).length > 0
-          this.modifiedTestsForThisSuite = hasImpactedTests
-            ? this.getModifiedTestForThisSuite(modifiedTests)
-            : this.getModifiedTestForThisSuite(this.testEnvironmentOptions._ddModifiedTests)
+          const hasImpactedTests = Object.keys(modifiedFiles).length > 0
+          this.modifiedFiles = hasImpactedTests ? modifiedFiles : this.testEnvironmentOptions._ddModifiedFiles
         } catch (e) {
           log.error('Error parsing impacted tests', e)
           this.isImpactedTestsEnabled = false
@@ -290,19 +288,6 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {
       return result
     }
 
-    getModifiedTestForThisSuite (modifiedTests) {
-      if (this.modifiedTestsForThisSuite) {
-        return this.modifiedTestsForThisSuite
-      }
-      let modifiedTestsForThisSuite = modifiedTests
-      // If jest is using workers, modified tests are serialized to json.
-      // If jest runs in band, they are not.
-      if (typeof modifiedTestsForThisSuite === 'string') {
-        modifiedTestsForThisSuite = JSON.parse(modifiedTestsForThisSuite)
-      }
-      return modifiedTestsForThisSuite
-    }
-
     // Generic function to handle test retries
     retryTest ({
       jestEvent,
@@ -378,7 +363,7 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {
             this.testSourceFile,
             testStartLine,
             testEndLine,
-            this.modifiedTestsForThisSuite,
+            this.modifiedFiles,
             'jest'
           )
         }
@@ -465,7 +450,7 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {
             this.testSourceFile,
             testStartLine,
             testEndLine,
-            this.modifiedTestsForThisSuite,
+            this.modifiedFiles,
             'jest'
           )
           if (isModified && !retriedTestsToNumAttempts.has(testFullName) && this.isEarlyFlakeDetectionEnabled) {
@@ -831,12 +816,12 @@ function getCliWrapper (isNewJestVersion) {
           onDone = resolve
         })
 
-        impactedTestsCh.publish({ onDone })
+        modifiedFilesCh.publish({ onDone })
 
         try {
-          const { err, modifiedTests: receivedModifiedTests } = await impactedTestsPromise
+          const { err, modifiedFiles: receivedModifiedFiles } = await impactedTestsPromise
           if (!err) {
-            modifiedTests = receivedModifiedTests
+            modifiedFiles = receivedModifiedFiles
           }
         } catch (err) {
           log.error('Jest impacted tests error', err)
@@ -1237,7 +1222,7 @@ addHook({
       _ddIsTestManagementTestsEnabled,
       _ddTestManagementTests,
       _ddTestManagementAttemptToFixRetries,
-      _ddModifiedTests,
+      _ddModifiedFiles,
       ...restOfTestEnvironmentOptions
     } = testEnvironmentOptions
 
@@ -1365,17 +1350,15 @@ function sendWrapper (send) {
 
       const suiteTestManagementTests = testManagementTests?.jest?.suites?.[testSuite]?.tests || {}
 
-      const suiteModifiedTests = Object.keys(modifiedTests).length > 0
-        ? modifiedTests
-        : {}
-
       args[0].config = {
         ...config,
         testEnvironmentOptions: {
           ...config.testEnvironmentOptions,
           _ddKnownTests: suiteKnownTests,
           _ddTestManagementTests: suiteTestManagementTests,
-          _ddModifiedTests: suiteModifiedTests
+          // TODO: figure out if we can reduce the size of the modified files object
+          // Can we use `testSuite` (it'd have to be relative to repository root though)
+          _ddModifiedFiles: modifiedFiles
         }
       }
     }

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

@@ -68,7 +68,7 @@ const skippableSuitesCh = channel('ci:mocha:test-suite:skippable')
 const mochaGlobalRunCh = channel('ci:mocha:global:run')
 
 const testManagementTestsCh = channel('ci:mocha:test-management-tests')
-const impactedTestsCh = channel('ci:mocha:modified-tests')
+const modifiedFilesCh = channel('ci:mocha:modified-files')
 const workerReportTraceCh = channel('ci:mocha:worker-report:trace')
 const testSessionStartCh = channel('ci:mocha:session:start')
 const testSessionFinishCh = channel('ci:mocha:session:finish')
@@ -233,12 +233,12 @@ function getExecutionConfiguration (runner, isParallel, frameworkVersion, onFini
     })
   }
 
-  const onReceivedImpactedTests = ({ err, modifiedTests: receivedModifiedTests }) => {
+  const onReceivedImpactedTests = ({ err, modifiedFiles: receivedModifiedFiles }) => {
     if (err) {
-      config.modifiedTests = []
+      config.modifiedFiles = []
       config.isImpactedTestsEnabled = false
     } else {
-      config.modifiedTests = receivedModifiedTests
+      config.modifiedFiles = receivedModifiedFiles
     }
     if (config.isSuitesSkippingEnabled) {
       ctx.onDone = onReceivedSkippableSuites
@@ -260,7 +260,7 @@ function getExecutionConfiguration (runner, isParallel, frameworkVersion, onFini
     }
     if (config.isImpactedTestsEnabled) {
       ctx.onDone = onReceivedImpactedTests
-      impactedTestsCh.runStores(ctx, () => {})
+      modifiedFilesCh.runStores(ctx, () => {})
     } else if (config.isSuitesSkippingEnabled) {
       ctx.onDone = onReceivedSkippableSuites
       skippableSuitesCh.runStores(ctx, () => {})
@@ -284,7 +284,7 @@ function getExecutionConfiguration (runner, isParallel, frameworkVersion, onFini
       testManagementTestsCh.runStores(ctx, () => {})
     } if (config.isImpactedTestsEnabled) {
       ctx.onDone = onReceivedImpactedTests
-      impactedTestsCh.runStores(ctx, () => {})
+      modifiedFilesCh.runStores(ctx, () => {})
     } else if (config.isSuitesSkippingEnabled) {
       ctx.onDone = onReceivedSkippableSuites
       skippableSuitesCh.runStores(ctx, () => {})
@@ -321,7 +321,7 @@ function getExecutionConfiguration (runner, isParallel, frameworkVersion, onFini
       testManagementTestsCh.runStores(ctx, () => {})
     } else if (config.isImpactedTestsEnabled) {
       ctx.onDone = onReceivedImpactedTests
-      impactedTestsCh.runStores(ctx, () => {})
+      modifiedFilesCh.runStores(ctx, () => {})
     } else if (config.isSuitesSkippingEnabled) {
       ctx.onDone = onReceivedSkippableSuites
       skippableSuitesCh.runStores(ctx, () => {})
@@ -696,9 +696,8 @@ addHook({
     }
 
     if (config.isImpactedTestsEnabled) {
-      const testSuiteImpactedTests = config.modifiedTests || {}
       newWorkerArgs._ddIsImpactedTestsEnabled = true
-      newWorkerArgs._ddModifiedTests = testSuiteImpactedTests
+      newWorkerArgs._ddModifiedFiles = config.modifiedFiles || {}
     }
 
     // We pass the known tests for the test file to the worker

packages/datadog-instrumentations/src/mocha/utils.js

@@ -446,7 +446,7 @@ function getRunTestsWrapper (runTests, config) {
     if (config.isImpactedTestsEnabled) {
       suite.tests.forEach((test) => {
         isModifiedCh.publish({
-          modifiedTests: config.modifiedTests,
+          modifiedFiles: config.modifiedFiles,
           file: suite.file,
           onDone: (isModified) => {
             if (isModified) {

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

@@ -36,9 +36,9 @@ addHook({
     }
     if (this.options._ddIsImpactedTestsEnabled) {
       config.isImpactedTestsEnabled = true
-      config.modifiedTests = this.options._ddModifiedTests
+      config.modifiedFiles = this.options._ddModifiedFiles
       delete this.options._ddIsImpactedTestsEnabled
-      delete this.options._ddModifiedTests
+      delete this.options._ddModifiedFiles
     }
     if (this.options._ddIsTestManagementTestsEnabled) {
       config.isTestManagementTestsEnabled = true

packages/datadog-instrumentations/src/playwright.js

@@ -21,7 +21,7 @@ const testSessionFinishCh = channel('ci:playwright:session:finish')
 const libraryConfigurationCh = channel('ci:playwright:library-configuration')
 const knownTestsCh = channel('ci:playwright:known-tests')
 const testManagementTestsCh = channel('ci:playwright:test-management-tests')
-const impactedTestsCh = channel('ci:playwright:modified-tests')
+const modifiedFilesCh = channel('ci:playwright:modified-files')
 const isModifiedCh = channel('ci:playwright:test:is-modified')
 
 const testSuiteStartCh = channel('ci:playwright:test-suite:start')
@@ -59,7 +59,7 @@ let isTestManagementTestsEnabled = false
 let testManagementAttemptToFixRetries = 0
 let testManagementTests = {}
 let isImpactedTestsEnabled = false
-let modifiedTests = {}
+let modifiedFiles = {}
 const quarantinedOrDisabledTestsAttemptToFix = []
 let quarantinedButNotAttemptToFixFqns = new Set()
 let rootDir = ''
@@ -593,11 +593,11 @@ function runAllTestsWrapper (runAllTests, playwrightVersion) {
 
     if (isImpactedTestsEnabled && satisfies(playwrightVersion, MINIMUM_SUPPORTED_VERSION_RANGE_EFD)) {
       try {
-        const { err, modifiedTests: receivedModifiedTests } = await getChannelPromise(impactedTestsCh)
+        const { err, modifiedFiles: receivedModifiedFiles } = await getChannelPromise(modifiedFilesCh)
         if (err) {
           isImpactedTestsEnabled = false
         } else {
-          modifiedTests = receivedModifiedTests
+          modifiedFiles = receivedModifiedFiles
         }
       } catch (err) {
         isImpactedTestsEnabled = false
@@ -819,7 +819,7 @@ addHook({
       await Promise.all(allTests.map(async (test) => {
         const { isModified } = await getChannelPromise(isModifiedCh, {
           filePath: test._requireFile,
-          modifiedTests
+          modifiedFiles
         })
         if (isModified) {
           test._ddIsModified = true