diff --git a/src/actions/experiments/create.rs b/src/actions/experiments/create.rs index 80a1070a..4cd959ee 100644 --- a/src/actions/experiments/create.rs +++ b/src/actions/experiments/create.rs @@ -16,6 +16,7 @@ pub struct CreateExperiment { pub ignore_blacklist: bool, pub assign: Option, pub requirement: Option, + pub stat_run: Option, } impl CreateExperiment { @@ -34,6 +35,7 @@ impl CreateExperiment { ignore_blacklist: false, assign: None, requirement: None, + stat_run: None, } } } @@ -57,8 +59,8 @@ impl Action for CreateExperiment { "INSERT INTO experiments \ (name, mode, cap_lints, toolchain_start, toolchain_end, priority, created_at, \ status, github_issue, github_issue_url, github_issue_number, ignore_blacklist, \ - assigned_to, requirement) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14);", + assigned_to, requirement, stat_run) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15);", &[ &self.name, &self.mode.to_str(), @@ -74,6 +76,7 @@ impl Action for CreateExperiment { &self.ignore_blacklist, &self.assign.map(|a| a.to_string()), &self.requirement, + &self.stat_run, ], )?; @@ -130,6 +133,7 @@ mod tests { ignore_blacklist: true, assign: None, requirement: Some("linux".to_string()), + stat_run: None, } .apply(&ctx) .unwrap(); @@ -253,6 +257,7 @@ mod tests { ignore_blacklist: false, assign: None, requirement: None, + stat_run: None, } .apply(&ctx) .unwrap_err(); @@ -283,6 +288,7 @@ mod tests { ignore_blacklist: false, assign: None, requirement: None, + stat_run: None, } .apply(&ctx) .unwrap(); @@ -299,6 +305,7 @@ mod tests { ignore_blacklist: false, assign: None, requirement: None, + stat_run: None, } .apply(&ctx) .unwrap_err(); diff --git a/src/actions/experiments/edit.rs b/src/actions/experiments/edit.rs index f3d83194..1e36e8e8 100644 --- a/src/actions/experiments/edit.rs +++ b/src/actions/experiments/edit.rs @@ -205,6 +205,7 @@ mod tests { ignore_blacklist: false, assign: None, requirement: None, + stat_run: None, } .apply(&ctx) .unwrap(); diff --git a/src/cli.rs b/src/cli.rs index 92c01112..9d3d64fd 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -116,6 +116,12 @@ pub enum Crater { assign: Option, #[clap(name = "requirement", long = "requirement")] requirement: Option, + #[clap( + name = "stat-run", + long = "stat-run", + help = "For statistic testing mode (build-and-stat-test), amount of test runs." + )] + stat_run: Option, }, #[clap(name = "edit", about = "edit an experiment configuration")] @@ -305,6 +311,7 @@ impl Crater { ref ignore_blacklist, ref assign, ref requirement, + ref stat_run, } => { let config = Config::load()?; let db = Database::open()?; @@ -321,6 +328,7 @@ impl Crater { ignore_blacklist: *ignore_blacklist, assign: assign.clone(), requirement: requirement.clone(), + stat_run: *stat_run, } .apply(&ctx)?; } diff --git a/src/db/migrations.rs b/src/db/migrations.rs index 6132a491..e3344d11 100644 --- a/src/db/migrations.rs +++ b/src/db/migrations.rs @@ -363,6 +363,18 @@ fn migrations() -> Vec<(&'static str, MigrationKind)> { MigrationKind::SQL("alter table agents add column latest_work_for text;"), )); + // `results` table analog for statistic runs. + // statistic run performs one build and several test runs. + // run_num == 0 keeps build log. + migrations.push(( + "add_statistic_results", + MigrationKind::SQL( + " + ALTER TABLE experiments ADD COLUMN stat_run INTEGER DEFAULT 0; + ", + ), + )); + migrations } diff --git a/src/experiments.rs b/src/experiments.rs index 1a25973f..31cb96da 100644 --- a/src/experiments.rs +++ b/src/experiments.rs @@ -25,6 +25,7 @@ string_enum!(pub enum Status { string_enum!(pub enum Mode { BuildAndTest => "build-and-test", + BuildAndStatTest => "build-and-stat-test", BuildOnly => "build-only", CheckOnly => "check-only", Clippy => "clippy", @@ -259,6 +260,7 @@ pub struct Experiment { pub report_url: Option, pub ignore_blacklist: bool, pub requirement: Option, + pub stat_run: Option, } impl Experiment { @@ -678,6 +680,7 @@ pub struct ExperimentDBRecord { report_url: Option, ignore_blacklist: bool, requirement: Option, + stat_run: Option, } impl ExperimentDBRecord { @@ -700,6 +703,7 @@ impl ExperimentDBRecord { report_url: row.get("report_url")?, ignore_blacklist: row.get("ignore_blacklist")?, requirement: row.get("requirement")?, + stat_run: row.get("stat_run")?, }) } @@ -735,6 +739,7 @@ impl ExperimentDBRecord { report_url: self.report_url, ignore_blacklist: self.ignore_blacklist, requirement: self.requirement, + stat_run: self.stat_run, }) } } diff --git a/src/report/analyzer.rs b/src/report/analyzer.rs index f14c6fc1..c6a3d85e 100644 --- a/src/report/analyzer.rs +++ b/src/report/analyzer.rs @@ -166,6 +166,7 @@ mod tests { report_url: None, ignore_blacklist: false, requirement: None, + stat_run: None, }; let crates = record_crates! {db, ex, diff --git a/src/report/display.rs b/src/report/display.rs index 3f0d5f8a..513e2236 100644 --- a/src/report/display.rs +++ b/src/report/display.rs @@ -1,6 +1,7 @@ use crate::prelude::*; use crate::report::Comparison; -use crate::results::{BrokenReason, FailureReason, TestResult}; +use crate::results::{BrokenReason, FailureReason, StatFailureReasons, TestResult}; +use std::collections::HashMap; pub trait ResultName { fn short_name(&self) -> String; @@ -38,6 +39,41 @@ impl ResultName for FailureReason { } } +impl ResultName for StatFailureReasons { + fn short_name(&self) -> String { + let StatFailureReasons::Reasons(vec) = self else { + return String::new(); + }; + // ex: "failed (unknown) x2 | OOM x4 | ICE x1" + let unique_counts: HashMap = + vec.iter().fold(HashMap::new(), |mut map, val| { + *map.entry(val.short_name()).or_insert(0) += 1; + map + }); + unique_counts + .iter() + .map(|v| format!("{} x{}", v.0, v.1)) + .collect::>() + .join(" | ") + } + + fn long_name(&self) -> String { + let StatFailureReasons::Reasons(vec) = self else { + return String::new(); + }; + let unique_counts: HashMap = + vec.iter().fold(HashMap::new(), |mut map, val| { + *map.entry(val.long_name()).or_insert(0) += 1; + map + }); + unique_counts + .iter() + .map(|v| format!("{} x{}", v.0, v.1)) + .collect::>() + .join(" | ") + } +} + impl ResultName for BrokenReason { fn short_name(&self) -> String { match self { @@ -61,6 +97,7 @@ impl ResultName for TestResult { 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::TestsFail(reason) => format!("tests: {}", reason.short_name()), TestResult::TestSkipped => "test skipped".into(), TestResult::TestPass => "test passed".into(), TestResult::Error => "error".into(), @@ -73,6 +110,7 @@ impl ResultName for TestResult { 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::TestsFail(reason) => format!("tests: {}", reason.long_name()), TestResult::BrokenCrate(reason) => reason.long_name(), TestResult::TestSkipped | TestResult::TestPass @@ -118,6 +156,7 @@ impl ResultColor for TestResult { TestResult::BrokenCrate(_) => Color::Single("#44176e"), TestResult::BuildFail(_) => Color::Single("#db3026"), TestResult::TestFail(_) => Color::Single("#65461e"), + TestResult::TestsFail(_) => Color::Single("#65461e"), TestResult::TestSkipped | TestResult::TestPass => Color::Single("#62a156"), TestResult::Error => Color::Single("#d77026"), TestResult::PrepareFail(_) => Color::Striped("#44176e", "#d77026"), diff --git a/src/report/mod.rs b/src/report/mod.rs index 864f73a5..b9b97e5a 100644 --- a/src/report/mod.rs +++ b/src/report/mod.rs @@ -4,7 +4,9 @@ use crate::dirs::WORK_DIR; use crate::experiments::Experiment; use crate::prelude::*; use crate::report::analyzer::{analyze_report, ReportConfig, ToolchainSelect}; -use crate::results::{EncodedLog, EncodingType, FailureReason, ReadResults, TestResult}; +use crate::results::{ + EncodedLog, EncodingType, FailureReason, ReadResults, StatFailureReasons, TestResult, +}; use crate::toolchain::Toolchain; use crate::utils; use crates_index::SparseIndex; @@ -514,7 +516,7 @@ fn compare( (TestPass, TestPass) => Comparison::SameTestPass, // (spurious) fixed - (BuildFail(reason), TestSkipped | TestFail(_) | TestPass) + (BuildFail(reason), TestSkipped | TestFail(_) | TestsFail(_) | TestPass) | (TestFail(reason), TestPass) => { if reason.is_spurious() { Comparison::SpuriousFixed @@ -522,9 +524,16 @@ fn compare( Comparison::Fixed } } + (TestsFail(reason), TestPass) => { + if reason.is_spurious() { + Comparison::SpuriousFixed + } else { + Comparison::Fixed + } + } // (spurious) regressed - (TestSkipped | TestFail(_) | TestPass, BuildFail(reason)) + (TestSkipped | TestFail(_) | TestsFail(_) | TestPass, BuildFail(reason)) | (TestPass, TestFail(reason)) => { if reason.is_spurious() { Comparison::SpuriousRegressed @@ -532,14 +541,36 @@ fn compare( Comparison::Regressed } } + (TestPass, TestsFail(reason)) => { + if reason.is_spurious() { + Comparison::SpuriousRegressed + } else { + Comparison::Regressed + } + } (Skipped, _) | (_, Skipped) => Comparison::Skipped, (BrokenCrate(_), _) | (_, BrokenCrate(_)) => Comparison::Broken, (PrepareFail(_), _) | (_, PrepareFail(_)) => Comparison::PrepareFail, (Error, _) | (_, Error) => Comparison::Error, - (TestFail(_) | TestPass, TestSkipped) | (TestSkipped, TestFail(_) | TestPass) => { + (TestFail(_) | TestsFail(_) | TestPass, TestSkipped) + | (TestSkipped, TestFail(_) | TestsFail(_) | TestPass) => { panic!("can't compare {res1} and {res2}"); } + + // Stat tests comparison + (TestsFail(v1), TestsFail(v2)) => { + let (StatFailureReasons::Reasons(v1), StatFailureReasons::Reasons(v2)) = (v1, v2) + else { + panic!("can't compare {res1} and {res2}"); + }; + if v2.len() > v1.len() { + Comparison::Regressed + } else { + Comparison::SameTestFail + } + } + (TestsFail(_), TestFail(_)) | (TestFail(_), TestsFail(_)) => Comparison::SameTestFail, }, _ if config.should_skip(krate) => Comparison::Skipped, _ => Comparison::Unknown, @@ -940,6 +971,7 @@ mod tests { report_url: None, ignore_blacklist: false, requirement: None, + stat_run: None, }; let mut db = DummyDB::default(); diff --git a/src/results/db.rs b/src/results/db.rs index 90bd565e..2d21021a 100644 --- a/src/results/db.rs +++ b/src/results/db.rs @@ -266,10 +266,13 @@ impl crate::runner::RecordProgress for DatabaseDB<'_> { result: &TestResult, version: Option<(&Crate, &Crate)>, ) -> Fallible<()> { - self.store_result(ex, krate, toolchain, result, log, EncodingType::Plain)?; - if let Some((old, new)) = version { + let krate = if let Some((old, new)) = version { self.update_crate_version(ex, old, new)?; - } + new + } else { + krate + }; + self.store_result(ex, krate, toolchain, result, log, EncodingType::Plain)?; Ok(()) } } diff --git a/src/results/mod.rs b/src/results/mod.rs index 90dc9258..e53c8302 100644 --- a/src/results/mod.rs +++ b/src/results/mod.rs @@ -296,6 +296,62 @@ impl FailureReason { } } +#[derive(Debug, PartialEq, Eq, Clone, Hash, Serialize, Deserialize)] +pub enum StatFailureReasons { + Unknown, + Reasons(Vec), +} + +impl std::error::Error for StatFailureReasons {} + +impl ::std::fmt::Display for StatFailureReasons { + fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { + match self { + StatFailureReasons::Unknown => write!(f, "unknown"), + StatFailureReasons::Reasons(vec) => write!( + f, + "reasons({})", + vec.iter() + .map(|v| v.to_string()) + .collect::>() + .join("|"), + ), + } + } +} + +impl ::std::str::FromStr for StatFailureReasons { + type Err = ::anyhow::Error; + + fn from_str(s: &str) -> ::anyhow::Result { + if let (Some(idx), true) = (s.find('('), s.ends_with(')')) { + let prefix = &s[..idx]; + let contents = s[idx + 1..s.len() - 1].split("|"); + match prefix { + "reasons" => Ok(StatFailureReasons::Reasons( + contents.map(|st| st.parse().unwrap()).collect(), + )), + _ => bail!("unexpected prefix: {}", prefix), + } + } else { + match s { + "unknown" => Ok(StatFailureReasons::Unknown), + _ => bail!("unexpected value: {}", s), + } + } + } +} + +impl StatFailureReasons { + pub(crate) fn is_spurious(&self) -> bool { + if let StatFailureReasons::Reasons(vec) = self { + vec.iter().all(|v| v.is_spurious()) + } else { + false + } + } +} + string_enum!(pub enum BrokenReason { Unknown => "unknown", CargoToml => "cargo-toml", @@ -310,6 +366,7 @@ test_result_enum!(pub enum TestResult { PrepareFail(FailureReason) => "prepare-fail", BuildFail(FailureReason) => "build-fail", TestFail(FailureReason) => "test-fail", + TestsFail(StatFailureReasons) => "stat-tests-fail", } without_reason { TestSkipped => "test-skipped", diff --git a/src/runner/tasks.rs b/src/runner/tasks.rs index 8942b655..60a43112 100644 --- a/src/runner/tasks.rs +++ b/src/runner/tasks.rs @@ -43,6 +43,7 @@ impl<'ctx> TaskCtx<'ctx> { pub(super) enum TaskStep { BuildAndTest { tc: Toolchain, quiet: bool }, + BuildAndStatTest { tc: Toolchain, quiet: bool }, BuildOnly { tc: Toolchain, quiet: bool }, CheckOnly { tc: Toolchain, quiet: bool }, Clippy { tc: Toolchain, quiet: bool }, @@ -55,6 +56,9 @@ impl fmt::Debug for TaskStep { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let (name, quiet, tc) = match *self { TaskStep::BuildAndTest { ref tc, quiet } => ("build and test", quiet, Some(tc)), + TaskStep::BuildAndStatTest { ref tc, quiet } => { + ("build and statistically test", quiet, Some(tc)) + } TaskStep::BuildOnly { ref tc, quiet } => ("build", quiet, Some(tc)), TaskStep::CheckOnly { ref tc, quiet } => ("check", quiet, Some(tc)), TaskStep::Clippy { ref tc, quiet } => ("clippy", quiet, Some(tc)), @@ -107,6 +111,13 @@ impl Task { tc, quiet, ), + TaskStep::BuildAndStatTest { ref tc, quiet } => ( + &build_dir[tc], + "stat testing", + test::test_build_and_stat_test, + tc, + quiet, + ), TaskStep::BuildOnly { ref tc, quiet } => { (&build_dir[tc], "building", test::test_build_only, tc, quiet) } diff --git a/src/runner/test.rs b/src/runner/test.rs index 108c12d4..ce219c68 100644 --- a/src/runner/test.rs +++ b/src/runner/test.rs @@ -1,8 +1,8 @@ use crate::crates::Crate; use crate::experiments::CapLints; use crate::prelude::*; -use crate::results::DiagnosticCode; use crate::results::{BrokenReason, FailureReason, TestResult}; +use crate::results::{DiagnosticCode, StatFailureReasons}; use crate::runner::tasks::TaskCtx; use crate::runner::OverrideResult; use anyhow::Error; @@ -320,6 +320,36 @@ pub(super) fn test_build_and_test( }) } +// One build and several test runs +pub(super) fn test_build_and_stat_test( + ctx: &TaskCtx, + build_env: &Build, + local_packages_id: &[Package], +) -> Fallible { + let build_r = build(ctx, build_env, local_packages_id); + let mut errs = Vec::new(); + if build_r.is_ok() { + // Test loop + for _i in 0..ctx.experiment.stat_run.unwrap() { + if let Some(err) = test(ctx, build_env).err() { + errs.push(err); + } + } + } else { + return Ok(TestResult::BuildFail(failure_reason( + &build_r.err().unwrap(), + ))); + } + + if !errs.is_empty() { + Ok(TestResult::TestsFail(StatFailureReasons::Reasons( + errs.iter().map(|v| failure_reason(v)).collect(), + ))) + } else { + Ok(TestResult::TestPass) + } +} + pub(super) fn test_build_only( ctx: &TaskCtx, build_env: &Build, diff --git a/src/runner/worker.rs b/src/runner/worker.rs index 8019c21c..8b3ca330 100644 --- a/src/runner/worker.rs +++ b/src/runner/worker.rs @@ -115,10 +115,15 @@ impl<'a> Worker<'a> { // // For now we make no distinction between build failures and test failures // here, but that may change if this proves too slow. + // + // No retries for statistic tests. let mut should_retry = false; - if self.ex.toolchains.len() == 2 { + if self.ex.toolchains.len() == 2 + && !matches!(task.step, TaskStep::BuildAndStatTest { .. }) + { let toolchain = match &task.step { TaskStep::BuildAndTest { tc, .. } + | TaskStep::BuildAndStatTest { tc, .. } | TaskStep::BuildOnly { tc, .. } | TaskStep::CheckOnly { tc, .. } | TaskStep::Clippy { tc, .. } @@ -297,6 +302,10 @@ impl<'a> Worker<'a> { tc: tc.clone(), quiet, }, + Mode::BuildAndStatTest => TaskStep::BuildAndStatTest { + tc: tc.clone(), + quiet, + }, Mode::CheckOnly => TaskStep::CheckOnly { tc: tc.clone(), quiet, diff --git a/src/server/routes/agent.rs b/src/server/routes/agent.rs index 72165df2..2b469749 100644 --- a/src/server/routes/agent.rs +++ b/src/server/routes/agent.rs @@ -1,7 +1,7 @@ use crate::agent::Capabilities; use crate::experiments::{Assignee, Experiment}; use crate::prelude::*; -use crate::results::{DatabaseDB, EncodingType, FailureReason, ProgressData, TestResult}; +use crate::results::{DatabaseDB, EncodingType, ProgressData}; use crate::server::agents::WorkerInfo; use crate::server::api_types::{AgentConfig, ApiResponse}; use crate::server::auth::{auth_filter, AuthDetails}; @@ -238,41 +238,11 @@ impl RecordProgressThread { .with_label_values(&["record_progress_worker"]) .observe(start.elapsed().as_secs_f64()); - let to_metric = |f: &FailureReason| match f { - FailureReason::Unknown => "unknown", - FailureReason::OOM => "oom", - FailureReason::NoSpace => "no-space", - FailureReason::Timeout => "timeout", - FailureReason::ICE => "ice", - FailureReason::NetworkAccess => "network-access", - FailureReason::Docker => "docker", - FailureReason::CompilerDiagnosticChange => "compiler-diagnostic-change", - FailureReason::CompilerError(_) => "compiler-error", - FailureReason::DependsOn(_) => "dependency", - }; - metrics .crater_progress_report .with_label_values(&[ ex.name.as_str(), - // Reduce cardinality on the error kind to reduce # of distinct - // metrics created. - &match &result.data.result.result { - TestResult::BrokenCrate(r) => format!("broken-crate:{}", r), - TestResult::PrepareFail(r) => { - format!("prepare-fail:{}", to_metric(r)) - } - TestResult::BuildFail(r) => { - format!("build-fail:{}", to_metric(r)) - } - TestResult::TestFail(r) => { - format!("test-fail:{}", to_metric(r)) - } - TestResult::TestSkipped => "test-skipped".to_owned(), - TestResult::TestPass => "test-pass".to_owned(), - TestResult::Skipped => "skipped".to_owned(), - TestResult::Error => "error".to_owned(), - }, + &result.data.result.result.to_string(), ]) .inc(); } diff --git a/src/server/routes/ui/experiments.rs b/src/server/routes/ui/experiments.rs index 67e469bc..dbfc922e 100644 --- a/src/server/routes/ui/experiments.rs +++ b/src/server/routes/ui/experiments.rs @@ -35,6 +35,7 @@ impl ExperimentData { status_pretty, mode: match experiment.mode { Mode::BuildAndTest => "cargo test", + Mode::BuildAndStatTest => "cargo test", Mode::BuildOnly => "cargo build", Mode::CheckOnly => "cargo check", Mode::Clippy => "cargo clippy", diff --git a/src/server/routes/webhooks/args.rs b/src/server/routes/webhooks/args.rs index 803b68de..7df3793c 100644 --- a/src/server/routes/webhooks/args.rs +++ b/src/server/routes/webhooks/args.rs @@ -113,6 +113,7 @@ generate_parser!(pub enum Command { ignore_blacklist: Option = "ignore-blacklist", assign: Option = "assign", requirement: Option = "requirement", + stat_run: Option = "stat-run", }) "check" => Check(CheckArgs { diff --git a/src/server/routes/webhooks/commands.rs b/src/server/routes/webhooks/commands.rs index 1b5c2526..d9c28ab0 100644 --- a/src/server/routes/webhooks/commands.rs +++ b/src/server/routes/webhooks/commands.rs @@ -44,6 +44,7 @@ pub fn check( ignore_blacklist: args.ignore_blacklist, assign: args.assign, requirement: args.requirement, + stat_run: None, }, ) } @@ -148,6 +149,7 @@ pub fn run( ignore_blacklist: args.ignore_blacklist.unwrap_or(false), assign: args.assign, requirement: Some(requirement), + stat_run: args.stat_run, } .apply(&ActionsCtx::new(&data.db, &data.config))?; diff --git a/templates/report/layout.html b/templates/report/layout.html index 1530b813..2b705d18 100644 --- a/templates/report/layout.html +++ b/templates/report/layout.html @@ -12,7 +12,7 @@
-

Crater report for {{ ex.name }}

+

Crater report for {{ ex.name }}{% if ex.stat_run %} {% if ex.stat_run > 0 %} (statistic testing x{{ ex.stat_run }}) {% endif %} {% endif %}

    {% for item in nav %}