-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix: xml output generation for parallel tests #2734
fix: xml output generation for parallel tests #2734
Conversation
lib/reporter/global-reporter.js
Outdated
@@ -51,7 +51,7 @@ module.exports = class GlobalReporter { | |||
setupChildProcessListener(emitter) { | |||
const Concurrency = require('../runner/concurrency/concurrency.js'); | |||
|
|||
if (Concurrency.isMasterProcess()) { | |||
if (Concurrency.isMasterProcess() || Concurrency.isChildProcess()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we remove the check altogether?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should, but I wasn't sure, why the check exists in the first place. Let me update the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check is for the scenario when running in parallel with test workers. A child process will send the results back to the main process. When running tests across multiple browsers, the results aren't called by the main process anymore so it doesn't work (since the bug), so we need to make sure that the test workers scenario still works well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can probably just change the check to !Concurrency.isTestWorker()
Need to add test here |
Fixes #2728
Overview
XML output files are not generated when running parallel tests on multiple browsers, because when we run concurrent tests, we only check the master process to push the results to the 'GlobalReporter', but in fact, the tests are spawn through the child process.