reclassify prepare failure

config.toml

@@ -143,7 +143,6 @@ yices2 = { skip = true } # Sometimes times out, sometimes doesn't
 scheduled-executor = { skip-tests = true } # UB, allocator corruption
 skippy-rs = { skip-tests = true } # UB, out-of-bounds get_unchecked
 pepe-telemetry = { skip-test = true } # flaky test (concurrency)
-puid = { skip-tests = true } # flaky test (timing)
 watchable = { skip-tests = true } # flaky test (timing)
 lispi = { skip-tests = true } # flaky test (data races)
 cargo-ramdisk = { skip-tests = true } # flaky test (concurrency)

src/bin/test-report.rs

@@ -79,6 +79,6 @@ impl ReportWriter for NullWriter {
 
 impl fmt::Display for NullWriter {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self)
+        write!(f, "{self:?}")
     }
 }

src/db/mod.rs

@@ -60,7 +60,7 @@ impl<E: std::error::Error> r2d2::HandleError<E> for ErrorHandler {
         if log::log_enabled!(log::Level::Error) {
             log::error!("r2d2 error: {:?}", error);
         } else {
-            eprintln!("r2d2 error: {:?}", error);
+            eprintln!("r2d2 error: {error:?}");
         }
     }
 }

src/report/archives.rs

@@ -51,7 +51,7 @@ impl Drop for TempfileBackedBuffer {
     fn drop(&mut self) {
         unsafe {
             if let Err(e) = nix::sys::mman::munmap(self.mmap.as_ptr() as *mut _, self.mmap.len()) {
-                eprintln!("Failed to unmap temporary file: {:?}", e);
+                eprintln!("Failed to unmap temporary file: {e:?}");
             }
         }
     }

src/report/display.rs

@@ -58,6 +58,7 @@ impl ResultName for TestResult {
     fn short_name(&self) -> String {
         match self {
             TestResult::BrokenCrate(reason) => reason.short_name(),
+            TestResult::PrepareFail(reason) => format!("prepare {}", reason.short_name()),
             TestResult::BuildFail(reason) => format!("build {}", reason.short_name()),
             TestResult::TestFail(reason) => format!("test {}", reason.short_name()),
             TestResult::TestSkipped => "test skipped".into(),
@@ -69,6 +70,7 @@ impl ResultName for TestResult {
 
     fn long_name(&self) -> String {
         match self {
+            TestResult::PrepareFail(reason) => format!("prepare {}", reason.long_name()),
             TestResult::BuildFail(reason) => format!("build {}", reason.long_name()),
             TestResult::TestFail(reason) => format!("test {}", reason.long_name()),
             TestResult::BrokenCrate(reason) => reason.long_name(),
@@ -103,6 +105,7 @@ impl ResultColor for Comparison {
             Comparison::SameTestPass => Color::Single("#72a156"),
             Comparison::Error => Color::Single("#d77026"),
             Comparison::Broken => Color::Single("#44176e"),
+            Comparison::PrepareFail => Color::Striped("#44176e", "#d77026"),
             Comparison::SpuriousRegressed => Color::Striped("#db3026", "#d5433b"),
             Comparison::SpuriousFixed => Color::Striped("#5630db", "#5d3dcf"),
         }
@@ -117,6 +120,7 @@ impl ResultColor for TestResult {
             TestResult::TestFail(_) => Color::Single("#65461e"),
             TestResult::TestSkipped | TestResult::TestPass => Color::Single("#62a156"),
             TestResult::Error => Color::Single("#d77026"),
+            TestResult::PrepareFail(_) => Color::Striped("#44176e", "#d77026"),
             TestResult::Skipped => Color::Single("#494b4a"),
         }
     }

src/report/mod.rs

@@ -71,6 +71,7 @@ string_enum!(pub enum Comparison {
     Unknown => "unknown",
     Error => "error",
     Broken => "broken",
+    PrepareFail => "prepare-fail",
     SameBuildFail => "build-fail",
     SameTestFail => "test-fail",
     SameTestSkipped => "test-skipped",
@@ -87,7 +88,8 @@ impl Comparison {
             | Comparison::Unknown
             | Comparison::Error
             | Comparison::SpuriousRegressed
-            | Comparison::SpuriousFixed => true,
+            | Comparison::SpuriousFixed
+            | Comparison::PrepareFail => true,
             Comparison::Skipped
             | Comparison::Broken
             | Comparison::SameBuildFail
@@ -107,6 +109,7 @@ impl Comparison {
             | Comparison::SpuriousFixed
             | Comparison::Skipped
             | Comparison::Broken
+            | Comparison::PrepareFail
             | Comparison::SameBuildFail
             | Comparison::SameTestFail
             | Comparison::SameTestSkipped
@@ -406,7 +409,9 @@ fn gen_retry_list(res: &RawTestResults) -> String {
         .crates
         .iter()
         .filter(|crate_res| {
-            crate_res.res == Comparison::Regressed || crate_res.res == Comparison::SpuriousRegressed
+            crate_res.res == Comparison::Regressed
+                || crate_res.res == Comparison::SpuriousRegressed
+                || crate_res.res == Comparison::PrepareFail
         })
         .map(|crate_res| &crate_res.krate);
 
@@ -486,53 +491,37 @@ fn compare(
             }
             (BuildFail(_), BuildFail(FailureReason::ICE)) => Comparison::Regressed,
 
+            // same
             (BuildFail(_), BuildFail(_)) => Comparison::SameBuildFail,
-            (TestFail(_), TestFail(_)) => Comparison::SameTestFail,
             (TestSkipped, TestSkipped) => Comparison::SameTestSkipped,
+            (TestFail(_), TestFail(_)) => Comparison::SameTestFail,
             (TestPass, TestPass) => Comparison::SameTestPass,
 
-            (BuildFail(ref reason1), TestFail(ref reason2))
-                if reason1.is_spurious() || reason2.is_spurious() =>
-            {
-                Comparison::SpuriousFixed
-            }
-            (BuildFail(ref reason), TestSkipped)
-            | (BuildFail(ref reason), TestPass)
-            | (TestFail(ref reason), TestPass)
-                if reason.is_spurious() =>
-            {
-                Comparison::SpuriousFixed
-            }
-            (BuildFail(_), TestFail(_))
-            | (BuildFail(_), TestSkipped)
-            | (BuildFail(_), TestPass)
-            | (TestFail(_), TestPass) => Comparison::Fixed,
-            (TestFail(_), BuildFail(reason)) if !reason.is_spurious() => Comparison::Regressed,
-            (TestFail(reason1), BuildFail(reason2))
-                if reason1.is_spurious() || reason2.is_spurious() =>
-            {
-                Comparison::SpuriousRegressed
+            // (spurious) fixed
+            (BuildFail(reason), TestSkipped | TestFail(_) | TestPass)
+            | (TestFail(reason), TestPass) => {
+                if reason.is_spurious() {
+                    Comparison::SpuriousFixed
+                } else {
+                    Comparison::Fixed
+                }
             }
-            (TestPass, TestFail(reason))
-            | (TestPass, BuildFail(reason))
-            | (TestSkipped, BuildFail(reason))
-            | (TestFail(_), BuildFail(reason))
-                if reason.is_spurious() =>
-            {
-                Comparison::SpuriousRegressed
+
+            // (spurious) regressed
+            (TestSkipped | TestFail(_) | TestPass, BuildFail(reason))
+            | (TestPass, TestFail(reason)) => {
+                if reason.is_spurious() {
+                    Comparison::SpuriousRegressed
+                } else {
+                    Comparison::Regressed
+                }
             }
-            (TestPass, TestFail(_))
-            | (TestPass, BuildFail(_))
-            | (TestSkipped, BuildFail(_))
-            | (TestFail(_), BuildFail(_)) => Comparison::Regressed,
 
+            (PrepareFail(_), _) | (_, PrepareFail(_)) => Comparison::PrepareFail,
             (Error, _) | (_, Error) => Comparison::Error,
             (Skipped, _) | (_, Skipped) => Comparison::Skipped,
             (BrokenCrate(_), _) | (_, BrokenCrate(_)) => Comparison::Broken,
-            (TestFail(_), TestSkipped)
-            | (TestPass, TestSkipped)
-            | (TestSkipped, TestFail(_))
-            | (TestSkipped, TestPass) => {
+            (TestFail(_) | TestPass, TestSkipped) | (TestSkipped, TestFail(_) | TestPass) => {
                 panic!("can't compare {res1} and {res2}");
             }
         },
@@ -821,6 +810,7 @@ mod tests {
 
                 // Non-spurious fixes/regressions
                 BuildFail(Unknown), TestFail(Unknown) => Fixed;
+                BuildFail(Unknown), TestFail(OOM) => Fixed;
                 BuildFail(Unknown), TestSkipped => Fixed;
                 BuildFail(Unknown), TestPass => Fixed;
                 TestFail(Unknown), TestPass => Fixed;
@@ -837,7 +827,6 @@ mod tests {
 
                 // Spurious fixes/regressions
                 BuildFail(OOM), TestFail(Unknown) => SpuriousFixed;
-                BuildFail(Unknown), TestFail(OOM) => SpuriousFixed;
                 BuildFail(OOM), TestSkipped => SpuriousFixed;
                 BuildFail(OOM), TestPass => SpuriousFixed;
                 TestFail(OOM), TestPass => SpuriousFixed;
@@ -846,6 +835,10 @@ mod tests {
                 TestSkipped, BuildFail(OOM) => SpuriousRegressed;
                 TestFail(Unknown), BuildFail(OOM) => SpuriousRegressed;
 
+                // PrepareFail
+                PrepareFail(Unknown), BuildFail(Unknown) => PrepareFail;
+                BuildFail(Unknown), PrepareFail(Unknown) => PrepareFail;
+
                 // Errors
                 Error, TestPass => Error;
                 Error, TestSkipped => Error;

src/results/mod.rs

@@ -307,6 +307,7 @@ string_enum!(pub enum BrokenReason {
 test_result_enum!(pub enum TestResult {
     with_reason {
         BrokenCrate(BrokenReason) => "broken",
+        PrepareFail(FailureReason) => "prepare-fail",
         BuildFail(FailureReason) => "build-fail",
         TestFail(FailureReason) => "test-fail",
     }
@@ -356,6 +357,11 @@ mod tests {
 
         //"build-fail:depends-on()" => BuildFail(DependsOn(vec!["001"])),
         test_from_str! {
+            "prepare-fail:unknown" => PrepareFail(Unknown),
+            "prepare-fail:oom" => PrepareFail(OOM),
+            "prepare-fail:docker" => PrepareFail(Docker),
+            "prepare-fail:no-space" => PrepareFail(NoSpace),
+            "prepare-fail:timeout" => PrepareFail(Timeout),
             "build-fail:unknown" => BuildFail(Unknown),
             "build-fail:docker" => BuildFail(Docker),
             "build-fail:compiler-error(001, 002)" => BuildFail(CompilerError(btreeset!["001".parse().unwrap(), "002".parse().unwrap()])),

src/runner/test.rs

@@ -15,7 +15,7 @@ use rustwide::{Build, PrepareError};
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::io::ErrorKind;
 
-fn failure_reason(err: &Error) -> FailureReason {
+pub(crate) fn failure_reason(err: &Error) -> FailureReason {
     if let Some(reason) = err.downcast_ref::<FailureReason>() {
         reason.clone()
     } else if let Some(command_error) = err.downcast_ref::<CommandError>() {

src/runner/worker.rs

@@ -4,7 +4,7 @@ use crate::experiments::{Experiment, Mode};
 use crate::prelude::*;
 use crate::results::{BrokenReason, TestResult};
 use crate::runner::tasks::{Task, TaskStep};
-use crate::runner::test::detect_broken;
+use crate::runner::test::{detect_broken, failure_reason};
 use crate::runner::OverrideResult;
 use crate::toolchain::Toolchain;
 use crate::utils;
@@ -244,7 +244,7 @@ impl<'a> Worker<'a> {
                 let mut result = if self.config.is_broken(&krate) {
                     TestResult::BrokenCrate(BrokenReason::Unknown)
                 } else {
-                    TestResult::Error
+                    TestResult::PrepareFail(failure_reason(&err))
                 };
 
                 if let Some(OverrideResult(res)) = err.downcast_ref() {
@@ -256,11 +256,8 @@ impl<'a> Worker<'a> {
                         self.ex,
                         &krate,
                         tc,
-                        format!(
-                            "{}\n\nthis task or one of its parent failed: {:?}",
-                            logs, err
-                        )
-                        .as_bytes(),
+                        format!("{logs}\n\nthis task or one of its parent failed: {err:?}")
+                            .as_bytes(),
                         &result,
                         updated_version.as_ref().map(|new| (&krate, new)),
                     ) {
@@ -327,7 +324,7 @@ impl<'a> Worker<'a> {
                             self.ex,
                             &task.krate,
                             tc,
-                            format!("{}\n\n{:?}", storage, err).as_bytes(),
+                            format!("{storage}\n\n{err:?}").as_bytes(),
                             &test_result,
                             updated_version.as_ref().map(|new| (&krate, new)),
                         )?;

src/server/agents.rs

@@ -178,7 +178,6 @@ impl Agents {
             .transpose()?
             .map(|agent| agent.with_capabilities(&self.db))
             .transpose()
-            .map_err(Into::into)
     }
 
     pub fn record_heartbeat(&self, agent: &str) -> Fallible<()> {

src/server/routes/ui/experiments.rs

@@ -131,7 +131,7 @@ fn humanize(duration: Duration) -> String {
         Ok(d) => d,
         Err(_) => {
             // Don't try to make it pretty as a fallback.
-            return format!("{:?}", duration);
+            return format!("{duration:?}");
         }
     };
     if duration.as_secs() < 60 {

src/server/try_builds.rs

@@ -105,9 +105,8 @@ mod tests {
             &format!(
                 r#"
                     Try build passed.
-                    <!-- homu: {{"type": "TryBuildCompleted", "merge_sha": "{}"}} -->
-                "#,
-                COMMIT_A
+                    <!-- homu: {{"type": "TryBuildCompleted", "merge_sha": "{COMMIT_A}"}} -->
+                "#
             ),
         )
         .unwrap();

tests/minicrater/driver.rs

@@ -79,10 +79,7 @@ trait Compare {
                 "\n",
             );
             if changeset.distance != 0 {
-                eprintln!(
-                    "Difference between expected and actual reports:\n{}",
-                    changeset
-                );
+                eprintln!("Difference between expected and actual reports:\n{changeset}");
                 eprintln!("To expect the new report in the future run:");
                 eprintln!(
                     "$ cp {} {}\n",