test: migrate clean to snapbox #14096

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

        .run();
    // 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

Open

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
22 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: migrate clean to snapbox #14096

test: migrate clean to snapbox #14096

dieterplex commented Jun 18, 2024 •

edited

Loading

rustbot commented Jun 18, 2024

dieterplex Jun 19, 2024 •

edited

Loading

weihanglo Jun 19, 2024

dieterplex Jun 19, 2024

weihanglo Jun 19, 2024

weihanglo Jun 19, 2024

weihanglo Jun 20, 2024

dieterplex Jun 20, 2024 •

edited

Loading

weihanglo Jun 25, 2024

dieterplex Jun 26, 2024

weihanglo Jun 21, 2024

dieterplex Jun 21, 2024

epage Jun 21, 2024 •

edited

Loading

weihanglo Jun 21, 2024

weihanglo commented Jun 26, 2024

bors commented Jun 26, 2024

bors commented Jun 26, 2024

bors commented Jun 26, 2024

test: migrate clean to snapbox #14096

test: migrate clean to snapbox #14096

Conversation

dieterplex commented Jun 18, 2024 • edited Loading

rustbot commented Jun 18, 2024

dieterplex Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dieterplex Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epage Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weihanglo commented Jun 26, 2024

bors commented Jun 26, 2024

bors commented Jun 26, 2024

bors commented Jun 26, 2024

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