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

Test runner interacts badly with redirected child process #35136

Open
BartMassey opened this issue Jul 31, 2016 · 8 comments
Open

Test runner interacts badly with redirected child process #35136

BartMassey opened this issue Jul 31, 2016 · 8 comments
Labels
A-libtest Area: `#[test]` / the `test` library A-process Area: `std::process` and `std::env` C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@BartMassey
Copy link
Contributor

BartMassey commented Jul 31, 2016

(Moved here from rust-lang/cargo/2936 as it is believed to be a standard library bug.)

Consider the code at https://gitlab.com/BartMassey/ptyknot/tree/pipes-direct/misc/piperef-rs . The relevant portion is in piperef.rs:

        ...
       // Write "hello world" to stdout.                                       
        match write_mode {
            WriteMode::Macro => {println!("hello world");},
            WriteMode::C => {
                let buf = "hello world".as_bytes();
                let hello = std::ffi::CString::new(buf).unwrap();
                let bufptr = hello.as_ptr() as *const libc::c_void;
                check_cint!(libc::write(1, bufptr, buf.len()));
            }
        }
         ....

This code is run in a child process. When it is invoked by running it after cargo build, it works fine regardless of whether Macro or C mode is used. When it is invoked by cargo test, C mode works but Macro mode fails with an empty string. When it is invoked by cargo test -- --nocapture, both modes work again. Here's a transcript:

$ cargo build
   Compiling libc v0.2.14
   Compiling piperef v0.1.0 (file:///usr/local/src/ptyknot/misc/piperef-rs)
$ target/debug/piperef 
$ cargo test
   Compiling piperef v0.1.0 (file:///usr/local/src/ptyknot/misc/piperef-rs)
     Running target/debug/piperef-80efeb997239b2ac

running 2 tests
test write_macro_test ... FAILED$<2>
test write_c_test ... ok$<2>

failures:

---- write_macro_test stdout ----
    thread 'write_macro_test' panicked at 'assertion failed: `(left == right)` (left: `""`, right: `"hello world"`)', piperef.rs:60
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    write_macro_test

test result: FAILED$<2>. 1 passed; 1 failed; 0 ignored; 0 measured

error: test failed
$ cargo test -- --nocapture
     Running target/debug/piperef-80efeb997239b2ac

running 2 tests
test write_macro_test ... ok
test write_c_test ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured

$ 

It looks like the test runner is fouling up the environment somehow, but I can't figure out how by looking at stuff.

@ghost
Copy link

ghost commented Jul 31, 2016

There is a hidden functionality (std::io::set_print) that allows one to
provide a custom thread-local writer that will be used by print! like things.
libtest crate uses exactly that to capture the output, so in your test the
output never reaches the stdout.

@BartMassey
Copy link
Contributor Author

BartMassey commented Jul 31, 2016

Good to know; I looked for something like that, but never found it. Guess I wasn't looking carefully enough. Thanks much!

I don't quite understand what I should do about this, though? I dug around in stdio looking for a way to take stdout back at the beginning of the test, but couldn't quite figure out how to do it. In any case, gross... Suggestions?

For now, I guess I'll use stderr for the test, and use writeln!. Seems to work: the test runner doesn't grab stderr. Not ideal, but not hopeless either.

@Mark-Simulacrum
Copy link
Member

I'm not sure that there's much to be done here. The test runner does grab stdout by default, and I don't think that's going to change. Tests, I think, can call something along the lines of set_print(io::stdout()) before running if they want to return it to the default, but that's obviously non-ideal (and just passing --nocapture is probably easier). I'm going to close this as non-actionable, but if anyone has any thoughts as to how we could do better here, we'd be happy to hear them -- either here or in a new issue. We'll reopen at that point.

@BartMassey
Copy link
Contributor Author

I think maybe you could provide functionality that allows a test to re-grab the old stdout temporarily? It would require saving it somewhere. Perhaps some sort of "with" closure?

@Mark-Simulacrum
Copy link
Member

Hm, yeah, I think it's possible. I'll reopen; cc @rust-lang/libs, would we be interested in an API (probably in libtest) that allowed temporarily returning stdout/stderr for a given test? We could just have a #[no_capture] annotation as well, I think.

@BartMassey
Copy link
Contributor Author

BartMassey commented May 12, 2017

That would be great! I think I'd prefer the closure to #[no_capture] annotation for my specific case, since if I recall correctly, I wanted to do...things with stdout. But I don't remember for sure: it's been a while.

@sfackler
Copy link
Member

I dislike the weird stdout capturing logic in general and hope we can kill it off entirely someday specifically because it's magical and doesn't compose with child threads or processes.

@BartMassey
Copy link
Contributor Author

I'm actually only marginally sure what dumping stdout on the ground is even supposed to accomplish. Yes, it will make the tests quiet if they invoke code that writes on stdout. However, it seems like this is something that should be turned off by tests that want it rather than turned off for everybody, probably by some function which reassigns stdout when called. If I desire visible println!()s in my test (not my situation here), I don't see why the test framework should go out of its way to stop me.

@Mark-Simulacrum Mark-Simulacrum added the A-libtest Area: `#[test]` / the `test` library label Jun 20, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 25, 2017
@workingjubilee workingjubilee added the A-process Area: `std::process` and `std::env` label Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-libtest Area: `#[test]` / the `test` library A-process Area: `std::process` and `std::env` C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
Status: No status
Development

No branches or pull requests

4 participants