Skip to content

Commit 9f7aa87

Browse files
fix(junit): preserve suite identity for hook failures reported from workers (#5664)
* fix(junit): preserve suite identity for hook failures reported from workers Under run-workers, event.hook.failed for a BeforeSuite/AfterSuite failure arrives in the main process via Hook.simplify(), which only carries {hookName, title, error} — no ctx, so the reporter has no way to recover the owning suite. groupBySuite() then falls back to key = the failure entry itself, and buildXml's suiteName defaults to "Tests", splitting each hook failure into its own <testsuite name="Tests"> instead of the suite that actually failed. - Hook.simplify() now also serializes suiteTitle/suiteFile/suiteTags from this.runnable.parent (the real Suite, already used successfully on the main-thread path — see the "adds suite hook failures as failed testcases" unit test). - junitReporter's event.hook.failed listener falls back to these fields when hook.ctx is absent, reusing one synthetic suite object per title+file pair so multiple failures from the same worker-reported suite still group into a single <testsuite>. Reproduced with the exact scenario from #5645 (BeforeSuite + AfterSuite both throwing) run via `codecept run-workers 2`: before this change both failures land under two separate <testsuite name="Tests"> entries; after, both are under one <testsuite name="My">, matching the main-thread output shape. * test(junit): add run-workers regression test for suite hook attribution Adds an end-to-end regression test that spawns a real run-workers process against a dedicated fixture (BeforeSuite/AfterSuite both throwing) with junitReporter enabled, then parses the resulting report.xml to assert both failures land under the real suite name in a single <testsuite> element instead of two <testsuite name="Tests"> elements. Verified this test fails cleanly (not just a timeout) against the pre-fix junitReporter.js/hooks.js and passes against the fix. The new fixture lives in its own workers-junit-suite-hooks/ directory rather than the shared workers/ directory, since several other tests in this file glob workers/*.js against the base config and would otherwise pick up the new failing suite and see different pass/fail counts. --------- Co-authored-by: kapilvus <kapilvus@gmail.com>
1 parent 97a4515 commit 9f7aa87

5 files changed

Lines changed: 89 additions & 1 deletion

File tree

‎lib/mocha/hooks.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,22 @@ class Hook {
3333
}
3434

3535
simplify() {
36+
// this.runnable (context.ctx.test) is the hook's own runnable; its
37+
// .parent is the real Mocha Suite that owns this hook. Included here so
38+
// run-workers can forward suite identity to the main process, where
39+
// listeners (e.g. junitReporter) have no other way to recover it — the
40+
// worker's live Suite/ctx objects aren't serializable across the thread
41+
// boundary, only these plain fields are.
42+
const suite = this.runnable?.parent
3643
return {
3744
hookName: this.hookName,
3845
title: this.title,
3946
// test: this.test ? serializeTest(this.test) : null,
4047
// suite: this.suite ? serializeSuite(this.suite) : null,
4148
error: this.err ? serializeError(this.err) : null,
49+
suiteTitle: suite?.title || null,
50+
suiteFile: suite?.file || null,
51+
suiteTags: suite?.tags || [],
4252
}
4353
}
4454

‎lib/plugin/junitReporter.js‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,29 @@ export default function (config = {}) {
6767

6868
let written = false
6969
const hookFailures = []
70+
// groupBySuite() (below) groups by object identity, not by value — reused
71+
// across BeforeSuite/AfterSuite failures from the same worker-forwarded
72+
// suite so they land in one <testsuite>, not one per failure.
73+
const workerSuiteByKey = new Map()
7074

7175
event.dispatcher.on(event.hook.failed, hook => {
7276
if (!hook || !['BeforeSuite', 'AfterSuite'].includes(hook.hookName)) return
7377
const err = hook.err || hook.error
7478
if (!err) return
7579
const runnable = hook.ctx && hook.ctx.test
76-
const suite = runnable && runnable.parent
80+
// Under run-workers, hook.ctx is absent — the failure arrives as the
81+
// plain object from Hook.simplify() in the main process instead of a
82+
// live Hook instance. Fall back to the suiteTitle/suiteFile/suiteTags
83+
// simplify() carries across the worker boundary so the failure still
84+
// groups under its real suite instead of the "Tests" fallback.
85+
let suite = runnable && runnable.parent
86+
if (!suite && hook.suiteTitle) {
87+
const key = `${hook.suiteTitle} ${hook.suiteFile || ''}`
88+
if (!workerSuiteByKey.has(key)) {
89+
workerSuiteByKey.set(key, { title: hook.suiteTitle, file: hook.suiteFile, tags: hook.suiteTags })
90+
}
91+
suite = workerSuiteByKey.get(key)
92+
}
7793
hookFailures.push({
7894
title: hook.title || `${hook.hookName} hook failed`,
7995
state: 'failed',
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
export const config = {
2+
tests: './workers-junit-suite-hooks/*.js',
3+
timeout: 10000,
4+
output: './output',
5+
helpers: {
6+
FileSystem: {},
7+
Workers: {
8+
require: './workers_helper',
9+
},
10+
},
11+
include: {},
12+
async bootstrap() {},
13+
mocha: {},
14+
plugins: {
15+
junitReporter: {
16+
enabled: true,
17+
},
18+
},
19+
name: 'sandbox',
20+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
Feature('JunitWorkerSuiteHooks')
2+
3+
BeforeSuite(async () => {
4+
throw new Error('BeforeSuite worker failure')
5+
})
6+
7+
Scenario('should not be executed either', ({ I }) => {
8+
I.say('unreachable')
9+
})
10+
11+
AfterSuite(async () => {
12+
throw new Error('AfterSuite worker failure')
13+
})

‎test/runner/run_workers_test.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import fs from 'fs'
66
import semver from 'semver'
77
import { exec } from 'child_process'
88
import { fileURLToPath } from 'url'
9+
import xml2js from 'xml2js'
910
const __filename = fileURLToPath(import.meta.url)
1011
const __dirname = path.dirname(__filename)
1112

@@ -512,6 +513,34 @@ describe('CodeceptJS Workers Runner', function () {
512513
})
513514
})
514515

516+
it('should preserve suite identity for BeforeSuite/AfterSuite hook failures in the JUnit report', function (done) {
517+
if (!semver.satisfies(process.version, '>=11.7.0')) this.skip('not for node version')
518+
const reportFile = path.join(codecept_dir, 'output', 'report.xml')
519+
if (fs.existsSync(reportFile)) fs.rmSync(reportFile)
520+
521+
exec(`${codecept_run_glob('codecept.workers-junit.conf.js')} 1`, (err, stdout) => {
522+
;(async () => {
523+
expect(stdout).toContain('BeforeSuite worker failure')
524+
expect(stdout).toContain('AfterSuite worker failure')
525+
expect(err.code).toEqual(1)
526+
527+
expect(fs.existsSync(reportFile)).toEqual(true)
528+
const parsed = await new xml2js.Parser().parseStringPromise(fs.readFileSync(reportFile, 'utf8'))
529+
530+
// Both hook failures must land under the real suite name, grouped into a
531+
// single <testsuite>, not split across two <testsuite name="Tests"> elements.
532+
expect(parsed.testsuites.testsuite).toHaveLength(1)
533+
const suiteEl = parsed.testsuites.testsuite[0]
534+
expect(suiteEl.$.name).toEqual('JunitWorkerSuiteHooks')
535+
expect(suiteEl.testcase).toHaveLength(2)
536+
537+
const names = suiteEl.testcase.map(tc => tc.$.name)
538+
expect(names.some(n => n.includes('BeforeSuite'))).toEqual(true)
539+
expect(names.some(n => n.includes('AfterSuite'))).toEqual(true)
540+
})().then(done, done)
541+
})
542+
})
543+
515544
it('should handle large worker count without inflating statistics', function (done) {
516545
if (!semver.satisfies(process.version, '>=11.7.0')) this.skip('not for node version')
517546
// Test with more workers than tests to ensure no inflation

0 commit comments

Comments
 (0)