-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add output option to JSON reporter (#4131) #4607
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
Add output option to JSON reporter (#4131) #4607
Conversation
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.
@dorny thank you for this PR.
Yes, json.spec.js could benefit from some refactoring ...
I just have one remark, otherwise looks like a very diligent work.
Edit: the error is swallowed only for a failing test. For a passing test the error is printed to the console. This is a bug in our uncaught exception handler, which I will investigate later.
Edit II: no, it doesn't matter, whether the test fails or passes, the error gets swallowed.
|
The root of the error being swallowed is, that we have two So I see three options:
Edit: or we move the file operations to |
|
Yes, and there is Mocha in parallel mode, quite a recent feature. I'm unsure about the implications, but our docs states: We have to make sure there will be only one file created. |
|
@juergba thanks for your support. Right now I'm recovering from eye surgery so I've to reduce my screen time significantly for next few days. I will get back to this afterwards. Meanwhile feel free to modify PR yourself if you find solution. |
|
This PR hasn't had any recent activity, and I'm labeling it |
5d18965 to
6a93d47
Compare
d5162f3 to
eff4daa
Compare
juergba
left a comment
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.
@dorny thank you
Extends
JSONreporter with option to write output directly to file instead of a console.closes #4131
Description of the Change
EVENT_RUN_ENDit will write JSON output to given file - single call tofs.writeFileSync()fs.mkdirSync()runnerinstancedescribe()groupAlternate Designs
xunitreporter uses write stream and produces output continuously during test execution.Similar behavior is provided by
json-streamreporter.jsonreporter outputs a single json when the tests have completed so I've used single call tofs.writeFileSync()instead of creating write stream.I've also considered refactoring of
json.spec.js.Ideally unit test should execute
JSONReporterdirectly instead of using realRunnerinstance.Also tests should capture standard output and run assertion on it instead of using
testResultsproperty on runner which doesn't look to be used anywhere else.I've tried to rewrite tests using
createMockRunner()from test helpers but it didn't work.Mock runner doesn't supports simulation of multiple events and reporter state is not preserved between multiple runs.
Therefore it was not possible to use it for
jsonreporter without bigger changes.I still think refactoring the tests would improve code quality but it is out of scope of this PR.
Why should this be in core?
xunitreporterBenefits
Compared to redirecting report output to file using shell pipe: