detect tests that spawn subprocesses which leak handles #359
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
sunshowers
force-pushed
the
leak-detection
branch
12 times, most recently
from
41c01bc to
fb0a6e0
Compare
This will allow us to do things like select over file reads and a timeout. It also solves some other problems such as test flakiness on Windows (in the following commits).
* Convert relative paths to using the main separator. * Don't add paths if they don't exist on disk. I'd previously hypothesized that these fixes would help for Windows test flakiness, but they don't appear to do anything. What does appear to help is switching everything to `tokio::process::Command`, which I do in the future.
This appears to solve several problems at once: 1. Removes the redundant `make_test_command`/`make_test_expression` operations. 2. Adds automatic parallelism to the list step. 3. Appears to completely solve flakiness issues on Windows GitHub Actions runners (nextest-rs#362). Use the number of test threads to determine the degree of parallelism.
The flakiness issues on Windows appear to be completely resolved by the previous commit.
This test will be marked as leaky in the future.
Detect and expose leaky tests.
This prevents issues where the test process hangs because a subprocess inherited the stdin/out/err handles.
Some tests might accidentally read from standard input. Always close it.
Keep the same setting but make it configurable, both as a default setting and via overrides.
sunshowers
force-pushed
the
leak-detection
branch
from
fb0a6e0 to
da1ee60
Compare
sunshowers
changed the title
sunshowers
changed the title
This was referenced Jul 18, 2022
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.
Switch to tokio for the main runner loop so we get the ability to select across pipe reads and timeouts.
Use this to detect and handle tests which spawn subprocesses that leak handles. We treat them as passing tests for now, though might make leak failures configurable in the future.
Note that we can only detect subprocesses which inherit standard input/output/error. We can't detect subprocesses spawned with piped stdio that outlive the process. That's a far more challenging problem (see duct's gotchas.md for more). However this solves the immediate problem of nextest hanging because of unclosed pipes, and is probably the most common set of issues.
TODO:
Combine env var definitions between duct and tokio::process::Command(handled by switching list phase to use duct instead)cargo-nextest::integrationfixture checksCloses #15.