Fix false positive alerts from a run-pass test on Command. #19588
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
|
Sorry for the loss of precious CPU time of Buildbot! |
| let invalid = Command::new(too_long.as_slice()).spawn(); | ||
| assert!(invalid.is_err()); | ||
| } | ||
| let _failures = Vec::from_fn(100, |_i| { |
There was a problem hiding this comment.
Why are we storing all of the results? They're all just going to be Err(IoError { ... }), right?
There was a problem hiding this comment.
- It will do no harm anyways.
- This test code doesn't assume what will be returned in the
Errcase. Then runningfind_zombies()before destruction of theErrcase values means a (very slightly) stricter test.
sfackler
added a commit
to sfackler/rust
that referenced
this pull request
Dec 6, 2014
Test will be fixed in rust-lang#19588
sfackler
added a commit
to sfackler/rust
that referenced
this pull request
Dec 6, 2014
Test will be fixed in rust-lang#19588
|
I ended up disabling the test to get the queue unblocked. Could you rebase this and remove the |
Reported as a part of rust-lang#19120 The logic of rust-lang/rust@74fb798 was flawed because when a CI tool run the test parallely with other tasks, they all belong to a single session family and the test may pick up irrelevant zombie processes before they are reaped by the CI tool depending on timing. Also, panic! inside a loop over all children makes the logic simpler. By not destructing the return values of Command::spawn() until find_zombies() finishes, I believe we can conduct a slightly stricter test. Signed-off-by: NODA, Kai <nodakai@gmail.com>
nodakai
force-pushed
the
libstd-fix-zombie-children-finder
branch
from
54deac2 to
87424c6
Compare
|
@sfackler I've just pushed a rebased patch (with a small simplification of the loop logic) |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Dec 9, 2014
…inder Reported as a part of rust-lang#19120 The logic of rust-lang/rust@74fb798 was flawed because when a CI tool run the test parallely with other tasks, they all belong to a single session family and the test may pick up irrelevant zombie processes before they are reaped by the CI tool depending on timing.
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.
Reported as a part of #19120
The logic of 74fb798 was
flawed because when a CI tool run the test parallely with other tasks,
they all belong to a single session family and the test may pick up
irrelevant zombie processes before they are reaped by the CI tool
depending on timing.