Skip to content

Capture output from threads spawned in tests#75172

Closed
tmandry wants to merge 1 commit into
rust-lang:masterfrom
tmandry:test-thread-capture
Closed

Capture output from threads spawned in tests#75172
tmandry wants to merge 1 commit into
rust-lang:masterfrom
tmandry:test-thread-capture

Conversation

@tmandry

@tmandry tmandry commented Aug 5, 2020

Copy link
Copy Markdown
Member

Fixes #42474.

r? @dtolnay since you expressed interest in this, but feel free to redirect if you aren't the right person anymore.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 5, 2020

@dtolnay dtolnay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@dtolnay

dtolnay commented Aug 7, 2020

Copy link
Copy Markdown
Member

Er, CI failed though:

--- [ui] ui/panic-while-printing.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit code: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/panic-while-printing.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/panic-while-printing/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/panic-while-printing/auxiliary"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
error[E0277]: the trait bound `std::vec::Vec<_>: std::io::LocalOutput` is not satisfied
  --> /checkout/src/test/ui/panic-while-printing.rs:19:20
   |
LL |     set_panic(Some(Box::new(Vec::new())));
   |                    ^^^^^^^^^^^^^^^^^^^^ the trait `std::io::LocalOutput` is not implemented for `std::vec::Vec<_>`
   |
   = note: required for the cast to the object type `dyn std::io::LocalOutput`



--- [ui] ui/threads-sendsync/task-stderr.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit code: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/threads-sendsync/task-stderr.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/threads-sendsync/task-stderr/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/threads-sendsync/task-stderr/auxiliary"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
error[E0277]: the trait bound `Sink: std::io::LocalOutput` is not satisfied
  --> /checkout/src/test/ui/threads-sendsync/task-stderr.rs:24:28
   |
LL |         io::set_panic(Some(Box::new(sink)));
   |                            ^^^^^^^^^^^^^^ the trait `std::io::LocalOutput` is not implemented for `Sink`
   |
   = note: required for the cast to the object type `dyn std::io::LocalOutput`

@tmandry tmandry force-pushed the test-thread-capture branch from d266a8c to 4cd8de6 Compare August 18, 2020 00:31
@tmandry

tmandry commented Aug 18, 2020

Copy link
Copy Markdown
Member Author

Tests fixed!

@dtolnay

dtolnay commented Aug 18, 2020

Copy link
Copy Markdown
Member
error[E0277]: the trait bound `dyn realstd::io::LocalOutput: io::Write` is not satisfied
   --> library/std/src/panicking.rs:216:15
    |
216 |         write(&mut local);
    |               ^^^^^^^^^^ the trait `io::Write` is not implemented for `dyn realstd::io::LocalOutput`
    |
    = note: required because of the requirements on the impl of `io::Write` for `realstd::boxed::Box<dyn realstd::io::LocalOutput>`
    = note: required for the cast to the object type `dyn io::Write`

@tmandry

tmandry commented Aug 18, 2020

Copy link
Copy Markdown
Member Author

Yep, working on this now. Sorry for the noise.

@tmandry tmandry force-pushed the test-thread-capture branch from 4cd8de6 to 0d6fb9b Compare August 18, 2020 03:11
@tmandry tmandry force-pushed the test-thread-capture branch from 0d6fb9b to 38b920d Compare August 18, 2020 03:23
@tmandry

tmandry commented Aug 18, 2020

Copy link
Copy Markdown
Member Author

CI passed.

@dtolnay

dtolnay commented Aug 18, 2020

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Aug 18, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 38b920d has been approved by dtolnay

@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 Aug 18, 2020
tmandry added a commit to tmandry/rust that referenced this pull request Aug 18, 2020
Capture output from threads spawned in tests

Fixes rust-lang#42474.

r? @dtolnay since you expressed interest in this, but feel free to redirect if you aren't the right person anymore.
tmandry added a commit to tmandry/rust that referenced this pull request Aug 18, 2020
Capture output from threads spawned in tests

Fixes rust-lang#42474.

r? @dtolnay since you expressed interest in this, but feel free to redirect if you aren't the right person anymore.
@tmandry

