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 flaky test #81197

Closed
wants to merge 1 commit into from
Closed

Fix flaky test #81197

wants to merge 1 commit into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jan 19, 2021

Rustdoc writes to a pipe, and if the program on the other side has
exited, it will exit with SIGPIPE. To avoid that happening, call rustc
instead of true, which will wait to exit until it sees EOF on stdin.

cc #79705 (comment), #80924 (comment)

r? @ijackson

@jyn514 jyn514 added A-testsuite Area: The testsuite used to check the correctness of rustc T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-spurious Area: Spurious failures in builds (spuriously == for no apparent reason) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 19, 2021
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-9 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Suite("src/test/assembly") not skipped for "bootstrap::test::Assembly" -- not in ["src/tools/tidy"]
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 29 tests
iiiiiiiiiiiiiiiiiiiiiiiiiiiii

 finished in 0.059 seconds
Suite("src/test/incremental") not skipped for "bootstrap::test::Incremental" -- not in ["src/tools/tidy"]
Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
Suite("src/test/debuginfo") not skipped for "bootstrap::test::Debuginfo" -- not in ["src/tools/tidy"]
Check compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 116 tests
iiiiiiiiii.i.i..i..i..ii....i.i....ii..........iiii..........i....i...i.......ii.i.ii.....iiii.....i 100/116
test result: ok. 78 passed; 0 failed; 38 ignored; 0 measured; 0 filtered out; finished in 1.86s

 finished in 1.922 seconds
Suite("src/test/ui-fulldeps") not skipped for "bootstrap::test::UiFullDeps" -- not in ["src/tools/tidy"]
---
---- [rustdoc] rustdoc/issue-80893.rs stdout ----

error: rustdoc failed!
status: exit code: 101
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/issue-80893/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/issue-80893" "/checkout/src/test/rustdoc/issue-80893.rs" "--test" "-Z" "unstable-options" "--test-builder" "rustc"
------------------------------------------

running 1 test
test /checkout/src/test/rustdoc/issue-80893.rs - Foo (line 3) ... FAILED
test /checkout/src/test/rustdoc/issue-80893.rs - Foo (line 3) ... FAILED

failures:

---- /checkout/src/test/rustdoc/issue-80893.rs - Foo (line 3) stdout ----
thread '/checkout/src/test/rustdoc/issue-80893.rs - Foo (line 3)' panicked at 'Failed to spawn rustc process: Os { code: 2, kind: NotFound, message: "No such file or directory" }', src/librustdoc/doctest.rs:317:38



failures:
---
test result: FAILED. 410 passed; 1 failed; 2 ignored; 0 measured; 0 filtered out; finished in 25.59s



command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--rustdoc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "--src-base" "/checkout/src/test/rustdoc" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--suite" "rustdoc" "--mode" "rustdoc" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-9/bin/FileCheck" "--nodejs" "/usr/bin/node" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python3" "--lldb-python" "/usr/bin/python3" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "9.0.0" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets amdgpu amdgpuasmparser amdgpucodegen amdgpudesc amdgpudisassembler amdgpuinfo amdgpuutils 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 codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver engine executionengine fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interpreter ipo irreader jitlink lanai lanaiasmparser lanaicodegen lanaidesc lanaidisassembler lanaiinfo libdriver lineeditor linker lto 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 passes perfjitevents powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo riscvutils 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 windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info x86utils xcore xcorecodegen xcoredesc xcoredisassembler xcoreinfo xray" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test --exclude src/tools/tidy
Build completed unsuccessfully in 0:14:27

@jyn514
Copy link
Member Author

jyn514 commented Jan 19, 2021

Oh boo, rustc isn't available on CI because the only compiler available is the one built from source, which isn't in PATH.

Maybe we should just remove this test?

@Mark-Simulacrum
Copy link
Member

A run-make test should be able to do provide the rustc path correctly.

@jyn514
Copy link
Member Author

jyn514 commented Jan 19, 2021

Thanks, done. I had to build rustdoc for run-make tests.

r? @Mark-Simulacrum

