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.

Sign up for GitHub

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

Jump to bottom

test: migrate rust_version and rustc* to snapbox #14177

Merged
merged 3 commits into from
Jul 2, 2024
Merged

test: migrate rust_version and rustc* to snapbox #14177

merged 3 commits into from
Jul 2, 2024

Conversation

eth3lbert
Copy link
Contributor

What does this PR try to resolve?

Part of #14039.

Migrate following to snapbox:

  • tests/testsuite/rust_version.rs
  • tests/testsuite/rustc.rs
  • tests/testsuite/rustc_info_cache.rs

@rustbot
Copy link
Collaborator

rustbot commented Jul 1, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 1, 2024
@eth3lbert eth3lbert force-pushed the snapbox-rust branch 2 times, most recently from 09adbaf to a8c5c3e Compare July 1, 2024 22:03
@weihanglo weihanglo mentioned this pull request Jul 1, 2024
Open
eth3lbert
eth3lbert commented Jul 1, 2024
View reviewed changes
tests/testsuite/rustc.rs
.run();
}

#[allow(deprecated)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with_stderr_does_not_contain is being used in this function.

tests/testsuite/rustc.rs
@@ -622,6 589,7 @@ fn rustc_with_other_profile() {
p.cargo("rustc --profile test").run();
}

#[allow(deprecated)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with_stderr_does_not_contain is being used in this function.

tests/testsuite/rustc_info_cache.rs
@@ -10,6 8,7 @@ const MISS: &str = "[..] rustc info cache miss[..]";
const HIT: &str = "[..]rustc info cache hit[..]";
const UPDATE: &str = "[..]updated rustc info cache[..]";

#[allow(deprecated)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with_stderr_does_not_contain is being used in this function.

tests/testsuite/rustc_info_cache.rs
@@ -105,6 104,7 @@ fn rustc_info_cache() {
.run();
}

#[allow(deprecated)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with_stderr_does_not_contain is being used in this function.

tests/testsuite/rustc_info_cache.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the assertion that shows a pair of contain with does_not_contain, I'm inclined to keep it as is.

Copy link
Member

Choose a reason for hiding this comment

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

How bad is it if we compare the full output? I think we may need to add a comment for each cargo invocation for the expected hit/miss/update output.

I do want to kill does_not_contain as it is even more unreliable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! I'll give it a shot and see what results we can get.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, it seems like there are extra log messages related to rustup.

Copy link
Member

Choose a reason for hiding this comment

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

That is terribly bad. Let's keep the old tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done reverting!

weihanglo
weihanglo reviewed Jul 2, 2024
View reviewed changes
tests/testsuite/rustc.rs
.with_stdout_contains("target_vendor=\"unknown\"")
.with_stdout_contains("target_os=\"linux\"")
.with_stdout_contains("unix")
.with_stdout_data(str![[r#"
Copy link
Member

Choose a reason for hiding this comment

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

The order is not so importnat, should we use .unordered() and just one ... instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, this also works and make sense to me!

tests/testsuite/rustc_info_cache.rs
Copy link
Member

Choose a reason for hiding this comment

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

How bad is it if we compare the full output? I think we may need to add a comment for each cargo invocation for the expected hit/miss/update output.

I do want to kill does_not_contain as it is even more unreliable.

@weihanglo
Copy link
Member

Looks great. Thank you for the migration.

@bors r

@bors
Copy link
Collaborator

bors commented Jul 2, 2024

📌 Commit f51a580 has been approved by weihanglo

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 Jul 2, 2024
@bors
Copy link
Collaborator

bors commented Jul 2, 2024

⌛ Testing commit f51a580 with merge a515d46...

@bors
Copy link
Collaborator

bors commented Jul 2, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing a515d46 to master...

@bors bors merged commit a515d46 into rust-lang:master Jul 2, 2024
22 checks passed
@eth3lbert eth3lbert deleted the snapbox-rust branch July 2, 2024 21:26
@weihanglo weihanglo mentioned this pull request Jul 3, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 3, 2024
@bors
Auto merge of rust-lang#127258 - weihanglo:update-cargo, r=weihanglo
d68fe4e 
Update cargo

23 commits in 4ed7bee47f7dd4416b36fada1909e9a62c546246..a515d463427b3912ec0365d106791f88c1c14e1b
2024-06-25 16:28:22  0000 to 2024-07-02 20:53:36  0000
- test: migrate rust_version and rustc* to snapbox (rust-lang/cargo#14177)
- test: mirgate fix* and future_incompat_report to snapbox (rust-lang/cargo#14173)
- test：migrate `edition/error` to snapbox (rust-lang/cargo#14175)
- chore(deps): update compatible (rust-lang/cargo#14174)
- refactor(source): Clean up after PathSource/RecursivePathSource split (rust-lang/cargo#14169)
- test: Migrate some files to snapbox (rust-lang/cargo#14132)
- test:  fix several assertions (rust-lang/cargo#14167)
- test: replace glob with explicit unordered calls (rust-lang/cargo#14166)
- Make it clear that `CARGO_CFG_TARGET_FAMILY` is multi-valued (rust-lang/cargo#14165)
- Document `CARGO_CFG_TARGET_ABI` (rust-lang/cargo#14164)
- test: Migrate git to snapbox (rust-lang/cargo#14159)
- test: migrate some files to snapbox (rust-lang/cargo#14158)
- test: migrate registry and registry_auth to snapbox (rust-lang/cargo#14149)
- gix: remove `revision` feature from cargo (rust-lang/cargo#14160)
- test: migrate package* and publish* to snapbox (rust-lang/cargo#14130)
- More `update --breaking` tests (rust-lang/cargo#14049)
- test: migrate clean to snapbox (rust-lang/cargo#14096)
- Allow `unexpected_builtin_cfgs` lint in `user_specific_cfgs` test (rust-lang/cargo#14153)
- test: migrate search, source_replacement and standard_lib to snapbox (rust-lang/cargo#14151)
- Docs: Update config summary to include missing keys. (rust-lang/cargo#14145)
- test: migrate `dep_info/diagnostics/direct_minimal_versions` to snapbox (rust-lang/cargo#14143)
- Docs: Remove duplicate `strip` section. (rust-lang/cargo#14146)
- Docs: Fix curly quotes in config docs. (rust-lang/cargo#14144)
@rustbot rustbot added this to the 1.81.0 milestone Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@weihanglo weihanglo

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.81.0
Development

Successfully merging this pull request may close these issues.
4 participants
@eth3lbert @rustbot @weihanglo @bors