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

stop spraying numbers to stdout during test runs #892

Merged
merged 1 commit into from
Apr 8, 2022
Merged

Conversation

davepacheco
Copy link
Collaborator

After #832, a regular cargo test includes junk emitted by the "unauthorized" test. Here's what it looks like:

$ cargo test -p omicron-nexus --test=test_all integration_tests::unauthorized
...
    Finished test [unoptimized + debuginfo] target(s) in 2m 40s
     Running tests/test_all.rs (target/debug/deps/test_all-e908bfecb6111b9f)

running 2 tests
test integration_tests::unauthorized_coverage::test_unauthorized_coverage ... ok
03111555531115555555500411141115555411155550041115555411155555555004111411155554111555500411155554111555555550041114111555541115555004111411155555555555500411155554111555555550041114111555541115555004111555555555555555500411155554111555555550041114111555541115555004111555541115555555500411141115555411155550041115555411155555555004111555555554111555500411155555555555555550-55555555411155555555--55555555411155555555-041115555411155555555-041115555555541115555-041115555411155555555-041115555555541115555-04111555541115555555500411155555555411155550-55555555411155555555--55555555411155555555--55555555411155555555--55555555411155555555-04111555541115555555500411155555555411155550031115555555555555555004111555555555555555500311155555555555555550041115555555555555555003111555555555555555500411155555555555555550031115555555555555555004111555555555555555500311155555555555555550-31115555555555555555-0311155555555555555550-55555555311155555555-031115555311155555555-041115555555541115555-test integration_tests::unauthorized::test_unauthorized ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 58 filtered out; finished in 12.47s

After a lot of digging, I found that under normal conditions (i.e., without --nocapture), the built-in test runner apparently only redirects output from the print/eprint family. It does not redirect all of stdout -- and in particular it does not redirect writes made with write!. As a result, we were spraying these characters to stdout even when --nocapture wasn't specified.

After this change:

$ cargo test -p omicron-nexus --test=test_all integration_tests::unauthorized
   Compiling omicron-nexus v0.1.0 (/home/dap/omicron-auth-2/nexus)
   Compiling nexus-test-utils v0.1.0 (/home/dap/omicron-auth-2/nexus/test-utils)
    Finished test [unoptimized + debuginfo] target(s) in 1m 01s
     Running tests/test_all.rs (target/debug/deps/test_all-e908bfecb6111b9f)

running 2 tests
test integration_tests::unauthorized_coverage::test_unauthorized_coverage ... ok
test integration_tests::unauthorized::test_unauthorized ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 58 filtered out; finished in 12.64s

But you still get the output with --nocapture:

$ cargo test -p omicron-nexus --test=test_all -- --nocapture integration_tests::unauthorized::test_unauthorized
    Finished test [unoptimized + debuginfo] target(s) in 0.38s
     Running tests/test_all.rs (target/debug/deps/test_all-e908bfecb6111b9f)

running 1 test
log file: "/dangerzone/omicron_tmp/test_all-e908bfecb6111b9f-test_unauthorized.11897.0.log"
note: configured to log to "/dangerzone/omicron_tmp/test_all-e908bfecb6111b9f-test_unauthorized.11897.0.log"

SUMMARY OF REQUESTS MADE

KEY, USING HEADER AND EXAMPLE ROW:

          +----------------------------> privileged GET (expects 200 or 500)
          |                              (digit = last digit of status code)
          |
          |                          +-> privileged GET (expects same as above)
          |                          |   (digit = last digit of status code)
          |                          |   ('-' => skipped (N/A))
          ^                          ^
HEADER:   G GET  PUT  POST DEL  TRCE G  URL
EXAMPLE:  0 3111 5555 3111 5555 5555 0  /organizations
    ROW     ^^^^
            ||||                      TEST CASES FOR EACH HTTP METHOD:
            +|||----------------------< authenticated, unauthorized request
             +||----------------------< unauthenticated request
              +|----------------------< bad authentication: no such user
               +----------------------< bad authentication: invalid syntax

            \__/ \__/ \__/ \__/ \__/
            GET  PUT  etc.  The test cases are repeated for each HTTP method.

            The number in each cell is the last digit of the 400-level response
            that was expected for this test case.

    In this case, an unauthenthicated request to "GET /organizations" returned
    401.  All requests to "PUT /organizations" returned 405.