@jyn514
Copy link
Member Author

jyn514 commented Jan 21, 2021

@bors p=5

This is causing spurious failures.

jyn514 added a commit to jyn514/rust that referenced this pull request Jan 21, 2021
See rust-lang#81197 for what's going on
here; this is a temporary stopgap until someone has time to review the
proper fix.
@jyn514 jyn514 mentioned this pull request Jan 21, 2021
@jyn514
Copy link
Member Author

jyn514 commented Jan 21, 2021

@bors p=0

#81231 is fixing the immediate issue.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2021
Remove flaky test

See rust-lang#81197 for what's going on
here; this is a temporary stopgap until someone has time to review the
proper fix.

r? `@ghost`
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

r=me with two nits fixed

src/test/run-make/rustdoc-test-builder/Makefile Outdated Show resolved Hide resolved
@@ -1003,7 +1003,7 @@ note: if you're sure you want to do this, please open an issue as to why. In the

// Avoid depending on rustdoc when we don't need it.
if mode == "rustdoc"
|| (mode == "run-make" && suite.ends_with("fulldeps"))
|| mode == "run-make"
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to avoid this and instead move the test to fulldeps.

Copy link
Member Author

Choose a reason for hiding this comment

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

That seems a shame when this doesn't actually need a stage 2 build :/ that means you have to compiler rustc twice to run the test at all. Rustdoc only takes about a minute to build even with a clean cache.

Copy link
Member

Choose a reason for hiding this comment

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

I don't want to make this change in this PR. We can consider it separately.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I reverted the change. I do think it would be useful though - #81223 could also use it, and that test is much more likely to fail, so building the compiler twice is painful.

@Mark-Simulacrum Mark-Simulacrum 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 Jan 22, 2021
@jyn514 jyn514 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 Jan 22, 2021
@Mark-Simulacrum
Copy link
Member

r=me with commits squashed

@Mark-Simulacrum Mark-Simulacrum 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 Jan 23, 2021
Rustdoc writes to a pipe, and if the program on the other side has
exited, it will exit with SIGPIPE. To avoid that happening, call `rustc`
instead of `true`, which will wait to exit until it sees EOF on stdin.

- Use a run-make-fulldeps test so that rustc is available in CI

  This is not exactly right, because the test shouldn't require building
  rustc twice, but it avoids having to build rustdoc for run-make tests.
@jyn514
Copy link
Member Author

jyn514 commented Jan 24, 2021

@bors r=Mark-Simulacrum rollup

@Mark-Simulacrum Mark-Simulacrum 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 Feb 12, 2021
@ijackson
Copy link
Contributor

Wouldn't e.g. cat also have the same effect of exiting at EOF rather than immediately? If so, we could use a regular UI test.

AIUI the program we specify here gets passed rustc flags. So we need something that:

  • will accept or ignore rustc command line options
  • will read the whole input
  • works on Windows too (which I think rules out something like providing a little script which is a wrapper around cat >/dev/null)

rustc seems like the obvious answer. Maybe we are getting the stage0 rustc and we need a stage1?

@Mark-Simulacrum
Copy link
Member

Yeah I'm afraid I don't have the time to do a deep dive into why the rustc being passed isn't found; I would recommend starting by making sure it is getting passed by printing out the command rustdoc tries to run when it fails...

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 1, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 9, 2021
…rk-Simulacrum

Build rustdoc for run-make tests, not just run-make-fulldeps

Rustdoc almost never needs a full stage 2 compiler, and requiring
rustdoc tests to be in run-make-fulldeps adds a lot of compile time for
no reason.

This is the same change from rust-lang#81197, but separated into its own PR. I ran into this again today while working on rust-lang/docs.rs#1302.
r? `@Mark-Simulacrum`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 9, 2021
…rk-Simulacrum

Build rustdoc for run-make tests, not just run-make-fulldeps

Rustdoc almost never needs a full stage 2 compiler, and requiring
rustdoc tests to be in run-make-fulldeps adds a lot of compile time for
no reason.

