Skip to content

feat(doc test): add missing profile -C options to doc tests#17018

Open
QrkenBananen wants to merge 7 commits into
rust-lang:masterfrom
QrkenBananen:iss6570_doctest_profile
Open

feat(doc test): add missing profile -C options to doc tests#17018
QrkenBananen wants to merge 7 commits into
rust-lang:masterfrom
QrkenBananen:iss6570_doctest_profile

Conversation

@QrkenBananen
Copy link
Copy Markdown

What does this PR try to resolve?

Several -C profile options are missing in doc tests. such as opt-level and debug-assert. This extracts the logic from the regular test's build_base_args into separate functions and calls those from doc test argument collection.

Fixes #6570

How to test and review this PR?

Tests have been added that checks if cargo passes the new arguments to rustdoc.

"&Rc<[T]>" doesn't implement IntoIterator, but it can be changed to either
"&*Rc<[T]>" or "Rc<[T]>.as_ref()" to allow calling IntoIterator. And the
second option seems nicer.
…f &[T]

This allows the method to take more types of input, without forcing
users to collecting the arguments into a Vec.

the method is using AsRef to call OsStr's to_os_string() method, as it
actually requires OsStrings. So Into<OsString> seems more clear of the
method's requirements.

This also have the added benefit that Iterators of owned OsStrings can
use the OsString without allocating a new one.
@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler Command-test labels May 20, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 20, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 20, 2026

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ehuss, @epage, @weihanglo
  • @ehuss, @epage, @weihanglo expanded to ehuss, epage, weihanglo
  • Random selection from ehuss, epage, weihanglo

@QrkenBananen QrkenBananen marked this pull request as draft May 20, 2026 12:12
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 20, 2026
@QrkenBananen QrkenBananen force-pushed the iss6570_doctest_profile branch from 0500dd0 to 7504c58 Compare May 20, 2026 12:42
@QrkenBananen QrkenBananen force-pushed the iss6570_doctest_profile branch 2 times, most recently from 863565d to eab0a2b Compare May 20, 2026 14:02
Not adding tests for `-C linker` and `-C panic` as these
are already covered by `test::panic_abort_doc_tests` and
`cross_compile::doctest_xcompile_linker` respectively.
Remove `-C panic` and `-C codegen-linker` from `cargo_test.rs` so the
arguments are not passed twice.
@QrkenBananen QrkenBananen force-pushed the iss6570_doctest_profile branch from eab0a2b to 427b351 Compare May 20, 2026 15:57
@QrkenBananen
Copy link
Copy Markdown
Author

I changed the ProcessBuilder::args to use IntoIterator to avoid needing the functions to return Vecs that are immediately dropped in the simple cases. But since that requires bumping the version on cargo-utils, maybe it's worthwhile to update similar ProcessBuilder methods at the same time, or maybe it's better not to change the method, and just return Vecs and arrays?

@QrkenBananen QrkenBananen marked this pull request as ready for review May 20, 2026 16:41
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-build-execution Area: anything dealing with executing the compiler Command-test S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Doctests do not set profile codegen options

3 participants