-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Clarify the function of the test options #13236
Conversation
r? @weihanglo (rustbot has picked a reviewer for you, use r? to override) |
src/bin/cargo/commands/test.rs
Outdated
"Test only this package's library unit tests", | ||
"Test only the specified binary", | ||
"Test all binaries", | ||
"Test only the specified binary's unit tests", |
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.
For myself, I would prefer removing references to unit tests, rather than focusing more on unit/integration test terminology.
Its a bit of a misnomer to refer to them as unit/integration tests. The mechanism allows whitebox / blackbox testing and the user can put unit or integration tests in either location.
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.
Agree on Ed's argument. Mentioning unit tests is still confusing. “What does it mean by binary unit tests?”
src/bin/cargo/commands/test.rs
Outdated
"Test only the specified test target", | ||
"Test all test targets", | ||
"Test only the specified target's integration tests", | ||
"Test all test targets's integration tests and library unit tests", |
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 seems odd so I would want us to confirm it and whether it is expected behavior updating documenation
@rustbot author |
@weihanglo I understand that epage should mean to remove unit tests from the option description. Would you take a look? |
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 am okay with the change. @epage, do you have any other opinion?
@bors r+ |
☀️ Test successful - checks-actions |
Update cargo 10 commits in 84976cd699f4aea56cb3a90ce3eedeed9e20d5a5..1cff2ee6b92e0ad3f87c44b70b28f788b2528b3c 2024-01-12 15:55:43 +0000 to 2024-01-16 16:56:57 +0000 - doc: add a heading to highlight "How to find features enabled on dependencies" (rust-lang/cargo#13305) - fix(cargo-update): `--precise` accept arbitrary git revisions (rust-lang/cargo#13250) - Strip debuginfo when debuginfo is not requested (rust-lang/cargo#13257) - Update ahash dependency to 0.8.7 (rust-lang/cargo#13301) - docs: add more links to pkgid spec chapter (rust-lang/cargo#13298) - fix(metadata): Stabilize id format as PackageIDSpec (rust-lang/cargo#12914) - Introduce `-Zprecise-pre-release` unstable flag (rust-lang/cargo#13296) - Delete sentence about parentheses being unsupported in license (rust-lang/cargo#13292) - Add guidance on setting homepage in manifest (rust-lang/cargo#13293) - Clarify the function of the test options (rust-lang/cargo#13236) r? ghost
What does this PR try to resolve?
Make the description of test options clearer.
from #10936