G GET  PUT  POST DEL  TRCE G  URL
0 3111 5555 3111 5555 5555 0  /organizations
0 4111 4111 5555 4111 5555 0  /organizations/demo-org
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc
0 4111 4111 5555 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/firewall/rules
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/subnets
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/subnets/demo-vpc-subnet
0 4111 5555 5555 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/subnets/demo-vpc-subnet/network-interfaces
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/routers
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/routers/demo-vpc-router
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/routers/demo-vpc-router/routes
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/routers/demo-vpc-router/routes/demo-router-route
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/disks
0 4111 5555 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/disks/demo-disk
0 4111 5555 5555 5555 5555 0  /organizations/demo-org/projects/demo-project/instances/demo-instance/disks
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/disks/attach
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/disks/detach
0 4111 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/images
0 4111 5555 5555 4111 5555 -  /organizations/demo-org/projects/demo-project/images/demo-image
0 4111 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/snapshots
0 4111 5555 5555 4111 5555 -  /organizations/demo-org/projects/demo-project/snapshots/demo-snapshot
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/instances
0 4111 5555 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/instances/demo-instance
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/start
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/stop
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/reboot
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/migrate
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/instances/demo-instance/network-interfaces
0 4111 5555 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/instances/demo-instance/network-interfaces/default
0 3111 5555 5555 5555 5555 0  /roles
0 4111 5555 5555 5555 5555 0  /roles/fleet.admin
0 3111 5555 5555 5555 5555 0  /users
0 4111 5555 5555 5555 5555 0  /users/db-init
0 3111 5555 5555 5555 5555 0  /hardware/racks
0 4111 5555 5555 5555 5555 0  /hardware/racks/c19a698f-c6f9-4a17-ae30-20d711b8f7dc
0 3111 5555 5555 5555 5555 0  /hardware/sleds
0 4111 5555 5555 5555 5555 0  /hardware/sleds/b6d65341-167c-41df-9b5c-41cded99c229
0 3111 5555 5555 5555 5555 0  /sagas
- 3111 5555 5555 5555 5555 -  /sagas/48a1b8c8-fc1c-6fea-9de9-fdeb8dda7823
0 3111 5555 5555 5555 5555 0  /timeseries/schema
- 5555 5555 3111 5555 5555 -  /updates/refresh
0 3111 5555 3111 5555 5555 -  /images
0 4111 5555 5555 4111 5555 -  /images/demo-image
test integration_tests::unauthorized::test_unauthorized ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 59 filtered out; finished in 12.41s


@davepacheco davepacheco requested a review from plotnick April 8, 2022 00:20
Copy link
Contributor

@david-crespo david-crespo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't argue with that

@davepacheco
Copy link
Collaborator Author

I wanted to capture a bit more of the debugging here. I noticed the spew of output and recognized as coming from this test. I found it surprising because I deliberately wrote this output to stdout so that it would be captured in the normal case. Once I convinced myself it really was trying to write to stdout and it wasn't being captured, I tried reproducing it with a little more information using this test program:

fn main() {
    doit();
}

fn doit() {
    let t = term::stdout();
    if let Some(mut term) = t {
        eprintln!("stdout found");
        term.fg(term::color::GREEN).unwrap();
        write!(term, "writing this to stdout terminal!\n").unwrap();
        term.reset().unwrap();
        term.flush().unwrap();
    } else {
        eprintln!("no stdout found");
    }
}

#[cfg(test)]
#[test]
fn test_doit() {
    doit();
}

This way, the test code can be run as a binary or a unit test. As a program, it works the way I expect:

$ cargo run
   Compiling term-test v0.1.0 (/Users/dap/oxide/experiments/term)
    Finished dev [unoptimized + debuginfo] target(s) in 0.27s
     Running `target/debug/term-test`
stdout found
writing this to stdout terminal!

If you redirect stdout or stderr, you’re left with whatever you didn’t redirect in your terminal (as you’d expect):

