-
-
Notifications
You must be signed in to change notification settings - Fork 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
Add output option to JSON reporter (#4131) #4607
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
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
JSON
reporter with option to write output directly to file instead of a console.closes #4131
Description of the Change
EVENT_RUN_END
it will write JSON output to given file - single call tofs.writeFileSync()
fs.mkdirSync()
runner
instancedescribe()
groupAlternate Designs
xunit
reporter uses write stream and produces output continuously during test execution.Similar behavior is provided by
json-stream
reporter.json
reporter 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
JSONReporter
directly instead of using realRunner
instance.Also tests should capture standard output and run assertion on it instead of using
testResults
property 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
json
reporter 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?
xunit
reporterBenefits
Compared to redirecting report output to file using shell pipe: