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.

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

[ci-visibility] Work around jest's --forceExit #4049

Merged
merged 4 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
32 changes: 26 additions & 6 deletions integration-tests/ci-visibility-intake.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ let gitUploadStatus = DEFAULT_GIT_UPLOAD_STATUS
let infoResponse = DEFAULT_INFO_RESPONSE
let correlationId = DEFAULT_CORRELATION_ID
let knownTests = DEFAULT_KNOWN_TESTS
let waitingTime = null

class FakeCiVisIntake extends FakeAgent {
setKnownTests (newKnownTestsResponse) {
Expand All @@ -61,6 +62,10 @@ class FakeCiVisIntake extends FakeAgent {
settings = newSettings
}

setWaitingTime (newWaitingTime) {
waitingTime = newWaitingTime
}

async start () {
const app = express()
app.use(bodyParser.raw({ limit: Infinity, type: 'application/msgpack' }))
Expand All @@ -83,13 +88,25 @@ class FakeCiVisIntake extends FakeAgent {
})
})

// It can be slowed down with setWaitingTime
app.post(['/api/v2/citestcycle', '/evp_proxy/:version/api/v2/citestcycle'], (req, res) => {
res.status(200).send('OK')
this.emit('message', {
headers: req.headers,
payload: msgpack.decode(req.body, { codec }),
url: req.url
})
if (waitingTime) {
this.waitingTimeoutId = setTimeout(() => {
res.status(200).send('OK')
this.emit('message', {
headers: req.headers,
payload: msgpack.decode(req.body, { codec }),
url: req.url
})
}, waitingTime)
} else {
res.status(200).send('OK')
this.emit('message', {
headers: req.headers,
payload: msgpack.decode(req.body, { codec }),
url: req.url
})
juan-fernandez marked this conversation as resolved.
Show resolved Hide resolved
}
})

app.post([
Expand Down Expand Up @@ -214,6 +231,9 @@ class FakeCiVisIntake extends FakeAgent {
gitUploadStatus = DEFAULT_GIT_UPLOAD_STATUS
infoResponse = DEFAULT_INFO_RESPONSE
this.removeAllListeners()
if (this.waitingTimeoutId) {
clearTimeout(this.waitingTimeoutId)
}
return super.stop()
}

Expand Down
78 changes: 78 additions & 0 deletions integration-tests/ci-visibility.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,84 @@ testFrameworks.forEach(({
})
})
})
it('works with --forceExit and logs a warning', (done) => {
const eventsPromise = receiver
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/citestcycle'), (payloads) => {
assert.include(testOutput, "Jest's '--forceExit' flag has been passed")
const events = payloads.flatMap(({ payload }) => payload.events)

const testSession = events.find(event => event.type === 'test_session_end')
const testModule = events.find(event => event.type === 'test_module_end')
const testSuites = events.filter(event => event.type === 'test_suite_end')
const tests = events.filter(event => event.type === 'test')

assert.exists(testSession)
assert.exists(testModule)
assert.equal(testSuites.length, 2)
assert.equal(tests.length, 2)
})
// Needs to run with the CLI if we want --forceExit to work
childProcess = exec(
'node ./node_modules/jest/bin/jest --config config-jest.js --forceExit',
{
cwd,
env: {
...getCiVisAgentlessConfig(receiver.port),
DD_TRACE_DEBUG: '1',
DD_TRACE_LOG_LEVEL: 'warn'
},
stdio: 'inherit'
}
)
childProcess.on('exit', () => {
eventsPromise.then(() => {
done()
}).catch(done)
})
childProcess.stdout.on('data', (chunk) => {
testOutput += chunk.toString()
})
childProcess.stderr.on('data', (chunk) => {
testOutput += chunk.toString()
})
})
it('does not hang if server is not available and logs an error', (done) => {
// Very slow intake
receiver.setWaitingTime(30000)
const eventsPromise = receiver
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/citestcycle'), (payloads) => {
assert.include(testOutput, "Jest's '--forceExit' flag has been passed")
assert.include(testOutput, 'Timeout waiting for the tracer to flush')
const events = payloads.flatMap(({ payload }) => payload.events)

assert.equal(events.length, 0)
}, 12000)
// Needs to run with the CLI if we want --forceExit to work
childProcess = exec(
'node ./node_modules/jest/bin/jest --config config-jest.js --forceExit',
{
cwd,
env: {
...getCiVisAgentlessConfig(receiver.port),
DD_TRACE_DEBUG: '1',
DD_TRACE_LOG_LEVEL: 'warn'
},
stdio: 'inherit'
}
)
childProcess.on('exit', () => {
eventsPromise.then(() => {
receiver.setWaitingTime(0)
done()
}).catch(done)
})
childProcess.stdout.on('data', (chunk) => {
testOutput += chunk.toString()
})
childProcess.stderr.on('data', (chunk) => {
testOutput += chunk.toString()
})
})
}

it('can run tests and report spans', (done) => {
Expand Down
8 changes: 8 additions & 0 deletions integration-tests/config-jest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports = {
projects: [__dirname],
testPathIgnorePatterns: ['/node_modules/'],
cache: false,
testMatch: [
'**/ci-visibility/test/ci-visibility-test*'
]
}
6 changes: 4 additions & 2 deletions integration-tests/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ function getCiVisAgentlessConfig (port) {
DD_API_KEY: '1',
DD_CIVISIBILITY_AGENTLESS_ENABLED: 1,
DD_CIVISIBILITY_AGENTLESS_URL: `http://127.0.0.1:${port}`,
NODE_OPTIONS: '-r dd-trace/ci/init'
NODE_OPTIONS: '-r dd-trace/ci/init',
DD_INSTRUMENTATION_TELEMETRY_ENABLED: 'false'
}
}

Expand All @@ -291,7 +292,8 @@ function getCiVisEvpProxyConfig (port) {
...rest,
DD_TRACE_AGENT_PORT: port,
NODE_OPTIONS: '-r dd-trace/ci/init',
DD_CIVISIBILITY_AGENTLESS_ENABLED: '0'
DD_CIVISIBILITY_AGENTLESS_ENABLED: '0',
DD_INSTRUMENTATION_TELEMETRY_ENABLED: 'false'
}
}

Expand Down
29 changes: 28 additions & 1 deletion packages/datadog-instrumentations/src/jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ const knownTestsCh = channel('ci:jest:known-tests')

const itrSkippedSuitesCh = channel('ci:jest:itr:skipped-suites')

// Maximum time we'll wait for the tracer to flush
const FLUSH_TIMEOUT = 10000

let skippableSuites = []
let knownTests = []
let isCodeCoverageEnabled = false
Expand Down Expand Up @@ -415,6 +418,20 @@ function cliWrapper (cli, jestVersion) {
status = 'fail'
error = new Error(`Failed test suites: ${numFailedTestSuites}. Failed tests: ${numFailedTests}`)
}
let timeoutId

const flushPromise = new Promise((resolve) => {
juan-fernandez marked this conversation as resolved.
Show resolved Hide resolved
onDone = () => {
clearTimeout(timeoutId)
resolve()
}
})

const timeoutPromise = new Promise((resolve) => {
timeoutId = setTimeout(() => {
resolve('timeout')
}, FLUSH_TIMEOUT).unref()
})

sessionAsyncResource.runInAsyncScope(() => {
testSessionFinishCh.publish({
Expand All @@ -427,9 +444,15 @@ function cliWrapper (cli, jestVersion) {
hasUnskippableSuites,
hasForcedToRunSuites,
error,
isEarlyFlakeDetectionEnabled
isEarlyFlakeDetectionEnabled,
onDone
})
})
const waitingResult = await Promise.race([flushPromise, timeoutPromise])

if (waitingResult === 'timeout') {
log.error('Timeout waiting for the tracer to flush')
}

numSkippedSuites = 0

Expand Down Expand Up @@ -548,6 +571,10 @@ function configureTestEnvironment (readConfigsResult) {

isUserCodeCoverageEnabled = !!readConfigsResult.globalConfig.collectCoverage

if (readConfigsResult.globalConfig.forceExit) {
log.warn("Jest's '--forceExit' flag has been passed. This may cause loss of data.")
}

if (isCodeCoverageEnabled) {
const globalConfig = {
...readConfigsResult.globalConfig,
Expand Down
10 changes: 8 additions & 2 deletions packages/datadog-plugin-jest/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ class JestPlugin extends CiPlugin {
hasUnskippableSuites,
hasForcedToRunSuites,
error,
isEarlyFlakeDetectionEnabled
isEarlyFlakeDetectionEnabled,
onDone
}) => {
this.testSessionSpan.setTag(TEST_STATUS, status)
this.testModuleSpan.setTag(TEST_STATUS, status)
Expand Down Expand Up @@ -121,7 +122,12 @@ class JestPlugin extends CiPlugin {
this.testSessionSpan.finish()
this.telemetry.ciVisEvent(TELEMETRY_EVENT_FINISHED, 'session')
finishAllTraceSpans(this.testSessionSpan)
this.tracer._exporter.flush()

this.tracer._exporter.flush(() => {
if (onDone) {
onDone()
}
})
})

// Test suites can be run in a different process from jest's main one.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const {
getErrorTypeFromStatusCode
} = require('../telemetry')

const DEFAULT_NUM_RETRIES_EARLY_FLAKE_DETECTION = 2
const DEFAULT_EARLY_FLAKE_DETECTION_NUM_RETRIES = 2

function getLibraryConfiguration ({
url,
Expand Down Expand Up @@ -104,7 +104,7 @@ function getLibraryConfiguration ({
requireGit,
isEarlyFlakeDetectionEnabled: earlyFlakeDetectionConfig?.enabled ?? false,
earlyFlakeDetectionNumRetries:
earlyFlakeDetectionConfig?.slow_test_retries?.['5s'] || DEFAULT_NUM_RETRIES_EARLY_FLAKE_DETECTION
earlyFlakeDetectionConfig?.slow_test_retries?.['5s'] || DEFAULT_EARLY_FLAKE_DETECTION_NUM_RETRIES
}

log.debug(() => `Remote settings: ${JSON.stringify(settings)}`)
Expand Down
Loading