fix(junitReporter): stamp suites with their real start time - #5683
Open
luantaraschi wants to merge 1 commit into
Open
fix(junitReporter): stamp suites with their real start time#5683luantaraschi wants to merge 1 commit into
luantaraschi wants to merge 1 commit into
Conversation
junitReporter writes each `<testsuite timestamp>` from `suite.startedAt`,
but nothing ever set that field. Mocha's Suite does not carry it, and
`startedAt` was only assigned to individual tests, in
lib/listener/steps.js. So `toIso()` always fell through to its
`new Date()` fallback and every suite was stamped with the moment the XML
was serialized, which is after the suite (and its AfterSuite) finished.
The steps listener now stamps the suite on `event.suite.before`, mirroring
what it already does for tests.
Note this covers the in-process run. Under `run-workers` the parent only
receives `{ title }` for a test's parent suite, so the timestamp cannot
survive that boundary without changing the worker payload.
Closes codeceptjs#5668
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation/Description of the PR
Resolves #5668.
junitReporterwrites each<testsuite timestamp>fromsuite.startedAt(lib/plugin/junitReporter.js:135), but nothing ever set that field. Confirming @mirao's read:startedAtis assigned in exactly one place,lib/listener/steps.js:20, and only to individual tests. Mocha'sSuitedoes not carry it. SotoIso()always fell through to itsnew Date()fallback and every suite got stamped with the moment the XML was serialized, which is after that suite and itsAfterSuitehad already finished.The steps listener now stamps the suite on
event.suite.before, mirroring what it already does for tests two lines below.One thing worth flagging: the existing
junitReporter_test.jsbuilds its fixtures withstartedAt: Date.now()on each suite object, so the tests were exercising a field that production never produced. That is why the reporter's own suite passed while the output was wrong. I left those fixtures alone since they now describe reality, and put the new coverage on the listener instead.Scope note: this covers the in-process run. Under
run-workers,lib/mocha/test.jsserializes a test's parent down to{ title }and dropsstartedAtfrom the test payload as well, so a suite timestamp cannot cross that boundary without changing the worker payload. That looked like a separate change, so I left it out rather than widen this PR.Type of change
Checklist:
npm run docs) — N/A, no public API changenpm run lint)npm test)Two tests in
test/unit/listener/steps_suite_started_at_test.js. They fail on the commit before this change withexpected undefined to be a number, and pass with it.Full unit suite on Windows: 758 passing / 13 failing before, 760 passing / 11 failing after. The 11 remaining failures are pre-existing path assertions that expect POSIX-style paths and see a
C:drive letter (utils_test.js,utils/trace_test.js); they are identical with and without this change.