tmandry commented Aug 19, 2020

Copy link
Copy Markdown
Member Author

@bors rollup=iffy

I'm trying to track down the cause of #75684 (comment) and while I don't think this can cause it (nor can I reproduce locally), nothing else in that change seems related.

@bors

bors commented Aug 19, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 38b920d with merge 7b86e1eeec51cc8ce5e8edf130f2df638605da97...

@tmandry

tmandry commented Aug 19, 2020

Copy link
Copy Markdown
Member Author

@bors retry
Yielding to rollup

@bors

bors commented Aug 19, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 38b920d with merge 049ea2e42ed0a109e27a83fa7636690d19e3bfc8...

@tmandry

tmandry commented Aug 19, 2020

Copy link
Copy Markdown
Member Author

@bors retry

@bors

bors commented Aug 19, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 38b920d with merge ea7c21aedf7c1bbab87df466b39bb79968f6804f...

@bors

bors commented Aug 19, 2020

Copy link
Copy Markdown
Collaborator

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 19, 2020
@tmandry

tmandry commented Aug 19, 2020

Copy link
Copy Markdown
Member Author

Same failure. Seems to only happen on the dist-i586-gnu-i586-i686-musl bot, I'll investigate more tomorrow.

@bors

bors commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #74862) made this pull request unmergeable. Please resolve the merge conflicts.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 18, 2020
@crlf0710

Copy link
Copy Markdown
Member

Triage: There's merge conflicts now.

@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 Oct 8, 2020
@m-ou-se

m-ou-se commented Oct 20, 2020

Copy link
Copy Markdown
Member

This PR partially breaks the effect of std::io::stdio::LOCAL_STREAMS:

/// Flag to indicate LOCAL_STDOUT and/or LOCAL_STDERR is used.
///
/// If both are None and were never set on any thread, this flag is set to
/// false, and both LOCAL_STDOUT and LOCAL_STDOUT can be safely ignored on all
/// threads, saving some time and memory registering an unused thread local.

clone_io should first check LOCAL_STREAMS.load(Relaxed), and return (None, None) if it was false. Otherwise, spawning a thread in non-test program will needlessly register thread local destructors for LOCAL_STDOUT and LOCAL_STDERR.

@dtolnay

dtolnay commented Oct 23, 2020

Copy link
Copy Markdown
Member

Closing in favor of #78227 which includes a rebase of this commit.

@dtolnay dtolnay closed this Oct 23, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 25, 2020
…, r=m-ou-se

Capture output from threads spawned in tests

This is revival of rust-lang#75172.

Original text:
> Fixes rust-lang#42474.
>
> r? `@dtolnay` since you expressed interest in this, but feel free to redirect if you aren't the right person anymore.

---

Closes rust-lang#75172.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 25, 2020
…, r=m-ou-se

Capture output from threads spawned in tests

This is revival of rust-lang#75172.

Original text:
> Fixes rust-lang#42474.
>
> r? `@dtolnay` since you expressed interest in this, but feel free to redirect if you aren't the right person anymore.

---

Closes rust-lang#75172.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 26, 2020
…, r=m-ou-se

Capture output from threads spawned in tests

This is revival of rust-lang#75172.

Original text:
> Fixes rust-lang#42474.
>
> r? `@dtolnay` since you expressed interest in this, but feel free to redirect if you aren't the right person anymore.

---

Closes rust-lang#75172.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 26, 2020
…, r=m-ou-se

Capture output from threads spawned in tests

This is revival of rust-lang#75172.

Original text:
> Fixes rust-lang#42474.
>
> r? `@dtolnay` since you expressed interest in this, but feel free to redirect if you aren't the right person anymore.

---

Closes rust-lang#75172.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 27, 2020
…r=m-ou-se

Capture output from threads spawned in tests

This is revival of rust-lang#75172.

Original text:
> Fixes rust-lang#42474.
>
> r? `@​dtolnay` since you expressed interest in this, but feel free to redirect if you aren't the right person anymore.

---

Closes rust-lang#75172.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cargo test doesn't capture print from threads

6 participants