Skip to content

Commit 77f33f2

Browse files
pass browser name correctly
1 parent 1dca7e0 commit 77f33f2

File tree

2 files changed

+45
-20
lines changed

2 files changed

+45
-20
lines changed

integration-tests/playwright/playwright.spec.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2011,9 +2011,10 @@ versions.forEach((version) => {
20112011
context(`with fullyParallel=${parallelism}`, () => {
20122012
/**
20132013
* Due to a bug in the playwright plugin, durations of test suites that included skipped tests
2014-
* were not reported correctly, as they dragged until the end of the test session.
2014+
* were not reported correctly, as they dragged out until the end of the test session.
20152015
* This test checks that a long suite, which makes the test session longer,
20162016
* does not affect the duration of a short suite, which is expected to finish earlier.
2017+
* This only happened with tests that included skipped tests.
20172018
*/
20182019
it('should report the correct test suite duration when there are skipped tests', async () => {
20192020
const receiverPromise = receiver

packages/datadog-instrumentations/src/playwright.js

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,15 @@ function testBeginHandler (test, browserName, shouldCreateTestSpan) {
338338
}
339339
}
340340

341-
function testEndHandler (test, annotations, testStatus, error, isTimeout, shouldCreateTestSpan) {
341+
function testEndHandler ({
342+
test,
343+
annotations,
344+
testStatus,
345+
error,
346+
isTimeout,
347+
shouldCreateTestSpan,
348+
projects
349+
}) {
342350
const {
343351
_requireFile: testSuiteAbsolutePath,
344352
results,
@@ -431,18 +439,19 @@ function testEndHandler (test, annotations, testStatus, error, isTimeout, should
431439
const skippedTests = remainingTestsByFile[testSuiteAbsolutePath]
432440
.filter(test => test.expectedStatus === 'skipped')
433441

434-
skippedTests.forEach(test => {
442+
for (const test of skippedTests) {
443+
const browserName = getBrowserNameFromProjects(projects, test)
435444
testSkipCh.publish({
436445
testName: getTestFullname(test),
437446
testSuiteAbsolutePath,
438447
testSourceLine: test.location.line,
439-
browserName: 'chromium', // TODO: REMOVE HARDCODE
448+
browserName,
440449
isNew: test._ddIsNew,
441450
isDisabled: test._ddIsDisabled,
442451
isModified: test._ddIsModified,
443452
isQuarantined: test._ddIsQuarantined
444453
})
445-
})
454+
}
446455
remainingTestsByFile[testSuiteAbsolutePath] = []
447456

448457
const testStatuses = testSuiteToTestStatuses.get(testSuiteAbsolutePath)
@@ -483,10 +492,11 @@ function dispatcherHook (dispatcherExport) {
483492
shimmer.wrap(dispatcherExport.Dispatcher.prototype, '_createWorker', createWorker => function () {
484493
const dispatcher = this
485494
const worker = createWorker.apply(this, arguments)
495+
const projects = getProjectsFromDispatcher(dispatcher)
496+
486497
worker.process.on('message', ({ method, params }) => {
487498
if (method === 'testBegin') {
488499
const { test } = dispatcher._testById.get(params.testId)
489-
const projects = getProjectsFromDispatcher(dispatcher)
490500
const browser = getBrowserNameFromProjects(projects, test)
491501
const shouldCreateTestSpan = test.expectedStatus === 'skipped'
492502
testBeginHandler(test, browser, shouldCreateTestSpan)
@@ -499,12 +509,15 @@ function dispatcherHook (dispatcherExport) {
499509
const isTimeout = testResult.status === 'timedOut'
500510
const shouldCreateTestSpan = test.expectedStatus === 'skipped'
501511
testEndHandler(
502-
test,
503-
params.annotations,
504-
STATUS_TO_TEST_STATUS[testResult.status],
505-
testResult.error,
506-
isTimeout,
507-
shouldCreateTestSpan
512+
{
513+
test,
514+
annotations: params.annotations,
515+
testStatus: STATUS_TO_TEST_STATUS[testResult.status],
516+
error: testResult.error,
517+
isTimeout,
518+
shouldCreateTestSpan,
519+
projects
520+
}
508521
)
509522
}
510523
})
@@ -519,10 +532,10 @@ function dispatcherHookNew (dispatcherExport, runWrapper) {
519532
shimmer.wrap(dispatcherExport.Dispatcher.prototype, '_createWorker', createWorker => function () {
520533
const dispatcher = this
521534
const worker = createWorker.apply(this, arguments)
535+
const projects = getProjectsFromDispatcher(dispatcher)
522536

523537
worker.on('testBegin', ({ testId }) => {
524538
const test = getTestByTestId(dispatcher, testId)
525-
const projects = getProjectsFromDispatcher(dispatcher)
526539
const browser = getBrowserNameFromProjects(projects, test)
527540
const shouldCreateTestSpan = test.expectedStatus === 'skipped'
528541
testBeginHandler(test, browser, shouldCreateTestSpan)
@@ -533,12 +546,15 @@ function dispatcherHookNew (dispatcherExport, runWrapper) {
533546
const isTimeout = status === 'timedOut'
534547
const shouldCreateTestSpan = test.expectedStatus === 'skipped'
535548
testEndHandler(
536-
test,
537-
annotations,
538-
STATUS_TO_TEST_STATUS[status],
539-
errors && errors[0],
540-
isTimeout,
541-
shouldCreateTestSpan
549+
{
550+
test,
551+
annotations,
552+
testStatus: STATUS_TO_TEST_STATUS[status],
553+
error: errors && errors[0],
554+
isTimeout,
555+
shouldCreateTestSpan,
556+
projects
557+
}
542558
)
543559
const testResult = test.results.at(-1)
544560
const isAtrRetry = testResult?.retry > 0 &&
@@ -679,7 +695,15 @@ function runAllTestsWrapper (runAllTests, playwrightVersion) {
679695
tests.forEach(test => {
680696
const browser = getBrowserNameFromProjects(projects, test)
681697
testBeginHandler(test, browser, true)
682-
testEndHandler(test, [], 'skip', null, false, true)
698+
testEndHandler({
699+
test,
700+
annotations: [],
701+
testStatus: 'skip',
702+
error: null,
703+
isTimeout: false,
704+
shouldCreateTestSpan: true,
705+
projects
706+
})
683707
})
684708
})
685709

0 commit comments

Comments
 (0)