search

rust-lang / crater

Run experiments across parts of the Rust ecosystem!
https://crater.rust-lang.org
643 stars 90 forks source link

"Pass, then timeout" should not be considered a spurious regression #739

Open workingjubilee opened 2 months ago

workingjubilee commented 2 months ago

Issue is visible in this crater report:

Crater was being used to diagnose usage of the new trait solver, which has hang issues in some cases.

This caused https://github.com/rust-lang/rust/issues/130056 to be opened after landing https://github.com/rust-lang/rust/pull/121848 even though we did notice essentially all the nalgebra dependents failed this way: nalgebra-glm, nalgebra-macros, nalgebra-mvn, nalgebra-py, nalgebra-spacetime, nalgebra_latex, nalgebra_linsys all failing with "build timed out" after the first round of tests passed

workingjubilee commented 2 months ago

other possible solutions might be to find some way to introduce retrying or extended second-pass build times that make this work.

pietroalbini commented 2 months ago

The reason why timeouts regressions are marked as spurious is because the amount of time it takes to build a crate varies wildly between builds, depending on whether the dependencies were cached or not in the target directory. If we didn't mark those as spurious every crater run would contain spurious regressions in the regressed crates list.

Skgland commented 2 months ago

other possible solutions might be to find some way to introduce retrying or extended second-pass build times that make this work.

I don't think this is what you meant but since afc64968b36bc9bda5de3dac703e19ae6823b378 spurious regressed crates are included in the retry-regressed-list.txt, though it looks like the crater run mentioned was from before that change.

Maybe it would help if the crater bot would mention an unusually high amount of spuriously regressed crates in addition to regressed and fixed crates, or spuriously regressed could be a sub-category under regressed?

i.e.

🎉 Experiment pr-121848-3 is completed! 📊 16 regressed and 3 fixed (501375 total) 📰 Open the full report.

đŸ”Ĩ Unusual high amount of spurious regressed crates 2489 (instead of ~500-600)

⚠ī¸ If you notice any spurious failure please add them to the blacklist! ℹī¸ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Skgland commented 2 months ago

The reason why timeouts regressions are marked as spurious is because the amount of time it takes to build a crate varies wildly between builds, depending on whether the dependencies were cached or not in the target directory. If we didn't mark those as spurious every crater run would contain spurious regressions in the regressed crates list.

This sounds like something along the lines of rust-lang/cargo#2644 might help as it would allow to make a two phase build, first deps only and then the crate itself.