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

cargo-test-support: Add features to the default Cargo.toml file #12997

Merged
merged 2 commits into from
Nov 22, 2023

Conversation

Rustin170506
Copy link
Member

@Rustin170506 Rustin170506 commented Nov 18, 2023

What does this PR try to resolve?

Found in Rustin170506/cargo-information#32 (comment).

I think we need to add features to the default Cargo.toml file.

How should we test and review this PR?

image

Additional information

Do you which Cargo command can cover this case? I want to add a unit test for it.
It seems I can use cargo metadata to cover it. I can add a dep to the package and try to check the output JSON.

@rustbot
Copy link
Collaborator

rustbot commented Nov 18, 2023

r? @epage

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

@rustbot rustbot added A-testing-cargo-itself Area: cargo's tests S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 18, 2023
@Rustin170506 Rustin170506 marked this pull request as draft November 18, 2023 15:19
@Rustin170506 Rustin170506 force-pushed the rustin-patch-cargo-test-support branch 2 times, most recently from 95626ed to ecf8671 Compare November 19, 2023 13:58
@Rustin170506 Rustin170506 marked this pull request as ready for review November 19, 2023 14:02
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Copy link
Member Author

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

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

🔢 Self-check

@@ -86,6 +86,185 @@ fn cargo_metadata_simple() {
.run();
}

#[cargo_test]
fn cargo_metadata_with_registered_dep() {
Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure if this test case is necessary. But we don't have a test for feature names. So I added it. Please let me know if we have a better way to cover it.

Copy link
Contributor

Choose a reason for hiding this comment

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

We have tests for features, just not from transitive dependencies. Unsure if this is providing enough value.

Copy link
Member Author

Choose a reason for hiding this comment

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

I dropped it. It is not a test for the real case, so it might not be worth adding it.

@Rustin170506 Rustin170506 changed the title Add features to the default Cargo.toml file cargo-test-support: Add features to the default Cargo.toml file Nov 19, 2023
@epage
Copy link
Contributor

epage commented Nov 22, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Nov 22, 2023

📌 Commit 504c4b2 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 22, 2023
@bors
Copy link
Contributor

bors commented Nov 22, 2023

⌛ Testing commit 504c4b2 with merge 65d0eb5...

@bors
Copy link
Contributor

bors commented Nov 22, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 65d0eb5 to master...

@bors bors merged commit 65d0eb5 into rust-lang:master Nov 22, 2023
22 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 25, 2023
Update cargo

7 commits in 71cd3a926f0cf41eeaf9f2a7f2194b2aff85b0f6..9b13310ca596020a737aaa47daa4ed9ff8898a2f
2023-11-20 15:30:57 +0000 to 2023-11-24 16:20:51 +0000
- feat: Add `CARGO_RUSTC_CURRENT_DIR` (unstable) (rust-lang/cargo#12996)
- Exited with hard error when custom build file no existence or not in package (rust-lang/cargo#12995)
- try running on windows (rust-lang/cargo#13042)
- refactor(toml): Better abstract inheritance details (rust-lang/cargo#13021)
- cargo-test-support: Add features to the default Cargo.toml file (rust-lang/cargo#12997)
- Migrate rustfix to the cargo repo (rust-lang/cargo#13005)
- typo: rusc -&gt; rustc (rust-lang/cargo#13019)

---

This also removes the check to ensure that `rustfix` between

* src/tools/cargo
* src/tools/compiletest

has the same version,
since `rust-lang/rustfix` has migrated to under `rust-lang/cargo`.

r? ghost
@rustbot rustbot added this to the 1.76.0 milestone Nov 25, 2023
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Nov 27, 2023
Update cargo

7 commits in 71cd3a926f0cf41eeaf9f2a7f2194b2aff85b0f6..9b13310ca596020a737aaa47daa4ed9ff8898a2f
2023-11-20 15:30:57 +0000 to 2023-11-24 16:20:51 +0000
- feat: Add `CARGO_RUSTC_CURRENT_DIR` (unstable) (rust-lang/cargo#12996)
- Exited with hard error when custom build file no existence or not in package (rust-lang/cargo#12995)
- try running on windows (rust-lang/cargo#13042)
- refactor(toml): Better abstract inheritance details (rust-lang/cargo#13021)
- cargo-test-support: Add features to the default Cargo.toml file (rust-lang/cargo#12997)
- Migrate rustfix to the cargo repo (rust-lang/cargo#13005)
- typo: rusc -&gt; rustc (rust-lang/cargo#13019)

---

This also removes the check to ensure that `rustfix` between

* src/tools/cargo
* src/tools/compiletest

has the same version,
since `rust-lang/rustfix` has migrated to under `rust-lang/cargo`.

r? ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing-cargo-itself Area: cargo's tests 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.

4 participants