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

[test-optimization] [SDTEST-1163] Playwright active test span #4843

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Prev Previous commit
Next Next commit
observability
juan-fernandez committed Mar 21, 2025
commit 5737d32b77243344a1bacc38e1ec5e38f4d9a526
953 changes: 474 additions & 479 deletions integration-tests/playwright/playwright.spec.js

Large diffs are not rendered by default.

63 changes: 57 additions & 6 deletions packages/datadog-instrumentations/src/playwright.js
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ const testSuiteFinishCh = channel('ci:playwright:test-suite:finish')

const workerReportCh = channel('ci:playwright:worker:report')

const testToAr = new WeakMap()
const testSuiteToAr = new Map()
const testSuiteToTestStatuses = new Map()
const testSuiteToErrors = new Map()
@@ -251,9 +252,12 @@ function getTestFullname (test) {
return names.join(' ')
}

function testBeginHandler (test) {
function testBeginHandler (test, browserName, isMainProcess) {
const {
_requireFile: testSuiteAbsolutePath,
location: {
line: testSourceLine
},
_type
} = test

@@ -271,10 +275,31 @@ function testBeginHandler (test) {
testSuiteStartCh.publish(testSuiteAbsolutePath)
})
}

// this handles tests that do not go through the worker process (because they're skipped)
if (isMainProcess) {
const testAsyncResource = new AsyncResource('bound-anonymous-fn')
testToAr.set(test, testAsyncResource)
const testName = getTestFullname(test)

testAsyncResource.runInAsyncScope(() => {
testStartCh.publish({
testName,
testSuiteAbsolutePath,
testSourceLine,
browserName
})
})
}
}

function testEndHandler (test, annotations, testStatus, error, isTimeout) {
const { _requireFile: testSuiteAbsolutePath, _type } = test
function testEndHandler (test, annotations, testStatus, error, isTimeout, isMainProcess) {
const { _requireFile: testSuiteAbsolutePath, results, _type } = test

let annotationTags
if (annotations.length) {
annotationTags = parseAnnotations(annotations)
}

if (_type === 'beforeAll' || _type === 'afterAll') {
const hookError = formatTestHookError(error, _type, isTimeout)
@@ -285,6 +310,23 @@ function testEndHandler (test, annotations, testStatus, error, isTimeout) {
return
}

if (isMainProcess) {
const testResult = results[results.length - 1]
const testAsyncResource = testToAr.get(test)
testAsyncResource.runInAsyncScope(() => {
testFinishCh.publish({
testStatus,
steps: testResult?.steps || [],
isRetry: testResult?.retry > 0,
error,
extraTags: annotationTags,
isNew: test._ddIsNew,
isQuarantined: test._ddIsQuarantined,
isEfdRetry: test._ddIsEfdRetry
})
})
}

if (testSuiteToTestStatuses.has(testSuiteAbsolutePath)) {
testSuiteToTestStatuses.get(testSuiteAbsolutePath).push(testStatus)
} else {
@@ -466,8 +508,8 @@ function runnerHook (runnerExport, playwrightVersion) {
// because they were skipped
tests.forEach(test => {
const browser = getBrowserNameFromProjects(projects, test)
testBeginHandler(test, browser)
testEndHandler(test, [], 'skip')
testBeginHandler(test, browser, true)
testEndHandler(test, [], 'skip', null, false, true)
})
})

@@ -687,6 +729,12 @@ addHook({
annotationTags = parseAnnotations(annotations)
}

let onDone

const flushPromise = new Promise(resolve => {
onDone = resolve
})

testAsyncResource.runInAsyncScope(() => {
testFinishCh.publish({
testStatus: STATUS_TO_TEST_STATUS[status],
@@ -697,10 +745,13 @@ addHook({
// probably not going to work
isNew: test._ddIsNew,
isQuarantined: test._ddIsQuarantined,
isEfdRetry: test._ddIsEfdRetry
isEfdRetry: test._ddIsEfdRetry,
onDone
})
})

await flushPromise

return res
})

11 changes: 6 additions & 5 deletions packages/datadog-plugin-playwright/src/index.js
Original file line number Diff line number Diff line change
@@ -184,6 +184,8 @@ class PlaywrightPlugin extends CiPlugin {
parent_id: id(span.parent_id)
}
if (span.name === 'playwright.test') {
// TODO: Let's pass rootDir, repositoryRoot, command, session id and module id as env vars
// so we don't need this re-serialization logic
formattedSpan.meta[TEST_SESSION_ID] = this.testSessionSpan.context().toTraceId()
formattedSpan.meta[TEST_MODULE_ID] = this.testModuleSpan.context().toSpanId()
formattedSpan.meta[TEST_COMMAND] = this.command
@@ -219,7 +221,8 @@ class PlaywrightPlugin extends CiPlugin {
isNew,
isEfdRetry,
isRetry,
isQuarantined
isQuarantined,
onDone
}) => {
const store = storage('legacy').getStore()
const span = store && store.span
@@ -246,7 +249,6 @@ class PlaywrightPlugin extends CiPlugin {
if (isQuarantined) {
span.setTag(TEST_MANAGEMENT_IS_QUARANTINED, 'true')
}

steps.forEach(step => {
const stepStartTime = step.startTime.getTime()
const stepSpan = this.tracer.startSpan('playwright.step', {
@@ -267,7 +269,6 @@ class PlaywrightPlugin extends CiPlugin {
}
stepSpan.finish(stepStartTime + stepDuration)
})

if (testStatus === 'fail') {
this.numFailedTests++
}
@@ -285,12 +286,12 @@ class PlaywrightPlugin extends CiPlugin {

finishAllTraceSpans(span)
if (process.env.DD_PLAYWRIGHT_WORKER) {
this.tracer._exporter.flush()
this.tracer._exporter.flush(onDone)
}
})
}

// TODO: this can be simplified now that it only runs in worker
// TODO: this runs both in worker and main process (main process: skipped tests that do not go through _runTest)
startTestSpan (testName, testSuiteAbsolutePath, testSuite, testSourceFile, testSourceLine, browserName) {
const testSuiteSpan = this._testSuites.get(testSuiteAbsolutePath)

Original file line number Diff line number Diff line change
@@ -69,8 +69,9 @@ class TestWorkerCiVisibilityExporter {
this._logsWriter.append({ testConfiguration, logMessage })
}

flush () {
this._writer.flush()
// TODO: add to other writers
flush (onDone) {
this._writer.flush(onDone)
this._coverageWriter.flush()
this._logsWriter.flush()
}
Original file line number Diff line number Diff line change
@@ -8,21 +8,21 @@ class Writer {
this._interprocessCode = interprocessCode
}

flush () {
flush (onDone) {
const count = this._encoder.count()

if (count > 0) {
const payload = this._encoder.makePayload()

this._sendPayload(payload)
this._sendPayload(payload, onDone)
}
}

append (payload) {
this._encoder.encode(payload)
}

_sendPayload (data) {
_sendPayload (data, onDone = () => {}) {
// ## Jest
// Only available when `child_process` is used for the jest worker.
// eslint-disable-next-line
@@ -35,8 +35,11 @@ class Writer {
// See cucumber code:
// eslint-disable-next-line
// https://github.com/cucumber/cucumber-js/blob/5ce371870b677fe3d1a14915dc535688946f734c/src/runtime/parallel/run_worker.ts#L13
// console.log('sending payload', !!process.send)
if (process.send) { // it only works if process.send is available
process.send([this._interprocessCode, data])
process.send([this._interprocessCode, data], () => {
onDone()
})
}
}
}