-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Rename tests/rustdoc into tests/rustdoc-html
#150645
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
base: main
Are you sure you want to change the base?
Rename tests/rustdoc into tests/rustdoc-html
#150645
Conversation
This comment has been minimized.
This comment has been minimized.
e2f1bcc to
29e47c2
Compare
This comment has been minimized.
This comment has been minimized.
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.
Could you please also update the dev-guide in this PR (in a separate commit is fine) which is a subtree in this repo, namely in src/doc/rustc-dev-guide/.
E.g., renaming src/doc/rustc-dev-guide/src/rustdoc-internals/rustdoc-test-suite.md to src/doc/rustc-dev-guide/src/rustdoc-internals/rustdoc-html-test-suite.md & updating chapter name, link texts etc. Highly appreciated! :)
|
Sure! |
29e47c2 to
ccbf24e
Compare
|
The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes. |
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.
Two more things, then r=me unless you'd like to get reviews from others, too.
ccbf24e to
2254355
Compare
|
|
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.
As you can see from the test expectation changes, running ./x test librustdoc rustdoc no longer tests tests/rustdoc (since it no longer exists).
You should update the expression from rustdoc to rustdoc-html, rename this file and rebless the snap. Similarly for the other test file.
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.
(If we ever introduce rustdoc as an alias for all rustdoc-* test suites, we can change the rustdoc-html back to rustdoc in these two tests (or just keep them and add new ones))
| [build] rustc 0 <host> -> RustdocGUITest 1 <host> | ||
| [test] rustdoc-gui 1 <host> | ||
| [test] compiletest-incremental 1 <host> | ||
| [build] rustc 1 <host> -> rustc 2 <host> |
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.
Hmm, I'm slightly worried about this disappearing... I don't see why that has happened (similarly for the other tests in this file). I haven't looked closely at these tests & I'm not super familar with bootstrap's innards, so I might be missing sth.
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.
Let's wait for @Kobzol to have a look to ensure I didn't break something.
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.
I suspect this is because x test rustdoc also tries to run unit tests for src/tools/rustdoc, despite that being a stub crate with no tests in it.
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.
Actually, looking at the results of running x test src/tools/rustdoc on main, I have no idea what's going on. 🤷♂️
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.
Ah, x test src/tools/rustdoc is effectively treated as an alias for x test src/librustdoc, so it's running the unit tests and doctests for src/librustdoc.
This comment has been minimized.
This comment has been minimized.
2254355 to
324ccf0
Compare
| (x_test_librustdoc_rustdoc, "test librustdoc rustdoc"), | ||
| (x_test_rustdoc, "test rustdoc"), |
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.
IMO we should leave these two lines untouched, and instead add a new line for (x_test_rustdoc_html, "test rustdoc-html").
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.
The primary purpose of these existing lines is to capture the weird aliasing behaviour of src/librustdoc and src/tools/rustdoc, so it’s expected that renaming tests/rustdoc will affect the snapshots.
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.
Changes LGTM (with #150645 (comment) changed).
|
☔ The latest upstream changes (presumably #150640) made this pull request unmergeable. Please resolve the merge conflicts. |
|
☔ The latest upstream changes made this pull request unmergeable. Please resolve the merge conflicts. |
As discussed on zulip, renaming this testsuite better reflects what they actually test. Later on, it could also allow us to add an alias to run all rustdoc tests at once.
r? @camelid