Skip to content

Commit

Permalink
fix cucumber
Browse files Browse the repository at this point in the history
  • Loading branch information
@juan-fernandez
juan-fernandez committed Jan 9, 2025
1 parent e340f7a commit 0ff41ea
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 68 deletions.
56 changes: 30 additions & 26 deletions integration-tests/cucumber/cucumber.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ const {
TEST_SESSION_NAME,
TEST_LEVEL_EVENT_TYPES,
DI_ERROR_DEBUG_INFO_CAPTURED,
DI_DEBUG_ERROR_FILE,
DI_DEBUG_ERROR_SNAPSHOT_ID,
DI_DEBUG_ERROR_LINE
DI_DEBUG_ERROR_PREFIX,
DI_DEBUG_ERROR_FILE_SUFFIX,
DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX,
DI_DEBUG_ERROR_LINE_SUFFIX
} = require('../../packages/dd-trace/src/plugins/util/test')
const { DD_HOST_CPU_COUNT } = require('../../packages/dd-trace/src/plugins/util/env')

Expand Down Expand Up @@ -1559,10 +1560,12 @@ versions.forEach(version => {
assert.equal(retriedTests.length, 1)
const [retriedTest] = retriedTests

assert.notProperty(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED)
assert.notProperty(retriedTest.meta, DI_DEBUG_ERROR_FILE)
assert.notProperty(retriedTest.metrics, DI_DEBUG_ERROR_LINE)
assert.notProperty(retriedTest.meta, DI_DEBUG_ERROR_SNAPSHOT_ID)
const hasDebugTags = Object.keys(retriedTest.meta)
.some(property =>
property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED
)

assert.isFalse(hasDebugTags)
})
const logsPromise = receiver
.gatherPayloadsMaxTimeout(({ url }) => url === logsEndpoint, (payloads) => {
Expand Down Expand Up @@ -1602,11 +1605,12 @@ versions.forEach(version => {

assert.equal(retriedTests.length, 1)
const [retriedTest] = retriedTests
const hasDebugTags = Object.keys(retriedTest.meta)
.some(property =>
property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED
)

assert.notProperty(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED)
assert.notProperty(retriedTest.meta, DI_DEBUG_ERROR_FILE)
assert.notProperty(retriedTest.metrics, DI_DEBUG_ERROR_LINE)
assert.notProperty(retriedTest.meta, DI_DEBUG_ERROR_SNAPSHOT_ID)
assert.isFalse(hasDebugTags)
})
const logsPromise = receiver
.gatherPayloadsMaxTimeout(({ url }) => url === logsEndpoint, (payloads) => {
Expand Down Expand Up @@ -1655,15 +1659,17 @@ versions.forEach(version => {
const [retriedTest] = retriedTests

assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true')
assert.propertyVal(
retriedTest.meta,
DI_DEBUG_ERROR_FILE,
'ci-visibility/features-di/support/sum.js'

assert.isTrue(
retriedTest.meta[`${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_FILE_SUFFIX}`]
.endsWith('ci-visibility/features-di/support/sum.js')
)
assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4)
assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID])
assert.equal(retriedTest.metrics[`${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_LINE_SUFFIX}`], 4)

const snapshotIdKey = `${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX}`
assert.exists(retriedTest.meta[snapshotIdKey])

snapshotIdByTest = retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID]
snapshotIdByTest = retriedTest.meta[snapshotIdKey]
spanIdByTest = retriedTest.span_id.toString()
traceIdByTest = retriedTest.trace_id.toString()
})
Expand Down Expand Up @@ -1733,14 +1739,12 @@ versions.forEach(version => {
assert.equal(retriedTests.length, 1)
const [retriedTest] = retriedTests

assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true')
assert.propertyVal(
retriedTest.meta,
DI_DEBUG_ERROR_FILE,
'ci-visibility/features-di/support/sum.js'
)
assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4)
assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID])
const hasDebugTags = Object.keys(retriedTest.meta)
.some(property =>
property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED
)

