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

Clarify the function of the test options #13236

Merged
merged 2 commits into from
Jan 12, 2024
Merged

Conversation

heisen-li
Copy link
Contributor

What does this PR try to resolve?

Make the description of test options clearer.

from #10936

@rustbot
Copy link
Collaborator

rustbot commented Jan 2, 2024

r? @weihanglo

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

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. Command-test S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 2, 2024
Comment on lines 40 to 41
"Test only this package's library unit tests",
"Test only the specified binary",
"Test all binaries",
"Test only the specified binary's unit tests",
Copy link
Contributor

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.

Copy link
Member

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?”

"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",
Copy link
Contributor

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

@weihanglo
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 4, 2024
@heisen-li
Copy link
Contributor Author

@weihanglo I understand that epage should mean to remove unit tests from the option description. Would you take a look?

Copy link
Member

@weihanglo weihanglo left a 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?

@epage
Copy link
Contributor

epage commented Jan 12, 2024

@bors r

@bors
Copy link
Collaborator

bors commented Jan 12, 2024

📌 Commit 5023f20 has been approved by epage

It is now in the queue for this repository.

@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-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels Jan 12, 2024
@bors
Copy link
Collaborator

bors commented Jan 12, 2024

⌛ Testing commit 5023f20 with merge 92395d9...

@bors
Copy link
Collaborator

bors commented Jan 12, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing 92395d9 to master...

@bors bors merged commit 92395d9 into rust-lang:master Jan 12, 2024
20 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 17, 2024
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
@ehuss ehuss added this to the 1.77.0 milestone Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. Command-test S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants