Skip to content
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

Detect invalid rustdoc test commands #80617

Merged

Conversation

GuillaumeGomez
Copy link
Member

Fixes #80570.

There are now errors displayed in case of bad command syntax:

---- [rustdoc] rustdoc/remove-url-from-headings.rs stdout ----

error: htmldocck failed!
status: exit code: 1
command: "/usr/bin/python" "/home/imperio/rust/rust/src/etc/htmldocck.py" "/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/test/rustdoc/remove-url-from-headings" "/home/imperio/rust/rust/src/test/rustdoc/remove-url-from-headings.rs"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
3: Invalid command: `!@has`, (try with `@!has`)
	// !@has - '//a[@href="http://a.a"]'

Encountered 1 errors

r? @camelid

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-doctests Area: Documentation tests, run by rustdoc labels Jan 2, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 2, 2021
Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't review the implementation since I'm not familiar with the code and not familiar with the regex style, but the output looks good. Do you want to try adding a failing assertion (like @has ... 'someRandomText') and check that the test fails just to make sure this didn't break anything? And you might to re-assign to someone else to review the implementation.

Thanks for fixing this!

print_err(
lineno,
line,
'Invalid command: `!@{0}{1}`, (try with `@!{1}`)'.format(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe say

Suggested change
'Invalid command: `!@{0}{1}`, (try with `@!{1}`)'.format(
'Invalid command: `!@{0}{1}`, (help: try with `@!{1}`)'.format(

to be consistent with rustc suggestion? Not a big deal though.

@GuillaumeGomez GuillaumeGomez force-pushed the detect-invalid-rustdoc-test-commands branch from c1d5a26 to da3eef6 Compare January 3, 2021 13:24
@GuillaumeGomez
Copy link
Member Author

Do you want to try adding a failing assertion (like @has ... 'someRandomText') and check that the test fails just to make sure this didn't break anything?

Automatically, it would require a lot of changes. So not sure if it's a good idea... However, just to confirm it works I did a small change and it failed as expected:

command: "/usr/bin/python" "/home/imperio/rust/rust/src/etc/htmldocck.py" "/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/test/rustdoc/generic-impl" "/home/imperio/rust/rust/src/test/rustdoc/generic-impl.rs"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
6: @has check failed
	`XPATH PATTERN` did not match
	// @has foo/struct.Bar.html '//h4' 'yolo'

@GuillaumeGomez
Copy link
Member Author

Also, I think @jyn514 was the last one to modify this file? Let's set him as reviewer then. :)

r? @jyn514

@rust-highfive rust-highfive assigned jyn514 and unassigned camelid Jan 3, 2021
@jyn514
Copy link
Member

jyn514 commented Jan 3, 2021

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Jan 3, 2021

📌 Commit da3eef6 has been approved by jyn514

@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 Jan 3, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 3, 2021
…laumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#80580 (Add suggestion for "ignore" doc code block)
 - rust-lang#80591 (remove allow(incomplete_features) from std)
 - rust-lang#80617 (Detect invalid rustdoc test commands)
 - rust-lang#80628 (reduce borrowing and (de)referencing around match patterns (clippy::match_ref_pats))
 - rust-lang#80646 (Clean up in `each_child_of_item`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fd0dbac into rust-lang:master Jan 3, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 3, 2021
@camelid
Copy link
Member

camelid commented Jan 3, 2021

Automatically, it would require a lot of changes. So not sure if it's a good idea... However, just to confirm it works I did a small change and it failed as expected:

Yes, I meant to do a manual test just to make sure it's still picking up the assertions :)

@GuillaumeGomez GuillaumeGomez deleted the detect-invalid-rustdoc-test-commands branch January 3, 2021 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-doctests Area: Documentation tests, run by rustdoc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

htmldocck: Don't silently ignore !@has comments
6 participants