Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[test optimization] Change failed test replay to opt out rather than opt in #5283

Merged
merged 5 commits into from
Feb 18, 2025
Merged

[test optimization] Change failed test replay to opt out rather than opt in #5283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2019818
change default value
juan-fernandez Feb 17, 2025
d7c1f4c
update tests
juan-fernandez Feb 17, 2025
03a028a
add config back
juan-fernandez Feb 17, 2025
dbb99bf
fix config
juan-fernandez Feb 17, 2025
86e5c78
Merge branch 'master' into juan-fernandez/flip-default-value-failed-t…
juan-fernandez Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions integration-tests/cucumber/cucumber.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'
}
)
Expand Down Expand Up @@ -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'
}
)
Expand Down Expand Up @@ -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'
}
)
Expand Down Expand Up @@ -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'
}
)
Expand Down
12 changes: 4 additions & 8 deletions integration-tests/jest/jest.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'
}
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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'
},
Expand Down
8 changes: 3 additions & 5 deletions integration-tests/mocha/mocha.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'
}
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down
14 changes: 6 additions & 8 deletions integration-tests/vitest/vitest.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'
}
Expand Down Expand Up @@ -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'
}
Expand Down Expand Up @@ -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'
}
Expand Down Expand Up @@ -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'
}
Expand Down
2 changes: 1 addition & 1 deletion packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
4 changes: 2 additions & 2 deletions packages/dd-trace/src/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions packages/dd-trace/src/plugins/ci_plugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
15 changes: 8 additions & 7 deletions packages/dd-trace/test/config.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
})
Expand Down Expand Up @@ -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', () => {
Expand Down
Loading