Skip to content

Add test for --test-builder success path #148608

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
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add test for --test-builder success path #148608

wants to merge 3 commits into from
+66 −3

Conversation

@osamakader
Copy link
Contributor

@osamakader osamakader commented Nov 6, 2025 •
edited
Loading

Fixes #148586

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 6, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 6, 2025

jieyouxu is currently at their maximum review capacity.
They may take a while to respond.

@rustbot
Copy link
Collaborator

rustbot commented Nov 6, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
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

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@osamakader
Add test for success path
8075c98 
Adds missing test coverage for rustdoc's `--test-builder` option.

The existing test only covered the error case (non-executable builder). This PR
adds:
- A custom test builder that logs arguments and forwards to rustc
- A test verifying that `--test-builder` successfully invokes the custom
builder with rustc-style arguments
- Improved comments explaining both the error and success test scenarios

The test validates that custom builders can properly intercept and handle
doctest compilation.

Signed-off-by: Osama Abdelkader <[email protected]>
@jieyouxu
Copy link
Member

jieyouxu commented Nov 7, 2025

r? rustdoc

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Nov 7, 2025
@rustbot rustbot assigned GuillaumeGomez and unassigned jieyouxu Nov 7, 2025
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Nov 7, 2025

Much appreciated, thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 7, 2025

📌 Commit 8075c98 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@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 Nov 7, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 7, 2025
@GuillaumeGomez
Rollup merge of rust-lang#148608 - osamakader:doc-test-builder, r=Gui…
1f80593 
…llaumeGomez

Add test for --test-builder success path

Fixes rust-lang#148586
@GuillaumeGomez GuillaumeGomez mentioned this pull request Nov 7, 2025
Closed
bors added a commit that referenced this pull request Nov 7, 2025
@bors
Auto merge of #148642 - GuillaumeGomez:rollup-qdothm9, r=GuillaumeGomez
81bfb36 
Rollup of 5 pull requests

Successful merges:

 - #145656 (Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro)
 - #148204 (Modify contributor email entries in .mailmap)
 - #148556 (Fix suggestion for returning async closures)
 - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability)
 - #148608 (Add test for --test-builder success path)

r? `@ghost`
`@rustbot` modify labels: rollup
GuillaumeGomez
GuillaumeGomez reviewed Nov 7, 2025
View reviewed changes
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Nov 7, 2025

Failed in #148642.

I think it should only be run on linux.

@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 Nov 7, 2025
@osamakader
Copy link
Contributor Author

osamakader commented Nov 7, 2025

I'm trying right now to use bare_rustc so we compile for the host architecture even in cross build.

@rustbot rustbot 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 Nov 7, 2025
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Nov 8, 2025

Let's confirm your fix works.

@bors try jobs=test-various

rust-bors bot added a commit that referenced this pull request Nov 8, 2025
@rust-bors
Auto merge of #148608 - osamakader:doc-test-builder, r=<try>
1410cf9 
Add test for --test-builder success path

try-job: test-various
@rust-bors

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-bors
Copy link

rust-bors bot commented Nov 8, 2025

💔 Test for 1410cf9 failed: CI. Failed jobs:

@osamakader
Copy link
Contributor Author

osamakader commented Nov 8, 2025

@bors try jobs=test-various

@rust-bors
Copy link

rust-bors bot commented Nov 8, 2025

@osamakader: 🔑 Insufficient privileges: not in try users

@osamakader
Copy link
Contributor Author

osamakader commented Nov 8, 2025

@GuillaumeGomez Can you please re-trigger the test? I don't have privileges to do so.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Nov 8, 2025

Need to fix the new issue before that. 😉

@osamakader
Copy link
Contributor Author

osamakader commented Nov 8, 2025

@GuillaumeGomez Can you please re-try? thanks.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Nov 8, 2025

Here we go!

@bors try jobs=test-various

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors bot added a commit that referenced this pull request Nov 8, 2025
@rust-bors
Auto merge of #148608 - osamakader:doc-test-builder, r=<try>
7eefd77 
Add test for --test-builder success path

try-job: test-various
@rust-bors
Copy link

rust-bors bot commented Nov 8, 2025

☀️ Try build successful (CI)
Build commit: 7eefd77 (7eefd7758b1de3920d206290ad79e7f8d3e7d4bb, parent: fb23dd3c6b120f0d2e55e5f2c69a464df7b35fdf)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GuillaumeGomez GuillaumeGomez Awaiting requested review from GuillaumeGomez

Assignees

@GuillaumeGomez GuillaumeGomez

Labels

A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[rustdoc] Missing test for --test-builder option

6 participants

@osamakader @rustbot @rust-log-analyzer @jieyouxu @GuillaumeGomez @bors