Skip to content
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

tests::test_into_inner_after_wait test fail with Rust 1.72.0 and newer #23

Closed
decathorpe opened this issue Sep 26, 2023 · 2 comments
Closed

Comments

@decathorpe
Copy link

We maintain a package for this crate in Fedora Linux, and I noticed that the package now fails to build since we got the upgrade to Rust 1.72.0. This is the output from cargo test --release:

running 10 tests
test tests::test_into_inner_after_wait ... FAILED
test tests::test_into_inner_before_wait ... ok
test tests::test_kill ... ok
test tests::test_takes ... ok
test tests::test_try_wait ... ok
test tests::test_wait ... ok
test tests::test_waitid_after_exit_doesnt_hang ... ok
test tests::test_many_waiters ... ok
test tests::test_new ... ok
test unix::tests::test_send_signal ... ok
failures:
---- tests::test_into_inner_after_wait stdout ----
thread 'tests::test_into_inner_after_wait' panicked at 'called `Result::unwrap_err()` on an `Ok` value: ()', src/lib.rs:404:37
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
    tests::test_into_inner_after_wait
test result: FAILED. 9 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.11s
error: test failed, to rerun pass `--lib`
error: 1 target failed:
    `--lib`

There is a suspicious item in the Rust 1.72.0 release notes (https://github.com/rust-lang/rust/releases/tag/1.72.0):

Return Ok on kill if process has already exited

@oconnor663
Copy link
Owner

Wow, I hadn't noticed that change, thank you. I agree that the new kill behavior is better.

I just pushed 8001062 to remove that check from the test. Do you need a release with this fix, or is it enough for it to be on master?

@decathorpe
Copy link
Author

I don't need a new version, having confirmation that skipping this test is a safe thing to do is enough for now. Thank you!

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

No branches or pull requests

2 participants