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.
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
formatters: include html-formatter as a built-in option #1432
formatters: include html-formatter as a built-in option #1432
Changes from all commits
0551e27
8df6974
c07ad7a
2eeaa57
bb67afc
2107862
a594232
740838c
c866b5f
b9c20a0
a0df3b8
f03ee65
e908079
b9175c0
e63f088
01c66db
5e5837b
b6d252c
ad8a581
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Almost all of the rest of this file is just indentation changes - this is the addition, where we wait for each formatter to be finished.
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.
Thoughts on making the default formatter
finished
function end the stream?Then for the html formatter, it waits for the cucumber html stream to finish and then also awaits on super? Think that simplifies this a little bit.
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.
That does seem like a good balance.
In cases where the formatter has asked to write to
stdout
, we can't just end the stream when the formatter is done writing, because the process still needsstdout
. The current code accounts for this by not adding it to the list of streams to end.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.
If a formatter's writable stream is written to by piping another readable stream into it, then we just have to wait until the pipe is finished. We cannot prematurely close it.
This is what's happening in this case. The CucumberHtmlStream is piped to the formatter's stream. We have no control over when it's done.
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.
Okay I have pushed something which I believe accounts for all of this whilst keeping a straightforward API for formatter authors - though not sure it really comes out any simpler. Let me know what you think. Also clarified my comment above.
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.
I think it would be best to wait for it to have written everything. The correct way to keep stdout open is to
stream.pipe(process.stdout, {end: false})