test: migrate clean to snapbox by dieterplex · Pull Request #14096 · rust-lang/cargo

dieterplex · 2024-06-18T14:48:56Z

Part of #14039, migrating tests/testsuite/clean.rs to snapbox.

rustbot · 2024-06-18T14:49:01Z

rustbot has assigned @epage.
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

tests/testsuite/clean.rs

dieterplex · 2024-06-19T12:26:10Z

tests/testsuite/clean.rs

+            let obj = obj.unwrap().display().to_string();
+            expected.push_str(&format!(
+                "[REMOVING] {}\n",
+                obj.replace(p.root().to_str().unwrap(), "[ROOT]/foo")


Replacing root is not enough and there is only 1 of 2 hashes in filename being redacted. How about leave it as is ([..]) for now?

- [REMOVING] [ROOT]/foo/target/debug/deps/bar-966252632dc060f2.bar.cce207b4909ed527-cgu.0.rcgu.o + [REMOVING] [ROOT]/foo/target/debug/deps/bar-[HASH].bar.cce207b4909ed527-cgu.0.rcgu.o

Yeah fine we that. I don't think Cargo cares about codegen unit artifacts. What about making it conditional, something like this?

#[cfg(not(target_os = "macos"))] let expected = str![[r#" [REMOVING] [ROOT]/foo/target/debug/.fingerprint/bar-[HASH] [REMOVING] [ROOT]/foo/target/debug/deps/libbar-[HASH].rlib [REMOVING] [ROOT]/foo/target/debug/deps/bar-[HASH].d [REMOVING] [ROOT]/foo/target/debug/deps/libbar-[HASH].rmeta [REMOVED] [FILE_NUM] files, [FILE_SIZE]B total "#]]; #[cfg(target_os = "macos")] let expected = str![[r#" [REMOVING] [ROOT]/foo/target/debug/.fingerprint/bar-[HASH] [REMOVING] [ROOT]/foo/target/debug/deps/libbar-[HASH].rlib [REMOVING] [ROOT]/foo/target/debug/deps/bar-[HASH].d [REMOVING] [ROOT]/foo/target/debug/deps/libbar-[HASH].rmeta [REMOVING] [ROOT]/foo/target/debug/deps/bar-[HASH][..].o [REMOVED] [FILE_NUM] files, [FILE_SIZE]B total "#]];

looks promising, but is there only one .o file?

Nice catch. Could also just count the nubmer of bar-.*.o and push the same number of [REMOVING] [ROOT]/foo/target/debug/deps/bar-[HASH][..].o line.

tests/testsuite/alt_registry.rs

weihanglo · 2024-06-19T12:35:04Z

tests/testsuite/clean.rs

+            let obj = obj.unwrap().display().to_string();
+            expected.push_str(&format!(
+                "[REMOVING] {}\n",
+                obj.replace(p.root().to_str().unwrap(), "[ROOT]/foo")


Yeah fine we that. I don't think Cargo cares about codegen unit artifacts. What about making it conditional, something like this?

#[cfg(not(target_os = "macos"))] let expected = str![[r#" [REMOVING] [ROOT]/foo/target/debug/.fingerprint/bar-[HASH] [REMOVING] [ROOT]/foo/target/debug/deps/libbar-[HASH].rlib [REMOVING] [ROOT]/foo/target/debug/deps/bar-[HASH].d [REMOVING] [ROOT]/foo/target/debug/deps/libbar-[HASH].rmeta [REMOVED] [FILE_NUM] files, [FILE_SIZE]B total "#]]; #[cfg(target_os = "macos")] let expected = str![[r#" [REMOVING] [ROOT]/foo/target/debug/.fingerprint/bar-[HASH] [REMOVING] [ROOT]/foo/target/debug/deps/libbar-[HASH].rlib [REMOVING] [ROOT]/foo/target/debug/deps/bar-[HASH].d [REMOVING] [ROOT]/foo/target/debug/deps/libbar-[HASH].rmeta [REMOVING] [ROOT]/foo/target/debug/deps/bar-[HASH][..].o [REMOVED] [FILE_NUM] files, [FILE_SIZE]B total "#]];

weihanglo · 2024-06-20T02:39:05Z

tests/testsuite/clean.rs

    // Verify it didn't delete anything.
    let after = p.build_dir().ls_r();
    assert_eq!(before, after);
-    let expected = itertools::join(before.iter().map(|p| p.to_str().unwrap()), "\n");


https://github.com/rust-lang/cargo/actions/runs/9590054694/job/26444813493?pr=14096#step:11:4073

Paths on Windows didn't get normalized. It also happened here #14091 (comment).

Seems that expected string won't be normalized? https://docs.rs/snapbox/0.6.2/src/snapbox/assert/mod.rs.html#111-141

@epage, off the top of your head, what could be the cause?

--- Expected ++++ actual: stdout 1 1 | [ROOT]/foo/target 2 - [ROOT]/foo/target\.rustc_info.json 3 - [ROOT]/foo/target\CACHEDIR.TAG 4 - [ROOT]/foo/target\debug 5 - [ROOT]/foo/target\debug\.cargo-lock 6 - [ROOT]/foo/target\debug\.fingerprint 7 - [ROOT]/foo/target\debug\.fingerprint\bar-72b289f5cc241a07 8 - [ROOT]/foo/target\debug\.fingerprint\bar-72b289f5cc241a07\dep-lib-bar 9 - [ROOT]/foo/target\debug\.fingerprint\bar-72b289f5cc241a07\invoked.timestamp 10 - [ROOT]/foo/target\debug\.fingerprint\bar-72b289f5cc241a07\lib-bar 11 - [ROOT]/foo/target\debug\.fingerprint\bar-72b289f5cc241a07\lib-bar.json 12 - [ROOT]/foo/target\debug\.fingerprint\foo-ad5acdf1e6368b18 13 - [ROOT]/foo/target\debug\.fingerprint\foo-ad5acdf1e6368b18\dep-lib-foo 14 - [ROOT]/foo/target\debug\.fingerprint\foo-ad5acdf1e6368b18\invoked.timestamp 15 - [ROOT]/foo/target\debug\.fingerprint\foo-ad5acdf1e6368b18\lib-foo 16 - [ROOT]/foo/target\debug\.fingerprint\foo-ad5acdf1e6368b18\lib-foo.json 17 - [ROOT]/foo/target\debug\build 18 - [ROOT]/foo/target\debug\deps 19 - [ROOT]/foo/target\debug\deps\bar-72b289f5cc241a07.d 20 - [ROOT]/foo/target\debug\deps\foo-ad5acdf1e6368b18.d 21 - [ROOT]/foo/target\debug\deps\libbar-72b289f5cc241a07.rmeta 22 - [ROOT]/foo/target\debug\deps\libfoo-ad5acdf1e6368b18.rmeta 23 - [ROOT]/foo/target\debug\examples 24 - [ROOT]/foo/target\debug\incremental 2 + [ROOT]/foo/target/.rustc_info.json 3 + [ROOT]/foo/target/CACHEDIR.TAG 4 + [ROOT]/foo/target/debug/.cargo-lock 5 + [ROOT]/foo/target/debug/.fingerprint/bar-[HASH]/dep-lib-bar 6 + [ROOT]/foo/target/debug/.fingerprint/bar-[HASH]/invoked.timestamp 7 + [ROOT]/foo/target/debug/.fingerprint/bar-[HASH]/lib-bar 8 + [ROOT]/foo/target/debug/.fingerprint/bar-[HASH]/lib-bar.json 9 + [ROOT]/foo/target/debug/.fingerprint/bar-[HASH] 10 + [ROOT]/foo/target/debug/.fingerprint/foo-[HASH]/dep-lib-foo 11 + [ROOT]/foo/target/debug/.fingerprint/foo-[HASH]/invoked.timestamp 12 + [ROOT]/foo/target/debug/.fingerprint/foo-[HASH]/lib-foo 13 + [ROOT]/foo/target/debug/.fingerprint/foo-[HASH]/lib-foo.json 14 + [ROOT]/foo/target/debug/.fingerprint/foo-[HASH] 15 + [ROOT]/foo/target/debug/.fingerprint 16 + [ROOT]/foo/target/debug/build 17 + [ROOT]/foo/target/debug/deps/bar-[HASH].d 18 + [ROOT]/foo/target/debug/deps/foo-[HASH].d 19 + [ROOT]/foo/target/debug/deps/libbar-[HASH].rmeta 20 + [ROOT]/foo/target/debug/deps/libfoo-[HASH].rmeta 21 + [ROOT]/foo/target/debug/deps 22 + [ROOT]/foo/target/debug/examples 23 + [ROOT]/foo/target/debug/incremental 24 + [ROOT]/foo/target/debug

It seems cargo_test_support::paths::Path::ls_r() gives non-normalized paths, but actual output. Both are not normalized by calling out normalize_windows() in compare mod.

@dieterplex could you revert change in this specific test case, and put a #[allow(deprecated)], so that we can unblock the entire PR?

weihanglo · 2024-06-21T12:20:56Z

crates/cargo-test-support/src/compare.rs

@dieterplex could you extract this into a separate PR?
Other places like #14109 (comment) could also benefit from this before we fix the Windows path normalization issue. Thank you.

Sure, PR #14121.

Unresolved this since this uses #14121 and I'm concerned it might need to be reverted, depending on the answer to the question I posed.

See #14096 (comment). The PR description is updated as well #14121.

test: Auto-redact file number This is from <#14096 (comment)>. Although the number of files in `cargo package` is important, we have `validate_crate_contents` and `validate_upload_with_contents` that verify the exact contents. Redacting `Packaged` status should be fine.

weihanglo · 2024-06-26T20:18:42Z

Thanks a lot!

@bors r+

bors · 2024-06-26T20:18:44Z

📌 Commit 1794ce4 has been approved by weihanglo

It is now in the queue for this repository.

bors · 2024-06-26T20:19:52Z

⌛ Testing commit 1794ce4 with merge c81c32b...

bors · 2024-06-26T20:48:39Z

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

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 assigned epage Jun 18, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 18, 2024

dieterplex mentioned this pull request Jun 18, 2024

Port from bespoke assertions of snapbox #14039

Closed

weihanglo reviewed Jun 18, 2024

View reviewed changes

tests/testsuite/clean.rs Outdated Show resolved Hide resolved

tests/testsuite/clean.rs Outdated Show resolved Hide resolved

tests/testsuite/clean.rs Outdated Show resolved Hide resolved

eth3lbert mentioned this pull request Jun 18, 2024

test: migrate features and features(2|_namespaced) to snapbox #14100

Merged

dieterplex force-pushed the migrate-clean-snapbox branch from b278ca7 to a38cef2 Compare June 19, 2024 11:54

rustbot added the A-testing-cargo-itself Area: cargo's tests label Jun 19, 2024

dieterplex force-pushed the migrate-clean-snapbox branch from a38cef2 to e568ca6 Compare June 19, 2024 12:01

dieterplex commented Jun 19, 2024

View reviewed changes

weihanglo reviewed Jun 19, 2024

View reviewed changes

eth3lbert mentioned this pull request Jun 19, 2024

test: migrate path and paths to snapbox #14109

Merged

dieterplex force-pushed the migrate-clean-snapbox branch 2 times, most recently from dac398b to 57af1a1 Compare June 20, 2024 01:12

weihanglo reviewed Jun 20, 2024

View reviewed changes

weihanglo reviewed Jun 21, 2024

View reviewed changes

weihanglo mentioned this pull request Jun 21, 2024

test: Auto-redact file number #14121

Merged

weihanglo mentioned this pull request Jun 25, 2024

test: migrate some files to snapbox #14113

Merged

dieterplex added 2 commits June 26, 2024 17:28

test: migrate clean to snapbox

b3a9dec

test: Revert clean_dry_run for expected path not normalized

1794ce4

dieterplex force-pushed the migrate-clean-snapbox branch from 57af1a1 to 1794ce4 Compare June 26, 2024 09:42

weihanglo approved these changes Jun 26, 2024

View reviewed changes

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 Jun 26, 2024

bors merged commit c81c32b into rust-lang:master Jun 26, 2024

dieterplex deleted the migrate-clean-snapbox branch June 26, 2024 21:26

weihanglo mentioned this pull request Jul 3, 2024

Update cargo rust-lang/rust#127258

Merged

rustbot added this to the 1.81.0 milestone Jul 3, 2024

dieterplex mentioned this pull request Jul 16, 2024

Migrate global_cache_tracker snapbox #14244

Merged

Conversation

dieterplex commented Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 18, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dieterplex Jun 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dieterplex Jun 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

epage Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weihanglo commented Jun 26, 2024

Uh oh!

bors commented Jun 26, 2024

Uh oh!

bors commented Jun 26, 2024

Uh oh!

bors commented Jun 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dieterplex commented Jun 18, 2024 •

edited

Loading

dieterplex Jun 19, 2024 •

edited

Loading

dieterplex Jun 20, 2024 •

edited

Loading

epage Jun 21, 2024 •

edited

Loading