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

Fix --nocapture for run-make tests #134111

Merged
merged 2 commits into from
Dec 15, 2024
Merged

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 10, 2024

This was confusing because there are three layers of output hiding.

  1. libtest shoves all output into a buffer and does not print it unless the test fails or --nocapture is passed.
  2. compiletest chooses whether to print the output from any given process.
  3. run-make-support chooses what output to print.

This modifies 2 and 3.

  • compiletest: Don't require both --verbose and --nocapture to show the output of run-make tests.
  • compiletest: Print the output from rmake processes if they succeed. Previously this was only printed on failure.
  • compiletest: Distinguish rustc and rmake stderr by printing the command name (e.g. "--stderr--" to "--rustc stderr--").
  • run-make-support: Unconditionally print the needle/haystack being searched. Previously this was only printed on failure.

Before:

$ x t tests/run-make/linker-warning --force-rerun -- --nocapture
running 1 tests
.

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 281.64ms
$ x t tests/run-make/linker-warning --force-rerun -v -- --nocapture 2>&1 | wc -l
1004
$ x t tests/run-make/linker-warning --force-rerun -v -- --nocapture | tail -n40
running 1 tests

------stdout------------------------------

------stderr------------------------------
warning: unused import: `std::path::Path`
 --> /home/jyn/src/rust2/tests/run-make/linker-warning/rmake.rs:1:5
  |
1 | use std::path::Path;
  |     ^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `run_make_support::rfs::remove_file`
 --> /home/jyn/src/rust2/tests/run-make/linker-warning/rmake.rs:3:5
  |
3 | use run_make_support::rfs::remove_file;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: 2 warnings emitted

------------------------------------------
test [run-make] tests/run-make/linker-warning ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 285.89ms

After:

Testing stage1 compiletest suite=run-make mode=run-make (x86_64-unknown-linux-gnu)

running 1 tests
------rmake stdout------------------------------

------rmake stderr------------------------------
assert_contains_regex:
=== HAYSTACK ===
error: linking with `./fake-linker` failed: exit status: 1
  |
  = note: LC_ALL="C" PATH="/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin:...:/bin" VSLANG="1033" "./fake-linker" "-m64" "/tmp/rustcYqdAZT/symbols.o" "main.main.d17f5fbe6225cf88-cgu.0.rcgu.o" "main.2uoctswmurc6ir5rvoay0p9ke.rcgu.o" "-Wl,--as-needed" "-Wl,-Bstatic" "-Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-B/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin/gcc-ld" "-fuse-ld=lld" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/test/run-make/linker-warning/rmake_out" "-L" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "main" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs" "run_make_error"
  = note: error: baz

error: aborting due to 1 previous error

=== NEEDLE ===
fake-linker.*run_make_error
assert_not_contains_regex:
=== HAYSTACK ===

=== NEEDLE ===
fake-linker.*run_make_error

------------------------------------------
.

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 314.81ms

r? @jieyouxu

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Dec 10, 2024
@jyn514 jyn514 force-pushed the run-make-nocapture branch from 8f02d44 to aab18ee Compare December 10, 2024 09:11
@rustbot
Copy link
Collaborator

rustbot commented Dec 10, 2024

The run-make-support library was changed

cc @jieyouxu

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@jieyouxu
Copy link
Member

I'll do some local testing tmrw, thanks for the fix!

@jyn514 jyn514 force-pushed the run-make-nocapture branch from aab18ee to 3b463bd Compare December 10, 2024 09:16
@jieyouxu
Copy link
Member

jieyouxu commented Dec 12, 2024

FTR, local manual testing indicates there seems to be some stdout/stderr interleaving, possibly due to some combination of #134040 + #134123. Will look into it a bit later.

EDIT: fallout from #134040, unrelated to this PR, which will be addressed by revert #134207.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks! I've done some local testing and have some feedback.

