Passing `--nocapture` to rustdoc behaves in strange ways

[RalfJung]

Problem

I am debugging a diverging doctest by adding some printing. For example:

/// ```rust
/// eprintln!("stderr");
/// println!("stdout");
/// loop {}
/// ```
pub fn uwu() {}

Steps

I would expect cargo test --doc -- --nocapture to show the stderr and stdout of running the test. However, that does not seem to be the case; it only shows stderr and stdour of rustc as it builds the test.

I found that RUSTDOCFLAGS="-Zunstable-options --nocapture" cargo test --doc does what I want.

Strangely, RUSTDOCFLAGS="-Zunstable-options --nocapture" cargo test --doc -- --nocapture says that "Option 'nocapture' given more than once", but removing the one in RUSTDOCFLAGS does not have the same result as removing the one after --.

Possible Solution(s)

No response

Notes

I don't quite know what is going on here. cargo test --doc -- --help seems to show the help page of a libtest runner, not of rustdoc. The "Option 'nocapture' given more than once" error shows that cargo is forwarding its --nocapture argument to rustdoc, but somehow it is doing that in a way that doesn't actually have any effect?

Version

cargo 1.64.0-nightly (a5e08c470 2022-06-23)
release: 1.64.0-nightly
commit-hash: a5e08c4703f202e30cdaf80ca3e7c00baa59c496
commit-date: 2022-06-23
host: x86_64-unknown-linux-gnu
libgit2: 1.4.2 (sys:0.14.2 vendored)
libcurl: 7.83.1-DEV (sys:0.4.55+curl-7.83.1 vendored ssl:OpenSSL/1.1.1n)
os: Debian testing (bookworm) [64-bit]

[ehuss]

This is a consequence of the execution model for doctests. The execution model for rustc tests is:

1. rustc --test generates an executable whose main is the libtest harness synthesized main.
2. Cargo executes that executable with the arguments listed after --.
3. The libtest harness parses those arguments and performs the tests as instructed.
4. Cargo relays any output from the test to the console.

In contrast to rustdoc which is:

1. rustdoc --test instructs rustdoc to collect all code blocks, and handle running all tests itself.
2. Rustdoc internally uses the libtest harness to drive the tests.
3. Each "test" runs by compiling the code block into a separate executable with a standalone main. That executable does not do any argument parsing.
4. Arguments passed after -- to cargo are passed by cargo to rustdoc via --test-args.
5. Rustdoc grabs those --test-args and relays them to the test harness.
6. Crucially: The rustdoc test wrapper is spawning a process. Rustdoc's default behavior is to capture and discard all output. This means that passing --nocapture causes println! and friends to not be suppressed by libtest, but rustdoc is then in turn capturing that output and dropping it.

The reason RUSTDOCFLAGS works is because rustdoc has an unstable --nocapture flag which changes that last step to not drop the output, but instead relay it back to cargo which relays it to the console.

It's not really clear to me why rustdoc chose to implement --nocapture as a separate arg instead of looking at --test-args.

I'm not sure if this issue should remain open, as I think this is mostly a rustdoc issue. There is some design/UX to explore, though.

[RalfJung]

Arguments passed after -- to cargo are passed by cargo to rustdoc via --test-args.

Ah, that is the part that I missed. So rustdoc internally also uses the libtest harness to drive its running of the doctests, and passes the --test-args to that library?
That makes it impossible to pass an argument directly to rustdoc, which is a bit unfortunate.

However, this does not explain why RUSTDOCFLAGS="-Zunstable-options --nocapture" cargo test --doc -- --nocapture says that the option was given more than once, this it would be passed once to rustdoc and once to the libtest runner inside rustdoc, but not twice to the same component.

I'm not sure if this issue should remain open, as I think this is mostly a rustdoc issue. There is some design/UX to explore, though.

Basically, rustdoc should realize when --nocapture is passed to its libtest component, and also adjust its own behavior accordingly? I agree that would make most sense and will bring this up with rustdoc.

[ehuss]

That makes it impossible to pass an argument directly to rustdoc, which is a bit unfortunate.

Well, as you observed, you can use RUSTDOCFLAGS to pass directly.

this does not explain why … option was given more than once

--nocapture passed to rustdoc itself also forwards it to the test harness here. That essentially results in passing --nocapture --nocapture to the test harness.

Basically, rustdoc should realize when --nocapture is passed to its libtest component, and also adjust its own behavior accordingly? I agree that would make most sense and will bring this up with rustdoc.

Yea, I would think changing it by removing the --nocapture flag from rustdoc, and changing it to something like:

diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs
index ab72f4a3f50..84ce5b4f9af 100644
--- a/src/librustdoc/doctest.rs
+++ b/src/librustdoc/doctest.rs
@@ -110,7 +110,6 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
     };

     let test_args = options.test_args.clone();
-    let nocapture = options.nocapture;
     let externs = options.externs.clone();
     let json_unused_externs = options.json_unused_externs;

@@ -164,7 +163,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
             })
         })?;

-    run_tests(test_args, nocapture, tests);
+    run_tests(test_args, tests);

     // Collect and warn about unused externs, but only if we've gotten
     // reports for each doctest
@@ -207,15 +206,8 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
     Ok(())
 }

-pub(crate) fn run_tests(
-    mut test_args: Vec<String>,
-    nocapture: bool,
-    tests: Vec<test::TestDescAndFn>,
-) {
+pub(crate) fn run_tests(mut test_args: Vec<String>, tests: Vec<test::TestDescAndFn>) {
     test_args.insert(0, "rustdoctest".to_string());
-    if nocapture {
-        test_args.push("--nocapture".to_string());
-    }
     test::test_main(&test_args, tests, None);
 }

@@ -467,7 +459,8 @@ fn drop(&mut self) {
         cmd.current_dir(run_directory);
     }

-    let result = if rustdoc_options.nocapture {
+    let nocapture = rustdoc_options.test_args.iter().any(|arg| arg == "--nocapture");
+    let result = if nocapture {
         cmd.status().map(|status| process::Output {
             status,
             stdout: Vec::new(),
diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs
index 0b557ef244e..b3976b659c2 100644
--- a/src/librustdoc/markdown.rs
+++ b/src/librustdoc/markdown.rs
@@ -146,6 +146,6 @@ pub(crate) fn test(options: Options) -> Result<(), String> {

     find_testable_code(&input_str, &mut collector, codes, options.enable_per_target_ignores, None);

-    crate::doctest::run_tests(options.test_args, options.nocapture, collector.tests);
+    crate::doctest::run_tests(options.test_args, collector.tests);
     Ok(())
 }

[ehuss]

I'm going to close as I believe this should be fixed on the rustdoc side. Closing in favor of rust-lang/rust#98550.