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.

Sign up for GitHub

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

Jump to bottom

Add --nocapture option to rustdoc #86230

Merged
merged 5 commits into from
Jul 19, 2021
Merged

Add --nocapture option to rustdoc #86230

merged 5 commits into from
Jul 19, 2021

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Jun 11, 2021 •
edited
Loading

Fixes #26309.
Fixes #45724.

Once this PR is merged, I'll send a PR to cargo to also pass --nocapture to rustdoc.

cc @jyn514
r? @camelid

@GuillaumeGomez GuillaumeGomez added A-doctests Area: Documentation tests, run by rustdoc T-rustdoc labels Jun 11, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 11, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GuillaumeGomez GuillaumeGomez force-pushed the nocapture branch 2 times, most recently from 2a895a9 to 8f37814 Compare June 11, 2021 21:40
camelid
camelid reviewed Jun 12, 2021
View reviewed changes
src/doc/rustdoc/src/command-line-arguments.md Outdated Show resolved Hide resolved
src/librustdoc/config.rs Outdated Show resolved Hide resolved
src/librustdoc/doctest.rs Outdated Show resolved Hide resolved
camelid
camelid reviewed Jun 12, 2021
View reviewed changes
src/librustdoc/lib.rs Outdated Show resolved Hide resolved
src/librustdoc/markdown.rs Show resolved Hide resolved
@camelid camelid 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 Jun 12, 2021
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jun 12, 2021
Closed
@GuillaumeGomez
Copy link
Member Author

CI passed. Once this PR is merged, I'll send one to cargo.

@camelid camelid 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 Jun 13, 2021
@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed T-rustdoc-temp labels Jun 22, 2021
camelid
camelid reviewed Jul 3, 2021
View reviewed changes
src/librustdoc/doctest.rs Outdated Show resolved Hide resolved
@camelid camelid 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 Jul 3, 2021
@camelid
Copy link
Member

camelid commented Jul 3, 2021

Should this PR be marked as closing #45724 as well?

@camelid
Copy link
Member

camelid commented Jul 3, 2021

Also, it might be a good idea to test this code against the test case in #55813 to see if it fixes it.

@camelid
Copy link
Member

camelid commented Jul 3, 2021

I would also appreciate if you could explain to me a bit the difference between this new flag and --test-args --nocapture :)

@jyn514
Copy link
Member

jyn514 commented Jul 4, 2021

cc @ehuss - I don't know who owns libtest, but I've seen you reviewing a couple PRs, hopefully you have some input :)

@ehuss
Copy link
Contributor

ehuss commented Jul 8, 2021

I don't have any particular say in libtest. The libs team owns it, but in general nobody focuses on it.

I'd just echo @camelid's comments. I didn't look closely, but this seems strange that it is still capturing the output. I think the spirit of the --nocapture flag is to avoid capturing at all, to make it easier to debug tests that hang or run slowly or have other issues.

@bors
Copy link
Contributor

bors commented Jul 16, 2021

📌 Commit f9f6083 has been approved by camelid

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

jyn514 commented Jul 17, 2021

Shouldn't we have a way to pass through all these options to libtest? I wonder if it makes more sense to instead support a --test-args argument.

@GuillaumeGomez
Copy link
Member Author

nocapture needed changes on rustdoc side as well, but for other arguments, it'd be nice to pass arguments to libtest directly indeed. Can you open an issue for it please?

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 17, 2021
@GuillaumeGomez
Rollup merge of rust-lang#86230 - GuillaumeGomez:nocapture, r=camelid
6ca154d 
Add --nocapture option to rustdoc

Fixes rust-lang#26309.
Fixes rust-lang#45724.

Once this PR is merged, I'll send a PR to cargo to also pass `--nocapture` to rustdoc.

cc `@jyn514`
r? `@camelid`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 17, 2021
Closed
@JohnTitor
Copy link
Member

Failed in rollup: #87232 (comment)
@bors r-

@bors 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 Jul 18, 2021
@GuillaumeGomez
Copy link
Member Author

Failure came from the fact that I only "standardized" the stdout and not the stderr...

@GuillaumeGomez
Copy link
Member Author

@bors: r=camelid

@bors
Copy link
Contributor

bors commented Jul 18, 2021

📌 Commit d5e3294 has been approved by camelid

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 18, 2021
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 19, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 19, 2021
@bors
Auto merge of rust-lang#87269 - GuillaumeGomez:rollup-qukedv0, r=Guil…
8df945c 
…laumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#86230 (Add --nocapture option to rustdoc)
 - rust-lang#87210 (Rustdoc accessibility: make the sidebar headers actual headers)
 - rust-lang#87227 (Move asm! and global_asm! to core::arch)
 - rust-lang#87236 (Simplify command-line argument initialization on unix)
 - rust-lang#87251 (Fix "item info" width)
 - rust-lang#87256 (Extend HIR-based WF checking to associated type defaults)
 - rust-lang#87259 (triagebot shortcut config)
 - rust-lang#87268 (Don't create references to uninitialized data in `List::from_arena`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0fce468 into rust-lang:master Jul 19, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 19, 2021
@GuillaumeGomez GuillaumeGomez deleted the nocapture branch July 19, 2021 12:42
@woshilapin
Copy link
Contributor

woshilapin commented Jul 19, 2021 •
edited
Loading

The 2 above tickets have been automatically closed but if I understand correctly, there is still a missing piece in cargo to forward --nocapture to rustdoc, right? Should we reopen them until it's done?

@GuillaumeGomez
Copy link
Member Author

The work is done here so it's ok for them to be closed. I'm working on the cargo fix.

@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camelid camelid camelid left review comments

Assignees

@camelid camelid

Labels
A-doctests Area: Documentation tests, run by rustdoc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.55.0
10 participants
@GuillaumeGomez @rust-log-analyzer @camelid @jyn514 @ehuss @bors @JohnTitor @woshilapin @rust-highfive @rustbot