-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 #3982, missing output which lacks newlines #8671
Conversation
0ed5dcc
to
27a1cfc
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.
If the last line printed to stdout or stderr was missing a terminating newline, it would go entirely missing (in all languages). The reason for this is a bug in the engine's handling of plugin outputs: Go's Reader.ReadString('\n') returns a string containing what was read and/or an error; if the string terminated in a '\n', the error is nil, and the entire line is returned; if the stream ends, however, a non-nil error is returned *and* what was read is returned, even though it wasn't terminated in a newline. The fix is simple: instead of ignoring that text, we use it, and *then* exit the read-loop. Also added some test cases since this is subtle and easy to regress.
27a1cfc
to
b46fd45
Compare
Codecov Report
@@ Coverage Diff @@
## master #8671 +/- ##
==========================================
- Coverage 58.95% 58.66% -0.29%
==========================================
Files 634 555 -79
Lines 97338 93968 -3370
Branches 1382 1382
==========================================
- Hits 57381 55127 -2254
+ Misses 36708 35590 -1118
- Partials 3249 3251 +2
Continue to review full report at Codecov.
|
Hmm, @lukehoban, the codecov report diffs don't make sense to me. Do they to you? The lines it says I am removing seem entirely unrelated to these changes 🤔 https://app.codecov.io/gh/pulumi/pulumi/compare/8671/changes |
No - they don't make sense to me either - none of the results from PRs the last month have been "correct" - we need to take deeper look at this. I think this is in part due to the issue I opened in #8629, but it also seems that there are just deeper holes in the coverage reporting, which I've amended onto that issue. For now, unfortunately, we largely have to just ignore this reporting. |
5bafcd8
to
1b4245e
Compare
If the last line printed to stdout or stderr was missing a
terminating newline, it would go entirely missing (in all languages).
The reason for this is a bug in the engine's handling of plugin
outputs: Go's Reader.ReadString('\n') returns a string containing what
was read and/or an error; if the string terminated in a '\n', the
error is nil, and the entire line is returned; if the stream ends,
however, a non-nil error is returned and what was read is returned,
even though it wasn't terminated in a newline. The fix is simple:
instead of ignoring that text, we use it, and then exit the read-loop.
Also added some test cases since this is subtle and easy to regress.