Run rustdoc doctests relative to the workspace #9105
Conversation
|
r? @Eh2406 (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
CC @jyn514 This is currently blocked by having a rustdoc that supports that new flag. I did a "rustup update nightly" locally, but either its not in there yet, or cargo does not run the tests with nightly? Not sure… How can I control this? |
|
Thanks! Unfortunately due to the use of |
|
☔ The latest upstream changes (presumably #9122) made this pull request unmergeable. Please resolve the merge conflicts. |
ehuss
added
S-waiting-on-author
Swatinem
force-pushed
the
rustdoc-run-cwd
branch
4 times, most recently
from
58cc481 to
c4667b2
Compare
|
I updated the code to be compatible with both stable and nightly Rust but adding a few more conditions to the testsuite. I remember @jyn514 said there is no problem with stabilizing this option in rustdoc immediately as long as we validate that it actually does its job well. Due to the fact that cargo compiles/runs its testsuite on stable as well, I do think this needs to ride the trains to be turned on in cargo? |
Normally nightly tests are skipped when running with a stable toolchain I think. It seems strange to delay this just for the test suite. |
src/cargo/ops/cargo_test.rs
Outdated
| p.arg("--crate-name").arg(&unit.target.crate_name()); | ||
| p.arg("--test"); | ||
|
|
||
| if nightly_features_allowed() { |
There was a problem hiding this comment.
☝️ @jyn514 This is what I mean; removing this, every test that does a doctest will fail when run against stable rustc (up until it rides the trains)
There was a problem hiding this comment.
I don't understand the problem; why do you want to remove if nightly_features_allowed? That seems like the right check.
If it helps, cargo always runs against the same toolchain version of rustc; if cargo is nightly, rustc will be nightly too.
There was a problem hiding this comment.
I mean, this should get into stable at some point…
There was a problem hiding this comment.
Oh - that has to wait for rustdoc to stabilize the option. The plan is
- add the nightly feature in rustdoc
- pass it automatically in nightly cargo (this PR)
- stabilize the feature in rustdoc
- pass the flag unconditionally in cargo
There was a problem hiding this comment.
I think that this should probably be gated by a specific -Z flag in Cargo rather than simply unconditionally on the nightly channel
There was a problem hiding this comment.
That'll also make the impact in the tests less because the new changes will have to be opted-into instead of automatically happening on nightly.
There was a problem hiding this comment.
👍 fair enough! Rebased and added an unstable option that controls this behavior. That should be well enough for my purposes.
|
☔ The latest upstream changes (presumably #8640) made this pull request unmergeable. Please resolve the merge conflicts. |
Swatinem
force-pushed
the
rustdoc-run-cwd
branch
from
c4667b2 to
b1a8cce
Compare
Swatinem
force-pushed
the
rustdoc-run-cwd
branch
from
b1a8cce to
6433496
Compare
By doing so, rustdoc will also emit workspace-relative filenames for the doctests. This was first landed in rust-lang#8954 but later backed out in rust-lang#8996 because it changed the CWD of rustdoc test invocations. The second try relies on the new `--test-run-directory` rustdoc option which was added in rust-lang/rust#81264 to explicitly control the rustdoc test cwd. fixes rust-lang#8993
Swatinem
force-pushed
the
rustdoc-run-cwd
branch
from
6433496 to
b4c4028
Compare
|
@bors: r+ Looks good to me, thanks! |
|
📌 Commit b4c4028 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
Update cargo 11 commits in bf5a5d5e5d3ae842a63bfce6d070dfd438cf6070..572e201536dc2e4920346e28037b63c0f4d88b3c 2021-02-18 15:49:14 +0000 to 2021-02-24 16:51:20 +0000 - Pass the error message format to rustdoc (rust-lang/cargo#9128) - Fix test target_in_environment_contains_lower_case (rust-lang/cargo#9203) - Fix hang on broken stderr. (rust-lang/cargo#9201) - Make it more clear which module is being tested when running cargo test (rust-lang/cargo#9195) - Updates to edition handling. (rust-lang/cargo#9184) - Add --cfg and --rustc-cfg flags to output compiler configuration (rust-lang/cargo#9002) - Run rustdoc doctests relative to the workspace (rust-lang/cargo#9105) - Add support for [env] section in .cargo/config.toml (rust-lang/cargo#9175) - Add schema field and `features2` to the index. (rust-lang/cargo#9161) - Document the default location where cargo install emitting build artifacts (rust-lang/cargo#9189) - Do not exit prematurely if anything failed installing. (rust-lang/cargo#9185)
By doing so, rustdoc will also emit workspace-relative filenames for the doctests.
This was first landed in #8954 but later backed out in #8996 because it changed the CWD of rustdoc test invocations.
The second try relies on the new
--test-run-directoryrustdoc option which was added in rust-lang/rust#81264 to explicitly control the rustdoc test cwd.fixes #8993