Run cargo unittests by default with x.py test

[hkratz]

Cargo is vital, so its functionality should be tested for all tier 1 platforms in the CI. This PR includes it in the default x.py testsuite.

This adds about 5 min. to x.py test on my Mac.

@rustbot label A-rustbuild A-testsuite T-cargo T-infra

r? @Mark-Simulacrum

[Mark-Simulacrum]

I suspect this makes sense though the time hit will certainly be a bit unfortunate, but in practice test builders are currently not on our long tail for the most part AFAIK (dist-aarch64-apple is) so we should be OK.

@bors r+ rollup=iffy

[bors]

📌 Commit 34c78b5 has been approved by Mark-Simulacrum

[hkratz]

@Mark-Simulacrum This will fail in the i686 CI until rust-lang/cargo#10004 is fixed or disabled.

[Mark-Simulacrum]

ah, right

@bors r-

[infinity0]

In Debian I'm currently using this patch instead:

--- a/src/cargo/core/source/source_id.rs
+++ b/src/cargo/core/source/source_id.rs
@@ -621,7 +621,13 @@
 fn test_cratesio_hash() {
     let config = Config::default().unwrap();
     let crates_io = SourceId::crates_io(&config).unwrap();
-    assert_eq!(crate::util::hex::short_hash(&crates_io), "1ecc6299db9ec823");
+    assert!([
+        "1ecc6299db9ec823", // 64 LE
+        "1285ae84e5963aae", // 32 LE
+        "eae4ba8cbf2ce1c7", // 64 BE
+        "b420f105fcaca6de", // 32 BE
+    ]
+    .contains(&crate::util::hex::short_hash(&crates_io).as_str()));
 }
 
 /// A `Display`able view into a `SourceId` that will write it as a url

[ehuss]

This will likely add up to 10 to 15 minutes to many builders, including PR builds, which I think is a bit too much.

At a minimum, can this be excluded from PR checks? (x86_64-gnu-llvm-12)

Also, I think this will push on the long tail. x86_64-apple is already taking 2h 55m. dist-aarch64-apple is only taking 3h, so that will likely make x86_64-apple the longest running task, pushing overall CI times.

Personally, I don't think this should be done with such a broad stroke. If the concern is that 32-bit targets are not getting tested, then perhaps we should just add those to Cargo's CI? Having a rust-lang/rust PR break Cargo is extraordinarily rare, and I don't think running these tests so broadly will be very helpful.

[hkratz]

Personally, I don't think this should be done with such a broad stroke. If the concern is that 32-bit targets are not getting tested, then perhaps we should just add those to Cargo's CI? Having a rust-lang/rust PR break Cargo is extraordinarily rare, and I don't think running these tests so broadly will be very helpful.

Sounds reasonable. I can submit a PR for running Cargo's CI against i686 on Windows and Linux instead.