Capture output from threads spawned in tests#78227

SergioBenitez · 2020-10-22T10:26:07Z

This is revival of #75172.

Original text:

Fixes #42474.

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

Closes #75172.

library/test/src/helpers/sink.rs

m-ou-se · 2020-10-22T11:14:29Z

Looks good. Thanks!

Did you look at this failure from the original PR? #75172 (comment) Would that still be a problem?

m-ou-se · 2020-10-22T11:27:44Z

I think that failure is caused by the LOCAL_STDERR being destroyed before HANDLE is dropped in the spawned thread in that test:

rust/src/test/ui/issues/issue-24313.rs

Line 16 in 6b9fbf2

thread_local!(static HANDLE: Handle = Handle(0));

Before this change, that thread would not have LOCAL_STDERR set. But now that it is set, the panic!() in the Drop of this test's Handle will cause the panic handler to access it after it was set (in thread::spawn) and already destroyed, triggering this panic:

rust/library/std/src/thread/local.rs

Lines 248 to 251 in 6b9fbf2

    
                   self.try_with(f).expect( 
        
                       "cannot access a Thread Local Storage value \ 
        
                        during or after destruction", 
        
                   )

m-ou-se · 2020-10-22T11:30:32Z

Looks like that test doesn't run in the test framework, so now that this change checks LOCAL_STREAMS before setting any streams, the failure should be fixed.

m-ou-se

Lgtm, with one tiny nit.

m-ou-se · 2020-10-22T11:45:08Z

