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

Hide failed command unless in verbose mode #93492

Merged
merged 1 commit into from Feb 1, 2022

Conversation

Mark-Simulacrum
Copy link
Member

This is particularly intended for invoking compiletest; the command line there
is long (3,350 characters on my system) and takes up a lot of screen real estate
for little benefit to the majority of those running bootstrap. This moves
printing it to verbose mode (-v must be passed) which means that it's still
possible to access when needed for debugging.

The main downside is that CI logs will by-default become less usable for
debugging (particularly) spurious failures, but it is pretty rare for us to
really need the information there -- it's usually fairly obvious what is being
run with a little investigation.

r? @ehuss as you've done some of the spurious failure investigations, so can
(hopefully) confirm my intuition that this won't seriously hinder them.

This is particularly intended for invoking compiletest; the command line there
is long (3,350 characters on my system) and takes up a lot of screen real estate
for little benefit to the majority of those running bootstrap. This moves
printing it to verbose mode (-v must be passed) which means that it's still
possible to access when needed for debugging.

The main downside is that CI logs will by-default become less usable for
debugging (particularly) spurious failures, but it is pretty rare for us to
really need the information there -- it's usually fairly obvious what is being
run with a little investigation.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 30, 2022
@Mark-Simulacrum
Copy link
Member Author

As an example, for me locally the difference in running compiletest with one test failure is pretty significant (here the lines are chopped rather than wrapped, but at least my terminal typically wraps lines by default, which means there's a whole wall of text).

Before:

failures:
    [ui] ui/threads-sendsync/mpsc_stress.rs

test result: FAILED. 12448 passed; 1 failed; 111 ignored; 0 measured; 0 filtered out; finished in 61.52s

Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu

command did not execute successfully: "/home/mark/Build/rust/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/home/mark/Build/rust/build/x86_64-unknown-linux-gnu/stage1/lib" "--run-lib-path" "/home/mark/Build/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/home/mark/Build/rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "--src-base" "/home/mark/Build/rust/src/test/ui" "--build-base" "/home/mark/Build/rust/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage1-x86_64-unknown-linux-gnu" "--suite" "ui" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/home/mark/Build/rust/build/x86_64-unknown-linux-gnu/ci-llvm/bin/FileCheck" "--nodejs" "/usr/bin/node" "--npm" "/usr/bin/npm" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/home/mark/Build/rust/build/x86_64-unknown-linux-gnu/native/rust-test-helpers -Clink-arg=-fuse-ld=lld -Clink-arg=-Wl,--threads=1" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/home/mark/Build/rust/build/x86_64-unknown-linux-gnu/native/rust-test-helpers -Clink-arg=-fuse-ld=lld -Clink-arg=-Wl,--threads=1" "--docck-python" "/usr/bin/python" "--lldb-python" "/usr/bin/python" "--gdb" "/usr/bin/gdb" "--llvm-version" "13.0.0-rust-1.60.0-nightly" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter bpf bpfasmparser bpfcodegen bpfdesc bpfdisassembler bpfinfo cfguard codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker dwp engine executionengine extensions filecheck frontendopenacc frontendopenmp fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interfacestub interpreter ipo irreader jitlink libdriver lineeditor linker lto m68k m68kasmparser m68kcodegen m68kdesc m68kdisassembler m68kinfo mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcjit orcshared orctargetprocess passes powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo webassemblyutils windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info xray" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--channel" "nightly"
expected success, got: exit status: 1

After

failures:
    [ui] ui/threads-sendsync/mpsc_stress.rs

test result: FAILED. 12448 passed; 1 failed; 111 ignored; 0 measured; 0 filtered out; finished in 61.53s

Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu

@ehuss
Copy link
Contributor

ehuss commented Feb 1, 2022

Seems reasonable to me, I've rarely found those command outputs useful.

One minor downside is that you may need to now scroll up quite a ways just to figure out which overall testsuite is being run, as the final lines may not be obvious. It might be nice to have something similar to cargo that said To re-run these tests, run: ./x.py test library/std, but I suspect that could be difficult to do that accurately (or at least to get all the other flags the user may have passed like --stage=2).

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 1, 2022

📌 Commit 9bf6a5d has been approved by ehuss

@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 Feb 1, 2022
@Mark-Simulacrum
Copy link
Member Author

I suspect that the previous output was not super helpful towards giving a 'to rerun' help anyway, but yeah, I agree it'd be nice. I think Joshua tried doing something along those lines but it didn't really materialize well with the current bootstrap design.

@ehuss ehuss mentioned this pull request Feb 1, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 1, 2022
Rollup of 9 pull requests

Successful merges:

 - rust-lang#91343 (Fix suggestion to slice if scrutinee is a `Result` or `Option`)
 - rust-lang#93019 (If an integer is entered with an upper-case base prefix (0Xbeef, 0O755, 0B1010), suggest to make it lowercase)
 - rust-lang#93090 (`impl Display for io::ErrorKind`)
 - rust-lang#93456 (Remove an unnecessary transmute from opaque::Encoder)
 - rust-lang#93492 (Hide failed command unless in verbose mode)
 - rust-lang#93504 (kmc-solid: Increase the default stack size)
 - rust-lang#93513 (Allow any pretty printed line to have at least 60 chars)
 - rust-lang#93532 (Update books)
 - rust-lang#93533 (Update cargo)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 445fbff into rust-lang:master Feb 1, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 1, 2022
@jyn514
Copy link
Member

jyn514 commented Mar 3, 2022

I suspect that the previous output was not super helpful towards giving a 'to rerun' help anyway, but yeah, I agree it'd be nice. I think Joshua tried doing something along those lines but it didn't really materialize well with the current bootstrap design.

#86022

It was a lot more work than I was expecting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants