run_tests_in_ci.js: fix truncated log output #7553
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ess.exit(1)` to fix truncated output
|
dconeybe
changed the title
process.exitCode=1 instead of calling process.exit(1) to fix truncated output
Size Report 1Affected Products
Test Logs |
|
Have you tested to ensure that GHA evaluates the return code correctly to detect that an error has occurred? |
Size Analysis Report 1Affected Products
Test Logs |
I've tested locally, but that's a good point. I'll test it out in CI as well. Thanks for the idea. |
…ses the GitHub Actions to fail
…re stdout/stderr, rather than implementing it ourselves since our own implementation seems to truncate the output, probably due to a misunderstanding of output buffering.
…he 'child-process-promise' library, so that capturing output is fully controlled and does not truncate.
…tting the exception bubble up, to avoid truncating output
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR fixes a long-standing bug in the GitHub continuous integration tests where the logs from failed tests were truncated, masking the failures, making investigation nearly impossible. The problem was in
scripts/run_tests_in_ci.jswhich buffered the test command's output in memory then printed it after the fact.In order to ensure that the script completed with a non-zero exit code (to indicate test failure) it called
process.exit(1)(docs). The problem is thatprocess.exit()documents that itSo the first problem was that
run_tests_in_ci.jscallingprocess.exit(1)would terminate the process before the test logs had been completely written to stdout/stderr, leading to truncated log output.The fix is to instead set
process.exitCode=1, which tells node to use the given exit code once all asynchronous work has completed, including flushing stdout/stderr.The second problem had to do with how the output from the command was manually captured and was being printed before it was fully buffered from the child process. The fix for this was to directly use node's
child_processmodule rather than thechild-process-promisedependency so that output capturing could be reliably implemented.