Reject test runs when forks exit with an unexpected signal #680
Conversation
This file contains 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
The child process is no longer killed from the main process, instead it exits upon receiving an 'exit' message. This makes the changes from <#155> redundant.
|
By analyzing the blame information on this pull request, we identified @floatdrop, @ingro and @naptowncode to be potential reviewers |
|
I tested this branch. (no dependencies cache on these builds)
|
|
@thangngoc89 it seems fine locally (checked out MoOx/phenomic@f89c116). Maybe somehow strangely it did cache something? The dependency cache is based on the package version which hasn't changed in master. |
|
Oops hardcoded slashes in the assertions so the tests fail on Windows :( |
Yeah. I cleaned all cache. And tests are passing on node 5.
|
|
CI passes now. |
|
LGTM ;) |
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As discovered in #604 child processes can be killed by outside parties. In this case there is no exit code, though a signal is present. With this PR test runs are rejected with an appropriate error message, rather than the misleading "no test results received" message.
I've also cleaned up now redundant Node 0.10 SIGTERM handling (#155) and added a test for the existing test run rejection behavior.