src/tools/compiletest/src/runtest.rs Outdated Show resolved Hide resolved
src/tools/compiletest/src/runtest/run_make.rs Show resolved Hide resolved
src/tools/compiletest/src/runtest.rs Outdated Show resolved Hide resolved
@jieyouxu
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2024
@jyn514
Copy link
Member Author

jyn514 commented Dec 12, 2024

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 12, 2024
@jieyouxu
Copy link
Member

@rustbot blocked (on #134207)

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2024
@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Member

#134207 has merged so this is no longer blocked. However, I think there's one issue with the proc_name logic.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 13, 2024
This was confusing because there are three layers of output hiding.
1. libtest shoves all output into a buffer and does not print it unless the test fails or `--nocapture` is passed.
2. compiletest chooses whether to print the output from any given process.
3. run-make-support chooses what output to print.

This modifies 2 and 3.

- compiletest: Don't require both `--verbose` and `--nocapture` to show the output of run-make tests.
- compiletest: Distinguish rustc and rmake stderr by printing the command name (e.g. "--stderr--" to "--rustc stderr--").
- run-make-support: Unconditionally print the needle/haystack being searched. Previously this was only printed on failure.

Before:
```
$ x t tests/run-make/linker-warning --force-rerun -- --nocapture
running 1 tests
.

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 281.64ms
$ x t tests/run-make/linker-warning --force-rerun -v -- --nocapture 2>&1 | wc -l
1004
$ x t tests/run-make/linker-warning --force-rerun -v -- --nocapture | tail -n40
running 1 tests

------stdout------------------------------

------stderr------------------------------
warning: unused import: `std::path::Path`
 --> /home/jyn/src/rust2/tests/run-make/linker-warning/rmake.rs:1:5
  |
1 | use std::path::Path;
  |     ^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `run_make_support::rfs::remove_file`
 --> /home/jyn/src/rust2/tests/run-make/linker-warning/rmake.rs:3:5
  |
3 | use run_make_support::rfs::remove_file;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: 2 warnings emitted

------------------------------------------
test [run-make] tests/run-make/linker-warning ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 285.89ms
```

After:

```
Testing stage1 compiletest suite=run-make mode=run-make (x86_64-unknown-linux-gnu)

running 1 tests
------rmake stdout------------------------------

------rmake stderr------------------------------
assert_contains_regex:
=== HAYSTACK ===
error: linking with `./fake-linker` failed: exit status: 1
  |
  = note: LC_ALL="C" PATH="/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin:...:/bin" VSLANG="1033" "./fake-linker" "-m64" "/tmp/rustcYqdAZT/symbols.o" "main.main.d17f5fbe6225cf88-cgu.0.rcgu.o" "main.2uoctswmurc6ir5rvoay0p9ke.rcgu.o" "-Wl,--as-needed" "-Wl,-Bstatic" "-Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-B/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin/gcc-ld" "-fuse-ld=lld" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/test/run-make/linker-warning/rmake_out" "-L" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "main" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs" "run_make_error"
  = note: error: baz

error: aborting due to 1 previous error

=== NEEDLE ===
fake-linker.*run_make_error
assert_not_contains_regex:
=== HAYSTACK ===

=== NEEDLE ===
fake-linker.*run_make_error

------------------------------------------
.

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 314.81ms
```
@jyn514 jyn514 force-pushed the run-make-nocapture branch from 2a71007 to 056eb75 Compare December 15, 2024 01:46
@jyn514
Copy link
Member Author

jyn514 commented Dec 15, 2024

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 15, 2024
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Based on some local testing, this seems to work as expected (for the most part, On My Machine™). Example:

Testing stage1 compiletest suite=run-make mode=run-make (x86_64-pc-windows-msvc)

running 1 tests
------compiletest-self-test/rmake.exe stdout------------------------------
meow

------compiletest-self-test/rmake.exe stderr------------------------------
bark

------------------------------------------
.

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 382 filtered out; finished in 243.20ms

Thanks!

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 15, 2024

📌 Commit 056eb75 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 15, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 15, 2024
Rollup of 4 pull requests

Successful merges:

 - rust-lang#134111 (Fix `--nocapture` for run-make tests)
 - rust-lang#134329 (Add m68k_target_feature)
 - rust-lang#134331 (bootstrap: make ./x test error-index work)
 - rust-lang#134339 (Pass `TyCtxt` to early diagostics decoration)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fdd0eb0 into rust-lang:master Dec 15, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 15, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 15, 2024
Rollup merge of rust-lang#134111 - jyn514:run-make-nocapture, r=jieyouxu

Fix `--nocapture` for run-make tests

This was confusing because there are three layers of output hiding.
1. libtest shoves all output into a buffer and does not print it unless the test fails or `--nocapture` is passed.
2. compiletest chooses whether to print the output from any given process.
3. run-make-support chooses what output to print.

This modifies 2 and 3.

- compiletest: Don't require both `--verbose` and `--nocapture` to show the output of run-make tests.
- compiletest: Print the output from `rmake` processes if they succeed. Previously this was only printed on failure.
- compiletest: Distinguish rustc and rmake stderr by printing the command name (e.g. "--stderr--" to "--rustc stderr--").
- run-make-support: Unconditionally print the needle/haystack being searched. Previously this was only printed on failure.

Before:
```
$ x t tests/run-make/linker-warning --force-rerun -- --nocapture
running 1 tests
.

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 281.64ms
$ x t tests/run-make/linker-warning --force-rerun -v -- --nocapture 2>&1 | wc -l
1004
$ x t tests/run-make/linker-warning --force-rerun -v -- --nocapture | tail -n40
running 1 tests

------stdout------------------------------

------stderr------------------------------
warning: unused import: `std::path::Path`
 --> /home/jyn/src/rust2/tests/run-make/linker-warning/rmake.rs:1:5
  |
1 | use std::path::Path;
  |     ^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `run_make_support::rfs::remove_file`
 --> /home/jyn/src/rust2/tests/run-make/linker-warning/rmake.rs:3:5
  |
3 | use run_make_support::rfs::remove_file;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: 2 warnings emitted

------------------------------------------
test [run-make] tests/run-make/linker-warning ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 285.89ms
```

After:

```
Testing stage1 compiletest suite=run-make mode=run-make (x86_64-unknown-linux-gnu)

running 1 tests
------rmake stdout------------------------------

------rmake stderr------------------------------
assert_contains_regex:
=== HAYSTACK ===
error: linking with `./fake-linker` failed: exit status: 1
  |
  = note: LC_ALL="C" PATH="/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin:...:/bin" VSLANG="1033" "./fake-linker" "-m64" "/tmp/rustcYqdAZT/symbols.o" "main.main.d17f5fbe6225cf88-cgu.0.rcgu.o" "main.2uoctswmurc6ir5rvoay0p9ke.rcgu.o" "-Wl,--as-needed" "-Wl,-Bstatic" "-Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-B/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin/gcc-ld" "-fuse-ld=lld" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/test/run-make/linker-warning/rmake_out" "-L" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "main" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs" "run_make_error"
  = note: error: baz

error: aborting due to 1 previous error

=== NEEDLE ===
fake-linker.*run_make_error
assert_not_contains_regex:
=== HAYSTACK ===

=== NEEDLE ===
fake-linker.*run_make_error

------------------------------------------
.

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 377 filtered out; finished in 314.81ms
```

r? `@jieyouxu`
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 16, 2024
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#134111 (Fix `--nocapture` for run-make tests)
 - rust-lang#134202 (Remove `rustc::existing_doc_keyword` lint)
 - rust-lang#134314 (Make sure to use normalized ty for unevaluated const in default struct value)
 - rust-lang#134329 (Add m68k_target_feature)
 - rust-lang#134342 (crashes: more tests)
 - rust-lang#134354 (Handle fndef rendering together with signature rendering)

Failed merges:

 - rust-lang#133265 (Add a range argument to vec.extract_if)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants