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

add lint against unit tests in doctests #11872

Merged
merged 1 commit into from Nov 30, 2023

Conversation

llogiq
Copy link
Contributor

@llogiq llogiq commented Nov 26, 2023

During RustLab, Alice Ryhl brought to my attention that the Andoid team stumbled over the fact that if one attempts to write a unit test within a doctest, it will be summarily ignored. So this lint should help people wondering why their tests won't run.


changelog: New lint: [test_attr_in_doctest]
#11872

@rustbot
Copy link
Collaborator

rustbot commented Nov 26, 2023

r? @xFrednet

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 26, 2023
@llogiq llogiq force-pushed the test-attr-in-doctest branch 2 times, most recently from 1b1347b to 5a93643 Compare November 26, 2023 06:49
@llogiq
Copy link
Contributor Author

llogiq commented Nov 26, 2023

@rustbot author

I want to extend the span to also cover the signature (otherwise it will only show the attr) and avoid limiting on no_run and compile_fail tests.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 26, 2023
@llogiq
Copy link
Contributor Author

llogiq commented Nov 26, 2023

@rustbot review

I've now decided to limit the span to the item ident, which ensures it'll be in the message without potentially taking lots of lines that the declaration may take. I'm also thinking about renaming the doc::needless_doctest_main submodule because it is no longer only for that lint.

@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 from the author. (Use `@rustbot ready` to update this status) labels Nov 26, 2023
Copy link
Member

@xFrednet xFrednet left a comment

Choose a reason for hiding this comment

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

LGTM, two small nits and then it should be good :D

clippy_lints/src/doc/needless_doctest_main.rs Outdated Show resolved Hide resolved
tests/ui/test_attr_in_doctest.rs Show resolved Hide resolved
@llogiq
Copy link
Contributor Author

llogiq commented Nov 28, 2023

@xFrednet thanks for the quick review! Your nits should be gone now.

@xFrednet
Copy link
Member

LGTM, thank you! Nice new lint!

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 30, 2023

📌 Commit 0ba9bf9 has been approved by xFrednet

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 30, 2023

⌛ Testing commit 0ba9bf9 with merge 665fd52...

@bors
Copy link
Collaborator

bors commented Nov 30, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 665fd52 to master...

@bors bors merged commit 665fd52 into rust-lang:master Nov 30, 2023
8 checks passed
@llogiq llogiq deleted the test-attr-in-doctest branch December 1, 2023 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

4 participants