Skip to content

Fail tests that leak pipes #15

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

Closed
sunshowers opened this issue Feb 8, 2022 · 7 comments · Fixed by #359
Closed

Fail tests that leak pipes #15

sunshowers opened this issue Feb 8, 2022 · 7 comments · Fixed by #359
Labels
enhancement New feature or request
Milestone

Comments

@sunshowers
Copy link
Member

It is possible to write a test that ends up leaking a pipe (e.g. creates a process but doesn't terminate it). Currently, the test runner hangs on encountering such a test.

Instead, we should figure out a way to:

  • detect such a situation (a small amount of raciness between waiting on process exit and checking that the handles are closed is fine)
  • mark such a test as failure with a LEAK message or similar
@sunshowers sunshowers added this to the nextest-m2 milestone Feb 8, 2022
@sunshowers sunshowers added the enhancement New feature or request label Feb 8, 2022
@brkydnc
Copy link

brkydnc commented Feb 15, 2022

Hi, could you please give an example piece of code that causes this behavior? This is for understanding the problem better. Thanks in advance.

@sunshowers
Copy link
Member Author

What you need is:

  1. A test starts a server-like executable capturing its output
  2. The test panics without closing down the server
  3. The server never exits and stays running in memory

In these cases nextest will wait around for the server process to exit rather than marking the test as LEAK and moving on.

@Bauxitedev
Copy link

Bauxitedev commented Jul 7, 2022

I've run into this issue as well. Strangely enough, passing --no-capture to Nextest causes the tests to run normally. But if I remove the flag, the tests hang forever.

I also tried adding this to my .config/nextest.toml...

slow-timeout = {period = "30s", terminate-after = 2}

...but it still doesn't terminate the tests at all. (again, unless running with --no-capture, in which case it terminates properly)

This leads to an annoying Heisenbug where attempting to debug it, by passing --no-capture, causes the bug to disappear.

@Bauxitedev
Copy link

Bauxitedev commented Jul 7, 2022

Update: I made a quick reproduction test of the issue.

use std::{
    process::{Command, Stdio},
};

#[test]
fn bug() {
    let mut cmd = Command::new("sleep");
    cmd.args(vec!["9999"]);
    let mut child = cmd.spawn().unwrap();
}

If you run this test with cargo nextest, it will get stuck forever, even if you specify a value for terminate-after in .config/nextest.toml:
image

However, if you run cargo nextest --no-capture, it instantly finishes:
image

This is especially annoying for running a server process in the test, since if the test panics, the test will never finish and you don't get feedback about whether or not the test failed.

@sunshowers
Copy link
Member Author

Thanks for the further report. I agree that this should be a priority to fix given that even terminate-after isn't enough. Will look into it really soon.

@sunshowers
Copy link
Member Author

I started playing around with how to do this -- it involves switching to an async executor so that we can select on timeouts and pipe reads at the same time, which I'm doing in #359.

@sunshowers
Copy link
Member Author

@Bauxitedev this is now out as part of cargo-nextest 0.9.27.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants