Auto merge of #363 - pietroalbini:spurious-aware, r=pietroalbini

Detect and categorize spurious failures

This PR automatically detects and reports OOMs and timeouts:

![screenshot from 2018-11-10 10-58-03](https://user-images.githubusercontent.com/2299951/48300828-fee61200-e4e3-11e8-9bab-697878b6989d.png)

It also categorizes them as spurious when they are regressions/fixes:

![screenshot from 2018-11-10 12-23-23](https://user-images.githubusercontent.com/2299951/48300827-fee61200-e4e3-11e8-851f-70fa95f07569.png)

Author: bors

Cargo.lock

@@ -27,6 +27,7 @@ lazy_static = "1.0"
 mime = "0.3.1"
 minifier = { version = "0.0.20", features = ["html"] }
 nix = "0.11.0"
+paste = "0.1.3"
 petgraph = "0.4.11"
 r2d2 = "0.8.2"
 r2d2_sqlite = "0.7.0"

Cargo.toml

@@ -6,7 +6,7 @@ use std::fs;
 use std::path::{Path, PathBuf};
 use utils::size::Size;
 
-pub static IMAGE_NAME: &'static str = "crater";
+pub(crate) static IMAGE_NAME: &'static str = "crater";
 
 /// Builds the docker container image, 'crater', what will be used
 /// to isolate builds from each other. This expects the Dockerfile
@@ -24,7 +24,7 @@ pub(crate) fn is_running() -> bool {
 }
 
 #[derive(Copy, Clone)]
-pub enum MountPerms {
+pub(crate) enum MountPerms {
     ReadWrite,
     ReadOnly,
 }
@@ -50,7 +50,7 @@ impl MountConfig {
     }
 }
 
-pub struct ContainerBuilder {
+pub(crate) struct ContainerBuilder {
     image: String,
     mounts: Vec<MountConfig>,
     env: Vec<(String, String)>,
@@ -59,7 +59,7 @@ pub struct ContainerBuilder {
 }
 
 impl ContainerBuilder {
-    pub fn new<S: Into<String>>(image: S) -> Self {
+    pub(crate) fn new<S: Into<String>>(image: S) -> Self {
         ContainerBuilder {
             image: image.into(),
             mounts: Vec::new(),
@@ -69,7 +69,7 @@ impl ContainerBuilder {
         }
     }
 
-    pub fn mount<P1: Into<PathBuf>, P2: Into<PathBuf>>(
+    pub(crate) fn mount<P1: Into<PathBuf>, P2: Into<PathBuf>>(
         mut self,
         host_path: P1,
         container_path: P2,
@@ -83,22 +83,22 @@ impl ContainerBuilder {
         self
     }
 
-    pub fn env<S1: Into<String>, S2: Into<String>>(mut self, key: S1, value: S2) -> Self {
+    pub(crate) fn env<S1: Into<String>, S2: Into<String>>(mut self, key: S1, value: S2) -> Self {
         self.env.push((key.into(), value.into()));
         self
     }
 
-    pub fn memory_limit(mut self, limit: Option<Size>) -> Self {
+    pub(crate) fn memory_limit(mut self, limit: Option<Size>) -> Self {
         self.memory_limit = limit;
         self
     }
 
-    pub fn enable_networking(mut self, enable: bool) -> Self {
+    pub(crate) fn enable_networking(mut self, enable: bool) -> Self {
         self.enable_networking = enable;
         self
     }
 
-    pub fn create(self) -> Fallible<Container> {
+    pub(crate) fn create(self) -> Fallible<Container> {
         let mut args: Vec<String> = vec!["create".into()];
 
         for mount in &self.mounts {
@@ -128,7 +128,7 @@ impl ContainerBuilder {
         Ok(Container { id: out[0].clone() })
     }
 
-    pub fn run(self, quiet: bool) -> Fallible<()> {
+    pub(crate) fn run(self, quiet: bool) -> Fallible<()> {
         let container = self.create()?;
 
         // Ensure the container is properly deleted even if something panics
@@ -152,8 +152,26 @@ fn absolute(path: &Path) -> PathBuf {
     }
 }
 
+#[derive(Debug, Fail)]
+pub(crate) enum DockerError {
+    #[fail(display = "container ran out of memory")]
+    ContainerOOM,
+}
+
+#[derive(Deserialize)]
+#[serde(rename_all = "PascalCase")]
+struct InspectContainer {
+    state: InspectState,
+}
+
+#[derive(Deserialize)]
+struct InspectState {
+    #[serde(rename = "OOMKilled")]
+    oom_killed: bool,
+}
+
 #[derive(Serialize, Deserialize, Clone, Debug)]
-pub struct Container {
+pub(crate) struct Container {
     // Docker container ID
     id: String,
 }
@@ -165,14 +183,37 @@ impl Display for Container {
 }
 
 impl Container {
-    pub fn run(&self, quiet: bool) -> Fallible<()> {
-        RunCommand::new("docker")
+    fn inspect(&self) -> Fallible<InspectContainer> {
+        let output = RunCommand::new("docker")
+            .args(&["inspect", &self.id])
+            .hide_output(true)
+            .run_capture()?;
+
+        let mut data: Vec<InspectContainer> = ::serde_json::from_str(&output.0.join("\n"))?;
+        assert_eq!(data.len(), 1);
+        Ok(data.pop().unwrap())
+    }
+
+    pub(crate) fn run(&self, quiet: bool) -> Fallible<()> {
+        let res = RunCommand::new("docker")
             .args(&["start", "-a", &self.id])
             .quiet(quiet)
-            .run()
+            .run();
+        let details = self.inspect()?;
+
+        // Return a different error if the container was killed due to an OOM
+        if details.state.oom_killed {
+            if let Err(err) = res {
+                Err(err.context(DockerError::ContainerOOM).into())
+            } else {
+                Err(DockerError::ContainerOOM.into())
+            }
+        } else {
+            res
+        }
     }
 
-    pub fn delete(&self) -> Fallible<()> {
+    pub(crate) fn delete(&self) -> Fallible<()> {
         RunCommand::new("docker")
             .args(&["rm", "-f", &self.id])
             .run()

src/docker.rs

@@ -21,6 +21,8 @@ extern crate mime;
 extern crate minifier;
 #[cfg(unix)]
 extern crate nix;
+#[macro_use]
+extern crate paste;
 extern crate petgraph;
 extern crate r2d2;
 extern crate r2d2_sqlite;

src/lib.rs

@@ -1,5 +1,22 @@
+use failure::Context;
 pub use failure::{err_msg, Fail, Fallible, ResultExt};
 
+pub trait FailExt {
+    fn downcast_ctx<T: Fail>(&self) -> Option<&T>;
+}
+
+impl FailExt for dyn Fail {
+    fn downcast_ctx<T: Fail>(&self) -> Option<&T> {
+        if let Some(res) = self.downcast_ref::<T>() {
+            Some(res)
+        } else if let Some(ctx) = self.downcast_ref::<Context<T>>() {
+            Some(ctx.get_context())
+        } else {
+            None
+        }
+    }
+}
+
 macro_rules! to_failure_compat {
     ($($error:path,)*) => {
         pub trait ToFailureCompat<T> {

src/prelude.rs

@@ -100,7 +100,7 @@ mod tests {
     use flate2::read::GzDecoder;
     use mime::Mime;
     use report::DummyWriter;
-    use results::{DatabaseDB, TestResult, WriteResults};
+    use results::{DatabaseDB, FailureReason, TestResult, WriteResults};
     use std::io::Read;
     use tar::Archive;
 
@@ -130,7 +130,7 @@ mod tests {
         results
             .record_result(&ex, &ex.toolchains[1], &crate1, || {
                 info!("tc2 crate1");
-                Ok(TestResult::BuildFail)
+                Ok(TestResult::BuildFail(FailureReason::Unknown))
             }).unwrap();
         results
             .record_result(&ex, &ex.toolchains[0], &crate2, || {

src/report/archives.rs

@@ -4,7 +4,7 @@ use mime;
 use minifier;
 use prelude::*;
 use report::{archives::Archive, Comparison, CrateResult, ReportWriter, TestResults};
-use results::TestResult;
+use results::{FailureReason, TestResult};
 use std::collections::HashMap;
 
 #[derive(Serialize)]
@@ -29,21 +29,50 @@ impl ResultColor for Comparison {
             Comparison::SameTestSkipped => Color::Striped("#72a156", "#80b65f"),
             Comparison::SameTestPass => Color::Single("#72a156"),
             Comparison::Error => Color::Single("#d77026"),
+            Comparison::SpuriousRegressed => Color::Striped("#db3026", "#d5433b"),
+            Comparison::SpuriousFixed => Color::Striped("#5630db", "#5d3dcf"),
         }
     }
 }
 
 impl ResultColor for TestResult {
     fn color(&self) -> Color {
         match self {
-            TestResult::BuildFail => Color::Single("#db3026"),
-            TestResult::TestFail => Color::Single("#65461e"),
+            TestResult::BuildFail(_) => Color::Single("#db3026"),
+            TestResult::TestFail(_) => Color::Single("#65461e"),
             TestResult::TestSkipped | TestResult::TestPass => Color::Single("#62a156"),
             TestResult::Error => Color::Single("#d77026"),
         }
     }
 }
 
+trait ResultName {
+    fn name(&self) -> String;
+}
+
+impl ResultName for FailureReason {
+    fn name(&self) -> String {
+        match self {
+            FailureReason::Unknown => "failed".into(),
+            FailureReason::Broken => "broken".into(),
+            FailureReason::Timeout => "timed out".into(),
+            FailureReason::OOM => "OOM".into(),
+        }
+    }
+}
+
+impl ResultName for TestResult {
+    fn name(&self) -> String {
+        match self {
+            TestResult::BuildFail(reason) => format!("build {}", reason.name()),
+            TestResult::TestFail(reason) => format!("test {}", reason.name()),
+            TestResult::TestSkipped => "test skipped".into(),
+            TestResult::TestPass => "test passed".into(),
+            TestResult::Error => "error".into(),
+        }
+    }
+}
+
 #[derive(Serialize)]
 struct NavbarItem {
     label: &'static str,
@@ -90,6 +119,7 @@ struct ResultsContext<'a> {
 
     comparison_colors: HashMap<Comparison, Color>,
     result_colors: HashMap<TestResult, Color>,
+    result_names: HashMap<TestResult, String>,
 }
 
 #[derive(Serialize)]
@@ -110,6 +140,7 @@ fn write_report<W: ReportWriter>(
 ) -> Fallible<()> {
     let mut comparison_colors = HashMap::new();
     let mut result_colors = HashMap::new();
+    let mut result_names = HashMap::new();
 
     let mut categories = HashMap::new();
     for result in &res.crates {
@@ -118,19 +149,25 @@ fn write_report<W: ReportWriter>(
             continue;
         }
 
-        // Add the colors used in this run
+        // Add the colors and names used in this run
         comparison_colors
             .entry(result.res)
             .or_insert_with(|| result.res.color());
         if let Some(ref run) = result.runs[0] {
             result_colors
                 .entry(run.res)
                 .or_insert_with(|| run.res.color());
+            result_names
+                .entry(run.res)
+                .or_insert_with(|| run.res.name());
         }
         if let Some(ref run) = result.runs[1] {
             result_colors
                 .entry(run.res)
                 .or_insert_with(|| run.res.color());
+            result_names
+                .entry(run.res)
+                .or_insert_with(|| run.res.name());
         }
 
         let mut category = categories.entry(result.res).or_insert_with(Vec::new);
@@ -150,6 +187,7 @@ fn write_report<W: ReportWriter>(
 
         comparison_colors,
         result_colors,
+        result_names,
     };
 
     info!("generating {}", to);