diff --git a/integration-tests/ci-visibility/dynamic-instrumentation/is-jest.js b/integration-tests/ci-visibility/dynamic-instrumentation/is-jest.js deleted file mode 100644 index 483b2a543d3..00000000000 --- a/integration-tests/ci-visibility/dynamic-instrumentation/is-jest.js +++ /dev/null @@ -1,7 +0,0 @@ -module.exports = function () { - try { - return typeof jest !== 'undefined' - } catch (e) { - return false - } -} diff --git a/integration-tests/ci-visibility/dynamic-instrumentation/test-hit-breakpoint.js b/integration-tests/ci-visibility/dynamic-instrumentation/test-hit-breakpoint.js index ed2e3d14e51..57f1762edf9 100644 --- a/integration-tests/ci-visibility/dynamic-instrumentation/test-hit-breakpoint.js +++ b/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', () => { diff --git a/integration-tests/ci-visibility/dynamic-instrumentation/test-not-hit-breakpoint.js b/integration-tests/ci-visibility/dynamic-instrumentation/test-not-hit-breakpoint.js index 7960852a52c..bf051a37754 100644 --- a/integration-tests/ci-visibility/dynamic-instrumentation/test-not-hit-breakpoint.js +++ b/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 diff --git a/integration-tests/jest/jest.spec.js b/integration-tests/jest/jest.spec.js index 47a5af89b85..ac604d96b5e 100644 --- a/integration-tests/jest/jest.spec.js +++ b/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 diff --git a/integration-tests/mocha/mocha.spec.js b/integration-tests/mocha/mocha.spec.js index 1bb369c0627..a7c23b067df 100644 --- a/integration-tests/mocha/mocha.spec.js +++ b/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' } diff --git a/integration-tests/vitest/vitest.spec.js b/integration-tests/vitest/vitest.spec.js index bad651e6f83..eb2fe21ba78 100644 --- a/integration-tests/vitest/vitest.spec.js +++ b/integration-tests/vitest/vitest.spec.js @@ -35,7 +35,7 @@ const { DD_HOST_CPU_COUNT } = require('../../packages/dd-trace/src/plugins/util/ const NUM_RETRIES_EFD = 3 -const versions = ['2.1.8'] // was previously 'latest', but v3 breaks this test +const versions = ['1.6.0', 'latest'] const linePctMatchRegex = /Lines\s+:\s+([\d.]+)%/ diff --git a/packages/datadog-instrumentations/src/jest.js b/packages/datadog-instrumentations/src/jest.js index 898927aeaff..7a1001d11f3 100644 --- a/packages/datadog-instrumentations/src/jest.js +++ b/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() diff --git a/packages/datadog-instrumentations/src/vitest.js b/packages/datadog-instrumentations/src/vitest.js index f623882352e..b3f2a9af8b8 100644 --- a/packages/datadog-instrumentations/src/vitest.js +++ b/packages/datadog-instrumentations/src/vitest.js @@ -179,7 +179,9 @@ function getSortWrapper (sort) { const knownTestsResponse = await getChannelPromise(knownTestsCh) if (!knownTestsResponse.err) { knownTests = knownTestsResponse.knownTests - const testFilepaths = await this.ctx.getTestFilepaths() + const getFilePaths = this.ctx.getTestFilepaths || this.ctx._globTestFilepaths + + const testFilepaths = await getFilePaths.call(this.ctx) isEarlyFlakeDetectionFaultyCh.publish({ knownTests: knownTests.vitest || {}, @@ -492,15 +494,6 @@ addHook({ return vitestPackage }) -addHook({ - name: 'vitest', - versions: ['>=2.1.0'], - filePattern: 'dist/chunks/RandomSequencer.*' -}, (randomSequencerPackage) => { - shimmer.wrap(randomSequencerPackage.B.prototype, 'sort', getSortWrapper) - return randomSequencerPackage -}) - addHook({ name: 'vitest', versions: ['>=2.0.5 <2.1.0'], @@ -513,6 +506,24 @@ addHook({ return vitestPackage }) +addHook({ + name: 'vitest', + versions: ['>=2.1.0 <3.0.0'], + filePattern: 'dist/chunks/RandomSequencer.*' +}, (randomSequencerPackage) => { + shimmer.wrap(randomSequencerPackage.B.prototype, 'sort', getSortWrapper) + return randomSequencerPackage +}) + +addHook({ + name: 'vitest', + versions: ['>=3.0.0'], + filePattern: 'dist/chunks/resolveConfig.*' +}, (randomSequencerPackage) => { + shimmer.wrap(randomSequencerPackage.B.prototype, 'sort', getSortWrapper) + return randomSequencerPackage +}) + // Can't specify file because compiled vitest includes hashes in their files addHook({ name: 'vitest', @@ -533,15 +544,17 @@ addHook({ versions: ['>=1.6.0'], file: 'dist/index.js' }, (vitestPackage, frameworkVersion) => { - shimmer.wrap(vitestPackage, 'startTests', startTests => async function (testPath) { + shimmer.wrap(vitestPackage, 'startTests', startTests => async function (testPaths) { let testSuiteError = null if (!testSuiteStartCh.hasSubscribers) { return startTests.apply(this, arguments) } + // From >=3.0.1, the first arguments changes from a string to an object containing the filepath + const testSuiteAbsolutePath = testPaths[0]?.filepath || testPaths[0] const testSuiteAsyncResource = new AsyncResource('bound-anonymous-fn') testSuiteAsyncResource.runInAsyncScope(() => { - testSuiteStartCh.publish({ testSuiteAbsolutePath: testPath[0], frameworkVersion }) + testSuiteStartCh.publish({ testSuiteAbsolutePath, frameworkVersion }) }) const startTestsResponse = await startTests.apply(this, arguments) diff --git a/packages/dd-trace/src/ci-visibility/test-api-manual/test-api-manual-plugin.js b/packages/dd-trace/src/ci-visibility/test-api-manual/test-api-manual-plugin.js index 8e0b9351b06..8a0ba970bc9 100644 --- a/packages/dd-trace/src/ci-visibility/test-api-manual/test-api-manual-plugin.js +++ b/packages/dd-trace/src/ci-visibility/test-api-manual/test-api-manual-plugin.js @@ -13,15 +13,16 @@ class TestApiManualPlugin extends CiPlugin { constructor (...args) { super(...args) + this._isEnvDataCalcualted = false this.sourceRoot = process.cwd() - this.addSub('dd-trace:ci:manual:test:start', ({ testName, testSuite }) => { + this.unconfiguredAddSub('dd-trace:ci:manual:test:start', ({ testName, testSuite }) => { const store = storage.getStore() const testSuiteRelative = getTestSuitePath(testSuite, this.sourceRoot) const testSpan = this.startTestSpan(testName, testSuiteRelative) this.enter(testSpan, store) }) - this.addSub('dd-trace:ci:manual:test:finish', ({ status, error }) => { + this.unconfiguredAddSub('dd-trace:ci:manual:test:finish', ({ status, error }) => { const store = storage.getStore() const testSpan = store && store.span if (testSpan) { @@ -33,7 +34,7 @@ class TestApiManualPlugin extends CiPlugin { finishAllTraceSpans(testSpan) } }) - this.addSub('dd-trace:ci:manual:test:addTags', (tags) => { + this.unconfiguredAddSub('dd-trace:ci:manual:test:addTags', (tags) => { const store = storage.getStore() const testSpan = store && store.span if (testSpan) { @@ -41,6 +42,22 @@ class TestApiManualPlugin extends CiPlugin { } }) } + + // To lazily calculate env data. + unconfiguredAddSub (channelName, handler) { + this.addSub(channelName, (...args) => { + if (!this._isEnvDataCalcualted) { + this._isEnvDataCalcualted = true + this.configure(this._config, true) + } + return handler(...args) + }) + } + + configure (config, shouldGetEnvironmentData) { + this._config = config + super.configure(config, shouldGetEnvironmentData) + } } module.exports = TestApiManualPlugin diff --git a/packages/dd-trace/src/debugger/devtools_client/index.js b/packages/dd-trace/src/debugger/devtools_client/index.js index be466b06bd9..55afe4e62a2 100644 --- a/packages/dd-trace/src/debugger/devtools_client/index.js +++ b/packages/dd-trace/src/debugger/devtools_client/index.js @@ -146,10 +146,9 @@ session.on('Debugger.paused', async ({ params }) => { } } + ackEmitting(probe) // TODO: Process template (DEBUG-2628) - send(probe.template, logger, dd, snapshot, () => { - ackEmitting(probe) - }) + send(probe.template, logger, dd, snapshot) } }) diff --git a/packages/dd-trace/src/debugger/devtools_client/send.js b/packages/dd-trace/src/debugger/devtools_client/send.js index 12d9b8cad84..ce42e6d8b36 100644 --- a/packages/dd-trace/src/debugger/devtools_client/send.js +++ b/packages/dd-trace/src/debugger/devtools_client/send.js @@ -29,10 +29,9 @@ const ddtags = [ const path = `/debugger/v1/input?${stringify({ ddtags })}` -let callbacks = [] const jsonBuffer = new JSONBuffer({ size: config.maxTotalPayloadSize, timeout: 1000, onFlush }) -function send (message, logger, dd, snapshot, cb) { +function send (message, logger, dd, snapshot) { const payload = { ddsource, hostname, @@ -58,7 +57,6 @@ function send (message, logger, dd, snapshot, cb) { } jsonBuffer.write(json, size) - callbacks.push(cb) } function onFlush (payload) { @@ -69,11 +67,7 @@ function onFlush (payload) { headers: { 'Content-Type': 'application/json; charset=utf-8' } } - const _callbacks = callbacks - callbacks = [] - request(payload, opts, (err) => { - if (err) log.error('Could not send debugger payload', err) - else _callbacks.forEach(cb => cb()) + if (err) log.error('[debugger:devtools_client] Error sending probe payload', err) }) } diff --git a/packages/dd-trace/src/debugger/devtools_client/status.js b/packages/dd-trace/src/debugger/devtools_client/status.js index 7a7db799e53..541b03157f3 100644 --- a/packages/dd-trace/src/debugger/devtools_client/status.js +++ b/packages/dd-trace/src/debugger/devtools_client/status.js @@ -94,7 +94,7 @@ function onFlush (payload) { } request(form, options, (err) => { - if (err) log.error('[debugger:devtools_client] Error sending probe payload', err) + if (err) log.error('[debugger:devtools_client] Error sending diagnostics payload', err) }) } diff --git a/packages/dd-trace/src/plugins/ci_plugin.js b/packages/dd-trace/src/plugins/ci_plugin.js index 74c0961b1e0..60c1c59a9bc 100644 --- a/packages/dd-trace/src/plugins/ci_plugin.js +++ b/packages/dd-trace/src/plugins/ci_plugin.js @@ -184,14 +184,18 @@ module.exports = class CiPlugin extends Plugin { } } - configure (config) { + configure (config, shouldGetEnvironmentData = true) { super.configure(config) - if (config.isTestDynamicInstrumentationEnabled) { + 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/src/plugins/util/test.js b/packages/dd-trace/src/plugins/util/test.js index 11181e1d9eb..d8aab1a44da 100644 --- a/packages/dd-trace/src/plugins/util/test.js +++ b/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 } diff --git a/packages/dd-trace/src/proxy.js b/packages/dd-trace/src/proxy.js index a5d91d7761e..ccadb734021 100644 --- a/packages/dd-trace/src/proxy.js +++ b/packages/dd-trace/src/proxy.js @@ -166,7 +166,10 @@ class Tracer extends NoopProxy { if (config.isManualApiEnabled) { const TestApiManualPlugin = require('./ci-visibility/test-api-manual/test-api-manual-plugin') this._testApiManualPlugin = new TestApiManualPlugin(this) - this._testApiManualPlugin.configure({ ...config, enabled: true }) + // `shouldGetEnvironmentData` is passed as false so that we only lazily calculate it + // This is the only place where we need to do this because the rest of the plugins + // are lazily configured when the library is imported. + this._testApiManualPlugin.configure({ ...config, enabled: true }, false) } } if (config.ciVisAgentlessLogSubmissionEnabled) { diff --git a/packages/dd-trace/test/debugger/devtools_client/send.spec.js b/packages/dd-trace/test/debugger/devtools_client/send.spec.js index ea4551d8ff6..d94a0a0140f 100644 --- a/packages/dd-trace/test/debugger/devtools_client/send.spec.js +++ b/packages/dd-trace/test/debugger/devtools_client/send.spec.js @@ -54,13 +54,9 @@ describe('input message http requests', function () { }) it('should call request with the expected payload once the buffer is flushed', function (done) { - const callback1 = sinon.spy() - const callback2 = sinon.spy() - const callback3 = sinon.spy() - - send({ message: 1 }, logger, dd, snapshot, callback1) - send({ message: 2 }, logger, dd, snapshot, callback2) - send({ message: 3 }, logger, dd, snapshot, callback3) + send({ message: 1 }, logger, dd, snapshot) + send({ message: 2 }, logger, dd, snapshot) + send({ message: 3 }, logger, dd, snapshot) expect(request).to.not.have.been.called expectWithin(1200, () => { @@ -83,16 +79,6 @@ describe('input message http requests', function () { `git.repository_url%3A${repositoryUrl}` ) - expect(callback1).to.not.have.been.calledOnce - expect(callback2).to.not.have.been.calledOnce - expect(callback3).to.not.have.been.calledOnce - - request.firstCall.callback() - - expect(callback1).to.have.been.calledOnce - expect(callback2).to.have.been.calledOnce - expect(callback3).to.have.been.calledOnce - done() }) }) diff --git a/yarn.lock b/yarn.lock index 4d8e42d2abc..dbdfa31ed73 100644 --- a/yarn.lock +++ b/yarn.lock @@ -401,10 +401,10 @@ resolved "https://registry.npmjs.org/@colors/colors/-/colors-1.5.0.tgz" integrity "sha1-u1BFecHK6SPmV2pPXaQ9Jfl729k= sha512-ooWCrlZP11i8GImSjTHYHLkvFDP48nS4+204nGb1RiX/WXYHmJA2III9/e2DWVabCESdW7hBAEzHRqUn9OUVvQ==" -"@datadog/libdatadog@^0.3.0": - version "0.3.0" - resolved "https://registry.yarnpkg.com/@datadog/libdatadog/-/libdatadog-0.3.0.tgz#2fc1e2695872840bc8c356f66acf675da428d6f0" - integrity sha512-TbP8+WyXfh285T17FnLeLUOPl4SbkRYMqKgcmknID2mXHNrbt5XJgW9bnDgsrrtu31Q7FjWWw2WolgRLWyzLRA== +"@datadog/libdatadog@^0.4.0": + version "0.4.0" + resolved "https://registry.yarnpkg.com/@datadog/libdatadog/-/libdatadog-0.4.0.tgz#aeeea02973f663b555ad9ac30c4015a31d561598" + integrity sha512-kGZfFVmQInzt6J4FFGrqMbrDvOxqwk3WqhAreS6n9b/De+iMVy/NMu3V7uKsY5zAvz+uQw0liDJm3ZDVH/MVVw== "@datadog/native-appsec@8.4.0": version "8.4.0"