assert.isFalse(hasDebugTags)
})
const logsPromise = receiver
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => {
Expand Down
15 changes: 12 additions & 3 deletions packages/datadog-instrumentations/src/cucumber.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,9 @@ function wrapRun (pl, isLatestVersion) {
asyncResource.runInAsyncScope(() => {
testStartCh.publish(testStartPayload)
})
const promises = {}
try {
this.eventBroadcaster.on('envelope', shimmer.wrapFunction(null, () => (testCase) => {
this.eventBroadcaster.on('envelope', shimmer.wrapFunction(null, () => async (testCase) => {
// Only supported from >=8.0.0
if (testCase?.testCaseFinished) {
const { testCaseFinished: { willBeRetried } } = testCase
Expand All @@ -254,6 +255,11 @@ function wrapRun (pl, isLatestVersion) {

const failedAttemptAsyncResource = numAttemptToAsyncResource.get(numAttempt)
const isRetry = numAttempt++ > 0

if (promises.hitBreakpointPromise) {
await promises.hitBreakpointPromise
}

failedAttemptAsyncResource.runInAsyncScope(() => {
// the current span will be finished and a new one will be created
testRetryCh.publish({ isRetry, error })
Expand All @@ -263,7 +269,7 @@ function wrapRun (pl, isLatestVersion) {
numAttemptToAsyncResource.set(numAttempt, newAsyncResource)

newAsyncResource.runInAsyncScope(() => {
testStartCh.publish(testStartPayload) // a new span will be created
testStartCh.publish({ ...testStartPayload, promises }) // a new span will be created
})
}
}
Expand All @@ -273,7 +279,7 @@ function wrapRun (pl, isLatestVersion) {
asyncResource.runInAsyncScope(() => {
promise = run.apply(this, arguments)
})
promise.finally(() => {
promise.finally(async () => {
const result = this.getWorstStepResult()
const { status, skipReason } = isLatestVersion
? getStatusFromResultLatest(result)
Expand All @@ -296,6 +302,9 @@ function wrapRun (pl, isLatestVersion) {

const error = getErrorFromCucumberResult(result)

if (promises.hitBreakpointPromise) {
await promises.hitBreakpointPromise
}
attemptAsyncResource.runInAsyncScope(() => {
testFinishCh.publish({ status, skipReason, error, isNew, isEfdRetry, isFlakyRetry: numAttempt > 0 })
})
Expand Down
52 changes: 23 additions & 29 deletions packages/datadog-plugin-cucumber/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,7 @@ const {
TEST_MODULE,
TEST_MODULE_ID,
TEST_SUITE,
CUCUMBER_IS_PARALLEL,
TEST_NAME,
DI_ERROR_DEBUG_INFO_CAPTURED,
DI_DEBUG_ERROR_SNAPSHOT_ID,
DI_DEBUG_ERROR_FILE,
DI_DEBUG_ERROR_LINE
CUCUMBER_IS_PARALLEL
} = require('../../dd-trace/src/plugins/util/test')
const { RESOURCE_NAME } = require('../../../ext/tags')
const { COMPONENT, ERROR_MESSAGE } = require('../../dd-trace/src/constants')
Expand All @@ -50,8 +45,8 @@ const {
} = require('../../dd-trace/src/ci-visibility/telemetry')
const id = require('../../dd-trace/src/id')

const BREAKPOINT_HIT_GRACE_PERIOD_MS = 200
const isCucumberWorker = !!process.env.CUCUMBER_WORKER_ID
const debuggerParameterPerTest = new Map()

function getTestSuiteTags (testSuiteSpan) {
const suiteTags = {
Expand Down Expand Up @@ -210,7 +205,13 @@ class CucumberPlugin extends CiPlugin {
this.telemetry.ciVisEvent(TELEMETRY_CODE_COVERAGE_FINISHED, 'suite', { library: 'istanbul' })
})

this.addSub('ci:cucumber:test:start', ({ testName, testFileAbsolutePath, testSourceLine, isParallel }) => {
this.addSub('ci:cucumber:test:start', ({
testName,
testFileAbsolutePath,
testSourceLine,
isParallel,
promises
}) => {
const store = storage.getStore()
const testSuite = getTestSuitePath(testFileAbsolutePath, this.sourceRoot)
const testSourceFile = getTestSuitePath(testFileAbsolutePath, this.repositoryRoot)
Expand All @@ -227,24 +228,12 @@ class CucumberPlugin extends CiPlugin {

this.enter(testSpan, store)

const debuggerParameters = debuggerParameterPerTest.get(testName)

if (debuggerParameters) {
const spanContext = testSpan.context()

// TODO: handle race conditions with this.retriedTestIds
this.retriedTestIds = {
spanId: spanContext.toSpanId(),
traceId: spanContext.toTraceId()
}
const { snapshotId, file, line } = debuggerParameters

// TODO: should these be added on test:end if and only if the probe is hit?
// Sync issues: `hitProbePromise` might be resolved after the test ends
testSpan.setTag(DI_ERROR_DEBUG_INFO_CAPTURED, 'true')
testSpan.setTag(DI_DEBUG_ERROR_SNAPSHOT_ID, snapshotId)
testSpan.setTag(DI_DEBUG_ERROR_FILE, file)
testSpan.setTag(DI_DEBUG_ERROR_LINE, line)
this.activeTestSpan = testSpan
// Time we give the breakpoint to be hit
if (promises && this.runningTestProbeId) {
promises.hitBreakpointPromise = new Promise((resolve) => {
setTimeout(resolve, BREAKPOINT_HIT_GRACE_PERIOD_MS)
})
}
})

Expand All @@ -256,9 +245,13 @@ class CucumberPlugin extends CiPlugin {
}
span.setTag('error', error)
if (this.di && error && this.libraryConfig?.isDiEnabled) {
const testName = span.context()._tags[TEST_NAME]
const debuggerParameters = this.addDiProbe(error)
debuggerParameterPerTest.set(testName, debuggerParameters)
const probeInformation = this.addDiProbe(error)
if (probeInformation) {
const { probeId, stackIndex } = probeInformation
this.runningTestProbeId = probeId
this.testErrorStackIndex = stackIndex
// TODO: we're not waiting for setProbePromise to be resolved, so there might be race conditions
}
}
span.setTag(TEST_STATUS, 'fail')
span.finish()
Expand Down Expand Up @@ -363,6 +356,7 @@ class CucumberPlugin extends CiPlugin {
if (isCucumberWorker) {
this.tracer._exporter.flush()
}
this.activeTestSpan = null
}
})

Expand Down
2 changes: 1 addition & 1 deletion packages/datadog-plugin-jest/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ class JestPlugin extends CiPlugin {
span.setTag(TEST_STATUS, 'fail')
span.setTag('error', getFormattedError(error, this.repositoryRoot))
if (shouldSetProbe) {
const probeInformation = this.addDiProbe(error, this.onDiBreakpointHit.bind(this))
const probeInformation = this.addDiProbe(error)
if (probeInformation) {
const { probeId, setProbePromise, stackIndex } = probeInformation
this.runningTestProbeId = probeId
Expand Down
9 changes: 2 additions & 7 deletions packages/datadog-plugin-mocha/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@ const {
TEST_SUITE,
MOCHA_IS_PARALLEL,
TEST_IS_RUM_ACTIVE,
TEST_BROWSER_DRIVER,
TEST_NAME,
DI_ERROR_DEBUG_INFO_CAPTURED,
DI_DEBUG_ERROR_SNAPSHOT_ID,
DI_DEBUG_ERROR_FILE,
DI_DEBUG_ERROR_LINE
TEST_BROWSER_DRIVER
} = require('../../dd-trace/src/plugins/util/test')
const { COMPONENT } = require('../../dd-trace/src/constants')
const {
Expand Down Expand Up @@ -277,7 +272,7 @@ class MochaPlugin extends CiPlugin {
}
)
if (willBeRetried && this.di && this.libraryConfig?.isDiEnabled) {
const probeInformation = this.addDiProbe(err, this.onDiBreakpointHit.bind(this))
const probeInformation = this.addDiProbe(err)
if (probeInformation) {
const { probeId, stackIndex } = probeInformation
this.runningTestProbeId = probeId
Expand Down
4 changes: 2 additions & 2 deletions packages/dd-trace/src/plugins/ci_plugin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,15 @@ module.exports = class CiPlugin extends Plugin {
return this.di.removeProbe(probeId)
}

addDiProbe (err, onHitBreakpoint) {
addDiProbe (err) {
const [file, line, stackIndex] = getFileAndLineNumberFromError(err, this.repositoryRoot)

if (!file || !Number.isInteger(line)) {
log.warn('Could not add breakpoint for dynamic instrumentation')
return
}

const [probeId, setProbePromise] = this.di.addLineProbe({ file, line }, onHitBreakpoint)
const [probeId, setProbePromise] = this.di.addLineProbe({ file, line }, this.onDiBreakpointHit.bind(this))

return {
probeId,
Expand Down

0 comments on commit 0ff41ea

Please sign in to comment.