This is the same change from rust-lang#81197, but separated into its own PR. I ran into this again today while working on rust-lang/docs.rs#1302.
r? ``@Mark-Simulacrum``
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 9, 2021
…rk-Simulacrum

Build rustdoc for run-make tests, not just run-make-fulldeps

Rustdoc almost never needs a full stage 2 compiler, and requiring
rustdoc tests to be in run-make-fulldeps adds a lot of compile time for
no reason.

This is the same change from rust-lang#81197, but separated into its own PR. I ran into this again today while working on rust-lang/docs.rs#1302.
r? ```@Mark-Simulacrum```
@JohnCSimon JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 16, 2021
@JohnCSimon
Copy link
Member

@jyn514 - looks like this already has an approval and is waiting for merge.
@rustbot label: +S-waiting-on-review -S-waiting-on-author

@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 Mar 16, 2021
@jyn514
Copy link
Member Author

jyn514 commented Mar 16, 2021

@JohnCSimon This fails when tested on windows: #81197 (comment)

@rustbot label: +S-waiting-on-author -S-waiting-on-review

@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 Mar 16, 2021
@jyn514
Copy link
Member Author

jyn514 commented Mar 28, 2021

Yeah I'm afraid I don't have the time to do a deep dive into why the rustc being passed isn't found; I would recommend starting by making sure it is getting passed by printing out the command rustdoc tries to run when it fails...

I've tried to do this for about 15 minutes now and I'm really stumped - is there a way to show the output when a run-make test passes?

Here's what I've tried so far:

diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index 86d940cd733..9a33d952ff1 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -1257,7 +1257,7 @@ fn run(self, builder: &Builder<'_>) {
             cmd.arg("--verbose");
         }
 
-        if !builder.config.verbose_tests {
+        if !builder.config.verbose_tests && !builder.is_verbose() {
             cmd.arg("--quiet");
         }
 
@@ -1808,7 +1808,7 @@ fn run(self, builder: &Builder<'_>) {
         cargo.arg("--");
         cargo.args(&builder.config.cmd.test_args());
 
-        if !builder.config.verbose_tests {
+        if !builder.config.verbose_tests && !builder.is_verbose() {
             cargo.arg("--quiet");
         }
 
@@ -1921,7 +1921,7 @@ fn run(self, builder: &Builder<'_>) {
         dylib_path.insert(0, PathBuf::from(&*builder.sysroot_libdir(compiler, target)));
         cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());
 
-        if !builder.config.verbose_tests {
+        if !builder.config.verbose_tests && !builder.is_verbose() {
             cargo.arg("--quiet");
         }
 
@@ -1992,7 +1992,7 @@ fn run(self, builder: &Builder<'_>) {
             cargo.arg("'-Ctarget-feature=-crt-static'");
         }
 
-        if !builder.config.verbose_tests {
+        if !builder.config.verbose_tests && !builder.is_verbose() {
             cargo.arg("--quiet");
         }
 
$ RUST_TEST_NOCAPTURE=1 xpy test src/test/run-make-fulldeps --test-args rustdoc-test-builder -v
running 1 test


test [run-make] run-make-fulldeps/rustdoc-test-builder ... ok

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

	finished in 0.086 seconds

@jyn514
Copy link
Member Author

jyn514 commented Mar 28, 2021

Also, if there's a way to get CI to run tests for Windows instead of Linux on PR builds, that would be super helpful.

@crlf0710 crlf0710 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 17, 2021
@crlf0710
Copy link
Member

Also, if there's a way to get CI to run tests for Windows instead of Linux on PR builds, that would be super helpful.

I think it's possible. Maybe just edit the CI related configurations temporarily in this PR... and revert the changes after it's ready.

@jyn514
Copy link
Member Author

jyn514 commented Apr 23, 2021

I'm not planning to spend any more time on this.

@jyn514 jyn514 closed this Apr 23, 2021
@jyn514 jyn514 deleted the flaky-test branch April 23, 2021 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-spurious Area: Spurious failures in builds (spuriously == for no apparent reason) A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants