From 8eaacf2197a91ffadf1dee940671c5d3957e1a01 Mon Sep 17 00:00:00 2001 From: Juan Fernandez Date: Thu, 13 Feb 2025 17:09:02 +0100 Subject: [PATCH] fix vitest logic --- integration-tests/vitest/vitest.spec.js | 160 +++++------------- .../datadog-instrumentations/src/vitest.js | 40 +++-- packages/datadog-plugin-vitest/src/index.js | 7 +- 3 files changed, 70 insertions(+), 137 deletions(-) diff --git a/integration-tests/vitest/vitest.spec.js b/integration-tests/vitest/vitest.spec.js index c518f71d084..47166f5f4e2 100644 --- a/integration-tests/vitest/vitest.spec.js +++ b/integration-tests/vitest/vitest.spec.js @@ -1356,14 +1356,9 @@ versions.forEach((version) => { } }) }) - it('can quarantine tests', (done) => { - receiver.setSettings({ - test_management: { - enabled: true - } - }) - const eventsPromise = receiver + const getTestAssertions = (isQuarantining) => + receiver .gatherPayloadsMaxTimeout(({ url }) => url === '/api/v2/citestcycle', payloads => { const events = payloads.flatMap(({ payload }) => payload.events) const tests = events.filter(event => event.type === 'test').map(event => event.content) @@ -1371,7 +1366,11 @@ versions.forEach((version) => { const testSession = events.find(event => event.type === 'test_session_end').content - assert.propertyVal(testSession.meta, TEST_MANAGEMENT_ENABLED, 'true') + if (isQuarantining) { + assert.propertyVal(testSession.meta, TEST_MANAGEMENT_ENABLED, 'true') + } else { + assert.notProperty(testSession.meta, TEST_MANAGEMENT_ENABLED) + } const resourceNames = tests.map(span => span.resource) @@ -1382,73 +1381,22 @@ versions.forEach((version) => { ] ) - const failedTest = tests.find( + const quarantinedTest = tests.find( test => test.meta[TEST_NAME] === 'quarantine tests can quarantine a test' ) - // The test fails but the exit code is still 0 - // TODO: still mark it as fail - // TODO: add quarantine tag - // assert.equal(failedTest.meta[TEST_STATUS], 'fail') - // assert.equal(failedTest.meta[TEST_MANAGEMENT_IS_QUARANTINED], 'true') - }) - - childProcess = exec( - './node_modules/.bin/vitest run', - { - cwd, - env: { - ...getCiVisAgentlessConfig(receiver.port), - TEST_DIR: 'ci-visibility/vitest-tests/test-quarantine*', - NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init --no-warnings' - }, - stdio: 'inherit' - } - ) - - childProcess.stdout.pipe(process.stdout) - childProcess.stderr.pipe(process.stderr) - childProcess.on('exit', (exitCode) => { - eventsPromise.then(() => { - // exit code 0 even though one of the tests failed - assert.equal(exitCode, 0) - done() + if (isQuarantining) { + // TODO: do not flip the status of the test but still ignore failures + assert.equal(quarantinedTest.meta[TEST_STATUS], 'pass') + assert.propertyVal(quarantinedTest.meta, TEST_MANAGEMENT_IS_QUARANTINED, 'true') + } else { + assert.equal(quarantinedTest.meta[TEST_STATUS], 'fail') + assert.notProperty(quarantinedTest.meta, TEST_MANAGEMENT_IS_QUARANTINED) + } }) - }) - }) - it('fails if quarantine is not enabled', (done) => { - receiver.setSettings({ - test_management: { - enabled: false - } - }) - - const eventsPromise = receiver - .gatherPayloadsMaxTimeout(({ url }) => url === '/api/v2/citestcycle', payloads => { - const events = payloads.flatMap(({ payload }) => payload.events) - const tests = events.filter(event => event.type === 'test').map(event => event.content) - assert.equal(tests.length, 2) - - const testSession = events.find(event => event.type === 'test_session_end').content - - assert.notProperty(testSession.meta, TEST_MANAGEMENT_ENABLED) - - const resourceNames = tests.map(span => span.resource) - - assert.includeMembers(resourceNames, - [ - 'ci-visibility/vitest-tests/test-quarantine.mjs.quarantine tests can quarantine a test', - 'ci-visibility/vitest-tests/test-quarantine.mjs.quarantine tests can pass normally' - ] - ) - - const failedTest = tests.find( - test => test.meta[TEST_NAME] === 'quarantine tests can quarantine a test' - ) - assert.equal(failedTest.meta[TEST_STATUS], 'fail') - assert.notProperty(failedTest.meta, TEST_MANAGEMENT_IS_QUARANTINED) - }) + const runQuarantineTest = (done, isQuarantining, extraEnvVars = {}) => { + const testAssertionsPromise = getTestAssertions(isQuarantining) childProcess = exec( './node_modules/.bin/vitest run', @@ -1457,72 +1405,42 @@ versions.forEach((version) => { env: { ...getCiVisAgentlessConfig(receiver.port), TEST_DIR: 'ci-visibility/vitest-tests/test-quarantine*', - NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init --no-warnings' + NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init --no-warnings', + ...extraEnvVars }, stdio: 'inherit' } ) childProcess.on('exit', (exitCode) => { - eventsPromise.then(() => { - assert.equal(exitCode, 1) + testAssertionsPromise.then(() => { + if (isQuarantining) { + // exit code 0 even though one of the tests failed + assert.equal(exitCode, 0) + } else { + assert.equal(exitCode, 1) + } done() }) }) - }) - - it('does not enable quarantine tests if DD_TEST_MANAGEMENT_ENABLED is set to false', (done) => { - receiver.setSettings({ - test_management: { - enabled: true - } - }) - const eventsPromise = receiver - .gatherPayloadsMaxTimeout(({ url }) => url === '/api/v2/citestcycle', payloads => { - const events = payloads.flatMap(({ payload }) => payload.events) - const tests = events.filter(event => event.type === 'test').map(event => event.content) - assert.equal(tests.length, 2) - - const testSession = events.find(event => event.type === 'test_session_end').content + } - assert.notProperty(testSession.meta, TEST_MANAGEMENT_ENABLED) + it('can quarantine tests', (done) => { + receiver.setSettings({ test_management: { enabled: true } }) - const resourceNames = tests.map(span => span.resource) + runQuarantineTest(done, true) + }) - assert.includeMembers(resourceNames, - [ - 'ci-visibility/vitest-tests/test-quarantine.mjs.quarantine tests can quarantine a test', - 'ci-visibility/vitest-tests/test-quarantine.mjs.quarantine tests can pass normally' - ] - ) + it('fails if quarantine is not enabled', (done) => { + receiver.setSettings({ test_management: { enabled: false } }) - const failedTest = tests.find( - test => test.meta[TEST_NAME] === 'quarantine tests can quarantine a test' - ) - assert.equal(failedTest.meta[TEST_STATUS], 'fail') - assert.notProperty(failedTest.meta, TEST_MANAGEMENT_IS_QUARANTINED) - }) + runQuarantineTest(done, false) + }) - childProcess = exec( - './node_modules/.bin/vitest run', - { - cwd, - env: { - ...getCiVisAgentlessConfig(receiver.port), - TEST_DIR: 'ci-visibility/vitest-tests/test-quarantine*', - NODE_OPTIONS: '--import dd-trace/register.js -r dd-trace/ci/init --no-warnings', - DD_TEST_MANAGEMENT_ENABLED: 'false' - }, - stdio: 'inherit' - } - ) + it('does not enable quarantine tests if DD_TEST_MANAGEMENT_ENABLED is set to false', (done) => { + receiver.setSettings({ test_management: { enabled: true } }) - childProcess.on('exit', (exitCode) => { - eventsPromise.then(() => { - assert.equal(exitCode, 1) - done() - }) - }) + runQuarantineTest(done, false, { DD_TEST_MANAGEMENT_ENABLED: '0' }) }) }) } diff --git a/packages/datadog-instrumentations/src/vitest.js b/packages/datadog-instrumentations/src/vitest.js index f23233fd7b9..340fd188340 100644 --- a/packages/datadog-instrumentations/src/vitest.js +++ b/packages/datadog-instrumentations/src/vitest.js @@ -27,6 +27,7 @@ const quarantinedTestsCh = channel('ci:vitest:quarantined-tests') const taskToAsync = new WeakMap() const taskToStatuses = new WeakMap() const newTasks = new WeakSet() +const quarantinedTasks = new WeakSet() let isRetryReasonEfd = false const switchedStatuses = new WeakSet() const sessionAsyncResource = new AsyncResource('bound-anonymous-fn') @@ -361,7 +362,7 @@ addHook({ // `onAfterRunTask` is run after all repetitions or attempts are run shimmer.wrap(VitestTestRunner.prototype, 'onAfterRunTask', onAfterRunTask => async function (task) { - const { isEarlyFlakeDetectionEnabled, isQuarantinedTestsEnabled, quarantinedTests } = getProvidedContext() + const { isEarlyFlakeDetectionEnabled, isQuarantinedTestsEnabled } = getProvidedContext() if (isEarlyFlakeDetectionEnabled && taskToStatuses.has(task)) { const statuses = taskToStatuses.get(task) @@ -375,18 +376,9 @@ addHook({ } if (isQuarantinedTestsEnabled) { - const testName = getTestName(task) - - isQuarantinedCh.publish({ - quarantinedTests, - testSuiteAbsolutePath: task.file.filepath, - testName, - onDone: (isQuarantined) => { - if (isQuarantined) { - task.result.state = 'pass' - } - } - }) + if (quarantinedTasks.has(task)) { + task.result.state = 'pass' + } } return onAfterRunTask.apply(this, arguments) @@ -400,17 +392,34 @@ addHook({ } const testName = getTestName(task) let isNew = false + let isQuarantined = false const { isKnownTestsEnabled, isEarlyFlakeDetectionEnabled, - isDiEnabled + isDiEnabled, + isQuarantinedTestsEnabled, + quarantinedTests } = getProvidedContext() if (isKnownTestsEnabled) { isNew = newTasks.has(task) } + if (isQuarantinedTestsEnabled) { + isQuarantinedCh.publish({ + quarantinedTests, + testSuiteAbsolutePath: task.file.filepath, + testName, + onDone: (isTestQuarantined) => { + isQuarantined = isTestQuarantined + if (isTestQuarantined) { + quarantinedTasks.add(task) + } + } + }) + } + const { retry: numAttempt, repeats: numRepetition } = retryInfo // We finish the previous test here because we know it has failed already @@ -492,7 +501,8 @@ addHook({ isRetry: numAttempt > 0 || numRepetition > 0, isRetryReasonEfd, isNew, - mightHitProbe: isDiEnabled && numAttempt > 0 + mightHitProbe: isDiEnabled && numAttempt > 0, + isQuarantined }) }) return onBeforeTryTask.apply(this, arguments) diff --git a/packages/datadog-plugin-vitest/src/index.js b/packages/datadog-plugin-vitest/src/index.js index 2127f481aaf..6b0b5cabf60 100644 --- a/packages/datadog-plugin-vitest/src/index.js +++ b/packages/datadog-plugin-vitest/src/index.js @@ -19,7 +19,8 @@ const { TEST_EARLY_FLAKE_ENABLED, TEST_EARLY_FLAKE_ABORT_REASON, TEST_RETRY_REASON, - TEST_MANAGEMENT_ENABLED + TEST_MANAGEMENT_ENABLED, + TEST_MANAGEMENT_IS_QUARANTINED } = require('../../dd-trace/src/plugins/util/test') const { COMPONENT } = require('../../dd-trace/src/constants') const { @@ -81,6 +82,7 @@ class VitestPlugin extends CiPlugin { testSuiteAbsolutePath, isRetry, isNew, + isQuarantined, mightHitProbe, isRetryReasonEfd }) => { @@ -99,6 +101,9 @@ class VitestPlugin extends CiPlugin { if (isRetryReasonEfd) { extraTags[TEST_RETRY_REASON] = 'efd' } + if (isQuarantined) { + extraTags[TEST_MANAGEMENT_IS_QUARANTINED] = 'true' + } const span = this.startTestSpan( testName,