$ cargo run > /dev/null
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/term-test`
stdout found
$ cargo run 2> /dev/null
writing this to stdout terminal!
$

You can redirect stdout to a file, and we’ll see that the output we wrote to the "terminal" is there, escape sequences and all:

$ cargo run > foo
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/term-test`
stdout found
$ xxd foo
00000000: 1b5b 3332 6d77 7269 7469 6e67 2074 6869  .[32mwriting thi
00000010: 7320 746f 2073 7464 6f75 7420 7465 726d  s to stdout term
00000020: 696e 616c 210a 1b28 421b 5b6d            inal!..(B.[m
$

Okay, so term gives you back a terminal interface for stdout regardless of whether it’s a real terminal.

What about as a unit test?

If we use --nocapture, we get all of the output, as you’d expect:

$ cargo test -- --nocapture
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running unittests (target/debug/deps/term_test-f7fd9a9422a0407e)

running 1 test
stdout found
writing this to stdout terminal!
test test_doit ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

and the individual pieces go where you (might) expect:

$ cargo test -- --nocapture >/dev/null
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running unittests (target/debug/deps/term_test-f7fd9a9422a0407e)
stdout found

$ cargo test -- --nocapture 2>/dev/null

running 1 test
writing this to stdout terminal!
test test_doit ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

$

Sure enough: if you don’t use --nocapture, you still get the terminal output! Even though you don’t usually get stdout from tests without --nocapture.

$ cargo test
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running unittests (target/debug/deps/term_test-f7fd9a9422a0407e)

running 1 test
writing this to stdout terminal!
test test_doit ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

My first thought was that maybe term::stdout() has special-case logic to find the controlling terminal and write to that. I dug in the code a bit and found no such logic. Creating a stdout terminal just creates a terminal wrapper over the stream that happens to be Stdout:
https://github.com/Stebalien/term/blob/aa06a100aa93dcb31fc42a836eaedbc5955270b8/src/lib.rs#L82-L92

And the Write impl goes straight to the underlying Write that you gave it:
https://github.com/Stebalien/term/blob/aa06a100aa93dcb31fc42a836eaedbc5955270b8/src/terminfo/mod.rs#L392-L400

I modified my test program to print some stuff to stdout before and after the write to the terminal, so it looked like this:

        eprintln!("println to stderr (stdout found)");
        println!("println to stdout (before)");
        term.fg(term::color::GREEN).unwrap();
        write!(term, "terminal write to stdout\n").unwrap();
        term.reset().unwrap();
        term.flush().unwrap();
        println!("println to stdout (after)");

That has the same behavior!

$ cargo test
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running unittests (target/debug/deps/term_test-f7fd9a9422a0407e)

running 1 test
terminal write to stdout
test test_doit ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

$

I found this pretty surprising. I also modified the test program to sleep for 60 seconds, ran cargo test (without --nocapture), then ran pfiles on the test runner process to see what stdout (fd 1) really was. It was the controlling terminal for the shell from which I ran cargo test (/dev/pts/2)! That's surprising. How does the capture work then? This all makes me wonder if --nocapture works not by running the thing in a child process with redirected stdout (which is what I had assumed) but by somehow changing what println and friends redirect to in a way that doesn't affect the way term::stdout finds stdout (which is via std::io::stdout). But that would seem strange on levels.

But that's the case. When the test runner runs things in the same process (which I think is the normal mode), it uses this hidden API in std:
https://github.com/rust-lang/rust/blob/60e50fc1cfe0bb693a5f4f93eb83ef70854531e3/library/test/src/lib.rs#L579-L593

That API is implemented here:
https://github.com/rust-lang/rust/blob/2310da432ca1c3e87f1d17e069f2336f2d4939e5/library/std/src/io/stdio.rs#L969-L984

It appears to cause output from print/eprint and friends within the calling thread to be redirected to some other drain. OUTPUT_CAPTURE is only used in print_to, which is a non-pub function in the same file. That's used by _print and _eprint, which are documented as part of the internals of the print subsystem. I gather these implement print!/println!/eprint!/eprintln!. But these wouldn't be used for write!.

Sure enough, the behavior reproduces even with this program, with no term on the scene at all:

$ cat src/main.rs 
use std::io::Write;

fn main() {
    doit();
}

fn doit() {
    eprintln!("println to stderr (stdout found)");
    println!("println to stdout (before)");
    write!(std::io::stdout(), "write to stdout\n").unwrap();
    println!("println to stdout (after)");
}

#[cfg(test)]
#[test]
fn test_doit() {
    doit();
}

This produces:

$ cargo test
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running unittests (target/debug/deps/term_test-5fb129bbaf3ecd42)

running 1 test
write to stdout
test test_doit ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

I think this explains all the observations. The summary is that under normal operation (i.e., without --nocapture), output from print/eprint and friends within the test thread is captured, but not any other writes to stdout, including those via write! or from some other thread. This probably also explains why cargo test spews a bunch of log output -- we use slog_async, which would be writing the data to stdout from another tokio task, which in general could be running in a different thread.

@davepacheco davepacheco merged commit 15168d3 into main Apr 8, 2022
@davepacheco davepacheco deleted the term-fix branch April 8, 2022 02:06
@davepacheco
Copy link
Collaborator Author

Not surprisingly, I'm not the first person to run into this:

rust-lang/rust#12309
rust-lang/rust#42474
rust-lang/rust#90785
rust-lang/rust#92370

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

Successfully merging this pull request may close these issues.

2 participants