-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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.
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
Improve rustdoc gui tester #85191
Improve rustdoc gui tester #85191
Conversation
Actually, considering it's a simple enough PR, might be interesting to put @CraftSpider on it. :) r? @CraftSpider |
src/bootstrap/test.rs
Outdated
let _ = fs::remove_dir_all(&out_dir); | ||
|
||
// We generate docs for the libraries present in the rustdoc-gui's src folder. | ||
let libs_dir = Path::new("src/test/rustdoc-gui/src"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be joined onto builder.config.src
, otherwise this won't work when invoked from a different working directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, I did it in #84586 but that's definitely a good idea to have it ahead!
src/bootstrap/test.rs
Outdated
} | ||
} | ||
|
||
// We now run GUI tests. | ||
let mut command = Command::new(&nodejs); | ||
command | ||
.arg("src/tools/rustdoc-gui/tester.js") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this probably also needs to be builder.config.src.join, as well as the test-folder here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely, all the "src" paths need it. Like I said, I did it in #84586 but didn't think about doing it here. Thanks for the reminder!
Modulo changed working directory support, this seems fine to me. I didn't review anything beyond bootstrap. |
@Mark-Simulacrum Updated. The other changes are simply taking into account the fact that the doc directory changed and update one test to last master version. |
@bors r+ rollup |
📌 Commit 6b94990 has been approved by |
Rollup of 8 pull requests Successful merges: - rust-lang#83501 (rustdoc: Add unstable CLI option to show basic type layout information) - rust-lang#85018 (shrinking the deprecated method span) - rust-lang#85124 (rustdoc: remove explicit boolean comparisons.) - rust-lang#85136 (Change param name (k to key and v to value) in std::env module) - rust-lang#85162 (Fix typo in variable name) - rust-lang#85187 (Use .name_str() to format primitive types in error messages) - rust-lang#85191 (Improve rustdoc gui tester) - rust-lang#85196 (Revert "Auto merge of rust-lang#84797 - richkadel:cover-unreachable-statements…) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
I cherry-picked the commit from #84834 (and modified it a bit). I also used this opportunity to update it to last version (forgot to update GUI test in #85074, really can't wait to make #84586 finally work).
cc @Mark-Simulacrum for the changes in bootstrap.
r? @jsha