rustdoc: fix 98690 Panic if invalid path for -Z persist-doctests

[pinkforest]

Closes #98690 for rustdoc panic

I changed this to do eprintln and orderly panic instead of unwrap doing unhandled panic

~/gg/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc --test -Z unstable-options --persist-doctests /tmp/foobar main.rs
Couldn't create directory for doctest executables: Permission denied (os error 13)

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @CraftSpider (or someone else) soon.

Please see the contribution instructions for more information.

[pinkforest]

r? @GuillaumeGomez

[GuillaumeGomez]

Can you add a rustdoc-ui test please?

[GuillaumeGomez]

Hum actually, I'm not sure if it's possible to check this one in CI... Maybe you have an idea @notriddle @camelid ?

[pinkforest]

Yeah I tried to find CLI tests but there didn't seem anything to plug into.. would have to come up with something I think on another PR?

My first PR sorry.. 🦄

[GuillaumeGomez]

Don't worry, if it requires developing something new, we will just approve this PR and do it in another PR. I'm just checking I didn't miss something.

[pinkforest]

I tried to add in src/test/rustdoc/isue-98690.rs - however CI has access to / so I am wondering how I can simulate invalid path here as std::fs::create_dir_all(&path) on ubuntu-latest is very forgiving that regard

// compile-flags: --persist-doctests /cannot_write_here -Z unstable-options

#![crate_name = "foo"]

//! ```
//! use foo::dummy;
//! dummy();
//! ```

I am not even sure if the above would catch runtime panic and I cannot check the result of eprintln or not sure about negative tests

However it does catch non-panic exit(1) code say if I would have invalid arguments e.g. adding extra -hfff to cli:

---- [rustdoc] src/test/rustdoc/issue-98690.rs stdout ----

error: rustdoc failed!
status: exit status: 1
command: "/home/foobar/gg/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "-L" "/home/foobar/gg/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-L" "/home/foobar/gg/rust/build/x86_64-unknown-linux-gnu/test/rustdoc/issue-foo/auxiliary" "-o" "/home/foobar/gg/rust/build/x86_64-unknown-linux-gnu/test/rustdoc/issue-foo" "--deny" "warnings" "/home/foobar/gg/rust/src/test/rustdoc/issue-foo.rs" "--persist-doctests" "/cannot_write_here" "-Z" "unstable-options" "-hfff"
stdout: none
--- stderr -------------------------------
error: Unrecognized option: 'f'
------------------------------------------

The test failures on that fn were relying on the same method I was using to roll exit so I followed that without using exit codes

[GuillaumeGomez]

Oh that's because you need to put it into rustdoc-ui, not rustdoc. Then you need to add an .stderr alongside your .rs where the output will be.

[pinkforest]

Ah that's helpful.. I think /../../ will work in CI to simulate invalid path:

---- [ui] src/test/rustdoc-ui/issue-98690.rs stdout ----

error: Error: expected failure status (Some(1)) but received status Some(101).
status: exit status: 101
command: "/home/foobar/gg/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "/home/foobar/gg/rust/src/test/rustdoc-ui/issue-foo.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Cstrip=debuginfo" "-o" "/home/foobar/gg/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-ui/issue-foo" "-Cdebuginfo=0" "-Lnative=/home/foobar/gg/rust/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--test" "--persist-doctests" "/../../" "-Z" "unstable-options" "-L" "/home/foobar/gg/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-ui/issue-foo/auxiliary"
stdout: none
--- stderr -------------------------------
Couldn't create directory for doctest executables: Permission denied (os error 13)
------------------------------------------

rust/src/test/rustdoc-ui$ cat issue-68690.stderr

Couldn't create directory for doctest executables: Permission denied (os error 13)

I updated stderr but it is expecting exit status code 1 if I intepret the above correct

However this would be different to rest of the test errors if I just exit the whole program

Is there a way to put exit code 101 into test data as expected exit condition or should I exit with 1 code ?

EDIT: // failure-status: 101 - gotcha!

[pinkforest]

Added a test :) this will now test for the error out - Thanks for the help 💜 @GuillaumeGomez

[GuillaumeGomez]

I didn't even have time to answer you. Great work and thanks a lot for this! (It allowed me to discover few things on the tests too 😆 )

@bors r+ rollup

[bors]

📌 Commit 29e0e14 has been approved by GuillaumeGomez

[matthiaskrgr]

failed in a rollup @bors r-
https://github.com/rust-lang-ci/rust/runs/7135955038?check_suite_focus=true#step:25:31692
#98724

[GuillaumeGomez]

Let's go again!

@bors r+ rollup

[bors]

📌 Commit 29e0e14 has been approved by GuillaumeGomez

[notriddle]

@bors r-

Failed in rollup #98744

No idea why the test harness comment failed. only-linux should have worked.

[pinkforest]

The only other test that has only-windows it's the first header

I tried to briefly look for reason why it gets ignored from here:
https://github.com/Manishearth/compiletest-rs/blob/master/src/header.rs

It doesn't seem (by reading fn header_iter) that header placement should have any issue though

[pinkforest]

Ok it looks like rollup picked up the wrong commit thanks @ehuss and @jyn514 for the help
Bors says this:
Commit https://github.com/rust-lang/rust/commit/29e0e148d9336bd554a4ef400f628f4a21413349 has been approved by GuillaumeGomez

EDIT: Bors picked up the wrong commit for reason or another
EDIT.2: Solution may be to re-open the PR

[ehuss]

@bors r=GuillaumeGomez rollup

[bors]

📌 Commit 6565509 has been approved by GuillaumeGomez

[minxuanz]

Will this test fail if tested with root ?

[GuillaumeGomez]

Do you have an example in mind?

[minxuanz]

// compile-flags: --test --persist-doctests /../../ -Z unstable-options
// failure-status: 101
// only-linux

#![crate_name = "foo"]

//! ```rust
//! use foo::dummy;
//! dummy();
//! ```

root user have permission to create dir in /../../ , stderr is different

[GuillaumeGomez]

That sounds like the expected behaviour, no?