By the way: Panicking in the Drop implementaiton of a thread local in a thread in a unit test is probably not very common, but that situation could be partially fixed by making the panic handler use LOCAL_STDERR.try_with instead of .with. (It wouldn't capture the panic, but at least it would no longer trigger a double-panic.) Might be good as a follow-up PR (including a test that that works).

jyn514 · 2020-10-22T16:08:30Z

(FYI A-testsuite is for src/test, i.e. tests for the rust compiler itself, not for #[test])

SergioBenitez · 2020-10-23T01:15:28Z

Looks good. Thanks!

Did you look at this failure from the original PR? #75172 (comment) Would that still be a problem?

I thought there was a chance that the LOCAL_STREAMS check would resolve that issue. We'll see what the CI says.

Fixes rust-lang#42474.

m-ou-se · 2020-10-23T01:28:28Z

@bors r+

bors · 2020-10-23T01:28:30Z

📌 Commit db15596 has been approved by m-ou-se

bors · 2020-10-23T01:28:31Z

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

…, 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 · 2020-10-26T02:07:58Z

failed in rollup log (on wasm)

@bors r-

src/test/ui/test-thread-capture.rs

m-ou-se · 2020-10-26T22:21:50Z

@bors r+

bors · 2020-10-26T22:21:52Z

📌 Commit 7c4fe00 has been approved by m-ou-se

bors · 2020-10-27T11:43:24Z

⌛ Testing commit 7c4fe00 with merge 56d288f...

bors · 2020-10-27T13:57:46Z

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 56d288f to master...

m-ou-se · 2020-10-27T14:04:54Z

This is now merged. Thanks for reviving this!

SergioBenitez · 2020-10-27T14:37:44Z

This is now merged. Thanks for reviving this!

Thank you for the swift help!

Pkgsrc changes: * Adjust patches, convert tabs to spaces so that tests pass. * Remove patches which are no longer needed (upstream changed) * Minor adjustments for SunOS, e.g. disable stack probes. * Adjust cargo checksum patching accordingly. * Remove commented-out use of PATCHELF on NetBSD, which doesn't work anyway... Upstream changes: Version 1.49.0 (2020-12-31) ============================ Language ----------------------- - [Unions can now implement `Drop`, and you can now have a field in a union with `ManuallyDrop<T>`.][77547] - [You can now cast uninhabited enums to integers.][76199] - [You can now bind by reference and by move in patterns.][76119] This allows you to selectively borrow individual components of a type. E.g. ```rust #[derive(Debug)] struct Person { name: String, age: u8, } let person = Person { name: String::from("Alice"), age: 20, }; // `name` is moved out of person, but `age` is referenced. let Person { name, ref age } = person; println!("{} {}", name, age); ``` Compiler ----------------------- - [Added tier 1\* support for `aarch64-unknown-linux-gnu`.][78228] - [Added tier 2 support for `aarch64-apple-darwin`.][75991] - [Added tier 2 support for `aarch64-pc-windows-msvc`.][75914] - [Added tier 3 support for `mipsel-unknown-none`.][78676] - [Raised the minimum supported LLVM version to LLVM 9.][78848] - [Output from threads spawned in tests is now captured.][78227] - [Change os and vendor values to "none" and "unknown" for some targets][78951] \* Refer to Rust's [platform support page][forge-platform-support] for more information on Rust's tiered platform support. Libraries ----------------------- - [`RangeInclusive` now checks for exhaustion when calling `contains` and indexing.][78109] - [`ToString::to_string` now no longer shrinks the internal buffer in the default implementation.][77997] - [`ops::{Index, IndexMut}` are now implemented for fixed sized arrays of any length.][74989] Stabilized APIs --------------- - [`slice::select_nth_unstable`] - [`slice::select_nth_unstable_by`] - [`slice::select_nth_unstable_by_key`] The following previously stable methods are now `const`. - [`Poll::is_ready`] - [`Poll::is_pending`] Cargo ----------------------- - [Building a crate with `cargo-package` should now be independently reproducible.][cargo/8864] - [`cargo-tree` now marks proc-macro crates.][cargo/8765] - [Added `CARGO_PRIMARY_PACKAGE` build-time environment variable.] [cargo/8758] This variable will be set if the crate being built is one the user selected to build, either with `-p` or through defaults. - [You can now use glob patterns when specifying packages & targets.][cargo/8752] Compatibility Notes ------------------- - [Demoted `i686-unknown-freebsd` from host tier 2 to target tier 2 support.][78746] - [Macros that end with a semi-colon are now treated as statements even if they expand to nothing.][78376] - [Rustc will now check for the validity of some built-in attributes on enum variants.][77015] Previously such invalid or unused attributes could be ignored. - Leading whitespace is stripped more uniformly in documentation comments, which may change behavior. You read [this post about the changes][rustdoc-ws-post] for more details. - [Trait bounds are no longer inferred for associated types.][79904] Internal Only ------------- These changes provide no direct user facing benefits, but represent significant improvements to the internals and overall performance of rustc and related tools. - [rustc's internal crates are now compiled using the `initial-exec` Thread Local Storage model.][78201] - [Calculate visibilities once in resolve.][78077] - [Added `system` to the `llvm-libunwind` bootstrap config option.][77703] - [Added `--color` for configuring terminal color support to bootstrap.][79004] [75991]: rust-lang/rust#75991 [78951]: rust-lang/rust#78951 [78848]: rust-lang/rust#78848 [78746]: rust-lang/rust#78746 [78376]: rust-lang/rust#78376 [78228]: rust-lang/rust#78228 [78227]: rust-lang/rust#78227 [78201]: rust-lang/rust#78201 [78109]: rust-lang/rust#78109 [78077]: rust-lang/rust#78077 [77997]: rust-lang/rust#77997 [77703]: rust-lang/rust#77703 [77547]: rust-lang/rust#77547 [77015]: rust-lang/rust#77015 [76199]: rust-lang/rust#76199 [76119]: rust-lang/rust#76119 [75914]: rust-lang/rust#75914 [74989]: rust-lang/rust#74989 [79004]: rust-lang/rust#79004 [78676]: rust-lang/rust#78676 [79904]: rust-lang/rust#79904 [cargo/8864]: rust-lang/cargo#8864 [cargo/8765]: rust-lang/cargo#8765 [cargo/8758]: rust-lang/cargo#8758 [cargo/8752]: rust-lang/cargo#8752 [`slice::select_nth_unstable`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable [`slice::select_nth_unstable_by`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by [`slice::select_nth_unstable_by_key`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by_key [`hint::spin_loop`]: https://doc.rust-lang.org/stable/std/hint/fn.spin_loop.html [`Poll::is_ready`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_ready [`Poll::is_pending`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_pending [rustdoc-ws-post]: https://blog.guillaume-gomez.fr/articles/2020-11-11+New+doc+comment+handling+in+rustdoc

rust-highfive assigned dtolnay Oct 22, 2020

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

m-ou-se reviewed Oct 22, 2020

View reviewed changes

library/test/src/helpers/sink.rs Outdated Show resolved Hide resolved

m-ou-se added A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` A-testsuite Area: The testsuite used to check the correctness of rustc T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 22, 2020

m-ou-se approved these changes Oct 22, 2020

View reviewed changes

jyn514 added A-libtest Area: `#[test]` / the `test` library and removed A-testsuite Area: The testsuite used to check the correctness of rustc labels Oct 22, 2020

tmandry and others added 2 commits October 22, 2020 18:15

Capture output from threads spawned in tests

d0d0e78

Fixes rust-lang#42474.

Only load LOCAL_STREAMS if they are being used

db15596

SergioBenitez force-pushed the test-stdout-threading branch from f988c3f to db15596 Compare October 23, 2020 01:16

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 Oct 23, 2020

dtolnay assigned m-ou-se and unassigned dtolnay Oct 23, 2020

dtolnay mentioned this pull request Oct 23, 2020

Capture output from threads spawned in tests #75172

Closed

m-ou-se mentioned this pull request Oct 23, 2020

Panicking in Drop of thread_local gives fatal runtime error #78277

Closed

kylefleming mentioned this pull request Oct 23, 2020

Make a stderr configuration of the logger module, and use it in tests mobilecoinfoundation/mobilecoin#537

Merged

Dylan-DPC-zz mentioned this pull request Oct 25, 2020

Rollup of 9 pull requests #78362

Closed

Dylan-DPC-zz mentioned this pull request Oct 25, 2020

Rollup of 7 pull requests #78371

Closed

Dylan-DPC-zz mentioned this pull request Oct 26, 2020

Rollup of 10 pull requests #78383

Closed

Dylan-DPC-zz mentioned this pull request Oct 26, 2020

Rollup of 9 pull requests #78386

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 26, 2020

m-ou-se reviewed Oct 26, 2020

View reviewed changes

src/test/ui/test-thread-capture.rs Show resolved Hide resolved

Ignore threaded capture tests on WASM w/o threads

7c4fe00

SergioBenitez force-pushed the test-stdout-threading branch from 623cab4 to 7c4fe00 Compare October 26, 2020 21:35

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 26, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 27, 2020

bors merged commit 56d288f into rust-lang:master Oct 27, 2020

rustbot added this to the 1.49.0 milestone Oct 27, 2020

bors mentioned this pull request Oct 27, 2020

Add std::io::input simple input function. #75435

Closed

jebrosen mentioned this pull request Nov 1, 2020

Adopt Tracing rwf2/Rocket#1410

Closed

9 tasks

sfackler added the relnotes Marks issues that should be documented in the release notes of the next release. label Nov 5, 2020

m-ou-se mentioned this pull request Nov 6, 2020

Simplify output capturing #78714

Merged

AzureMarker mentioned this pull request Feb 8, 2022

Use custom test framework to cargo test ! rust3ds/ctru-rs#41

Merged

1 task

alexcrichton mentioned this pull request Jun 27, 2023

Panic messages sometimes eaten in tests rayon-rs/rayon#1066

Open

Uh oh!

Conversation

SergioBenitez commented Oct 22, 2020 • edited by m-ou-se Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

m-ou-se commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m-ou-se commented Oct 22, 2020

Uh oh!

m-ou-se commented Oct 22, 2020

Uh oh!

m-ou-se left a comment

Choose a reason for hiding this comment

Uh oh!

m-ou-se commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jyn514 commented Oct 22, 2020

Uh oh!

SergioBenitez commented Oct 23, 2020

Uh oh!

m-ou-se commented Oct 23, 2020

Uh oh!

bors commented Oct 23, 2020

Uh oh!

bors commented Oct 23, 2020

Uh oh!

Dylan-DPC-zz commented Oct 26, 2020

Uh oh!

Uh oh!

m-ou-se commented Oct 26, 2020

Uh oh!

bors commented Oct 26, 2020

Uh oh!

bors commented Oct 27, 2020

Uh oh!

bors commented Oct 27, 2020

Uh oh!

m-ou-se commented Oct 27, 2020

Uh oh!

SergioBenitez commented Oct 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

SergioBenitez commented Oct 22, 2020 •

edited by m-ou-se

Loading

m-ou-se commented Oct 22, 2020 •

edited

Loading

m-ou-se commented Oct 22, 2020 •

edited

Loading