From e7d7aed1e6113fe1c677b170e1af9a3a798d9058 Mon Sep 17 00:00:00 2001 From: Marcus Kainth Date: Fri, 25 Sep 2026 14:09:18 +0100 Subject: [PATCH] driver: decide whether a run regressed `clickdoom native regress NEW --history PATH` reads the lines `native diff --record` wrote and judges each line of NEW, in order, against what came before it. It exits 3 when anything regressed, and --findings writes each regression as a JSON line for whatever files the issue. Correctness is judged against the latest line with no error. It regressed when the first refused tic moves earlier or appears, when it names other bits at the same tic, or when a divergence appears or moves earlier. A refusal that moves later names new bits by construction, so bits are only compared at an unchanged tic. A line that compared nothing (compared_through null) says nothing about divergence either way. Cost is judged only against the line directly before it in NEW, and only when both carry the same run_id, CPU model and server version. Hosted runners differ by CPU model from job to job, and analysis times taken on two machines are not comparable, so history is never the cost baseline. A line for a commit already in the history is not judged again and becomes the next line's parent: a nightly that re-measures the last recorded commit first gets a same-VM baseline for the one after it. The limits default to 1.25x for stage1's analysis and 1.3x for the median tic. An error line is reported, judged for nothing, and breaks the cost chain. An empty NEW or a history path that does not exist fails with exit 1, since either would otherwise read as a clean night. --- driver/src/cli/mod.rs | 14 + driver/src/cli/native.rs | 5 +- driver/src/cli/native/regress.rs | 246 +++++++++++ driver/src/native/mod.rs | 4 +- driver/src/native/regress.rs | 394 ++++++++++++++++++ driver/tests/fixtures/regress/history.jsonl | 2 + .../tests/fixtures/regress/night-moved.jsonl | 2 + .../tests/fixtures/regress/night-steady.jsonl | 3 + 8 files changed, 668 insertions(+), 2 deletions(-) create mode 100644 driver/src/cli/native/regress.rs create mode 100644 driver/src/native/regress.rs create mode 100644 driver/tests/fixtures/regress/history.jsonl create mode 100644 driver/tests/fixtures/regress/night-moved.jsonl create mode 100644 driver/tests/fixtures/regress/night-steady.jsonl diff --git a/driver/src/cli/mod.rs b/driver/src/cli/mod.rs index 308cc6f2..99a7fb5b 100644 --- a/driver/src/cli/mod.rs +++ b/driver/src/cli/mod.rs @@ -223,11 +223,25 @@ mod tests { ], &["diff", "100", "--probe", "p.tsv"], &["diff", "100", "--probe", "p.tsv", "--summary"], + &["diff", "100", "--probe", "p.tsv", "--record", "r.jsonl"], &["load"], &["load", "--wad", "w.wad", "--map", "E1M1", "--demo", "DEMO1"], &["load", "--probe", "p.tsv"], &["play"], &["play", "--scale", "1", "--max-tics", "35"], + &["regress", "night.jsonl"], + &[ + "regress", + "night.jsonl", + "--history", + "results.jsonl", + "--findings", + "f.jsonl", + "--analysis-ratio", + "1.5", + "--tic-ratio", + "2", + ], &["render", "--frame", "40"], &["render", "--frame", "40", "--from", "probe", "--fb-hash"], &[ diff --git a/driver/src/cli/native.rs b/driver/src/cli/native.rs index e4874f7d..b52b48db 100644 --- a/driver/src/cli/native.rs +++ b/driver/src/cli/native.rs @@ -1,6 +1,6 @@ //! The `native` namespace. //! -//! Every subcommand here shares one connection and +//! Every subcommand here that talks to a server shares one connection and //! [`ConnArgs`](crate::client::ConnArgs). A subcommand with more to it than //! its argument list lives in its own module beside this one. @@ -8,6 +8,7 @@ pub mod demo; pub mod diff; pub mod load; pub mod play; +pub mod regress; pub mod render; use std::time::{Duration, Instant}; // purity-ok: pacing and latency measurement in the driver, never a value a statement reads @@ -48,6 +49,7 @@ pub enum Command { Diff(diff::DiffCmd), Load(load::LoadCmd), Play(play::PlayCmd), + Regress(regress::RegressCmd), Render(render::RenderCmd), SessionCheck(SessionCheckCmd), } @@ -93,6 +95,7 @@ pub(super) async fn run(cmd: &NativeCmd) -> Result { Command::Diff(cmd) => diff::run(cmd).await, Command::Load(cmd) => load::run(cmd).await, Command::Play(cmd) => play::run(cmd).await, + Command::Regress(cmd) => regress::run(cmd), Command::Render(cmd) => render::run(cmd).await, Command::SessionCheck(cmd) => session_check(cmd).await, } diff --git a/driver/src/cli/native/regress.rs b/driver/src/cli/native/regress.rs new file mode 100644 index 00000000..ac00294f --- /dev/null +++ b/driver/src/cli/native/regress.rs @@ -0,0 +1,246 @@ +//! `clickdoom native regress`: whether recorded differential runs got +//! worse. + +use std::io::Write; +use std::path::{Path, PathBuf}; + +use clap::Args; + +use crate::cli::{Exit, Failure, failed, gate}; +use crate::native::record::{self, Record}; +use crate::native::regress::{self, Finding, Limits, Verdict, short}; + +#[derive(Args)] +#[command( + about = "Judge `native diff --record` lines against the ones before them", + // Hard-wrapped: clap only rewraps help text with its `wrap_help` + // feature, which this binary does not enable. + long_about = "\ +Read the lines `native diff --record` wrote to NEW and judge each one, in +order, against the lines before it. Talks to no server. + +Correctness is judged against the latest line with no error, in --history +or earlier in NEW. It regressed when the first refused tic moves earlier or +appears, when it names other bits at the same tic, or when a divergence +appears or moves earlier. A refusal that moves later is progress. + +Cost is judged only against the line directly before it in NEW, and only +when both were measured by the same run (run_id) on the same CPU model and +server. It regressed when stage1_analysis_s is over --analysis-ratio times +the parent's, or tic_ms_p50 over --tic-ratio times. A line for a commit +already in --history is not judged again and serves as the next line's +parent, so a run that measures the last recorded commit first can judge +the cost of the one after it. + +Exit codes: 0 nothing regressed, 1 a file could not be read or NEW holds no +line, 3 something regressed." +)] +pub struct RegressCmd { + /// The lines to judge, in commit order + #[arg(value_name = "NEW")] + pub new: PathBuf, + /// Lines already judged, oldest first + #[arg(long, value_name = "PATH")] + pub history: Option, + /// Write every regression to this file as one JSON line + #[arg(long, value_name = "PATH")] + pub findings: Option, + /// How much slower stage1's analysis may be than the parent's + #[arg(long, default_value_t = 1.25, value_name = "RATIO")] + pub analysis_ratio: f64, + /// How much slower the median tic may be than the parent's + #[arg(long, default_value_t = 1.3, value_name = "RATIO")] + pub tic_ratio: f64, +} + +pub(crate) fn run(cmd: &RegressCmd) -> Result { + let read = |path: &Path| record::read(path).map_err(|err| failed(err.to_string())); + let history = match &cmd.history { + Some(path) => read(path)?, + None => Vec::new(), + }; + let new: Vec = read(&cmd.new)?; + if new.is_empty() { + return Err(failed(format!( + "{} holds no line, so nothing was judged", + cmd.new.display() + ))); + } + let limits = Limits { + analysis_ratio: cmd.analysis_ratio, + tic_ratio: cmd.tic_ratio, + }; + let verdicts = regress::judge(&history, &new, limits); + let findings: Vec<&Finding> = verdicts.iter().flat_map(findings).collect(); + for verdict in &verdicts { + println!("{}", summary(verdict)); + } + if let Some(path) = &cmd.findings { + write_findings(path, &findings)?; + } + if findings.is_empty() { + return Ok(Exit::Ok); + } + Err(gate(format!( + "{} regression(s) over {} line(s)", + findings.len(), + new.len() + ))) +} + +fn findings(verdict: &Verdict) -> &[Finding] { + match verdict { + Verdict::Judged { findings, .. } => findings, + _ => &[], + } +} + +/// One line per verdict, and one more per finding. +fn summary(verdict: &Verdict) -> String { + match verdict { + Verdict::Recorded { commit } => { + format!("{} already recorded, not judged again", short(commit)) + } + Verdict::Error { commit, error } => format!("{} not judged: {error}", short(commit)), + Verdict::Judged { + commit, + against, + cost_against, + findings, + } => { + let against = against + .as_deref() + .map_or("nothing before it".to_owned(), |c| short(c).to_owned()); + let cost = cost_against.as_deref().map_or( + "cost not judged: no parent measured by this run on this machine".to_owned(), + |c| format!("cost against {}", short(c)), + ); + let mut text = format!( + "{} against {against}, {cost}: {}", + short(commit), + match findings.len() { + 0 => "no regression".to_owned(), + n => format!("{n} regression(s)"), + } + ); + for finding in findings { + text.push_str(&format!("\n {finding}")); + } + text + } + } +} + +fn write_findings(path: &Path, findings: &[&Finding]) -> Result<(), Failure> { + let write = |err: std::io::Error| failed(format!("writing {}: {err}", path.display())); + let mut file = std::fs::File::create(path).map_err(write)?; + for finding in findings { + let line = serde_json::to_string(finding).expect("a finding serializes"); + writeln!(file, "{line}").map_err(write)?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn cmd(new: &Path, history: Option<&Path>, findings: Option<&Path>) -> RegressCmd { + RegressCmd { + new: new.to_owned(), + history: history.map(Path::to_owned), + findings: findings.map(Path::to_owned), + analysis_ratio: 1.25, + tic_ratio: 1.3, + } + } + + fn fixture(name: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/regress") + .join(name) + } + + fn scratch(name: &str) -> PathBuf { + std::env::temp_dir().join(format!("clickdoom-regress-{}-{name}", std::process::id())) + } + + /// A history whose last line refuses at 275, and a night whose second + /// commit refuses at 210: the command exits 3 and writes that finding. + #[test] + fn a_refusal_that_moved_earlier_exits_3_and_is_written() { + let out = scratch("moved.jsonl"); + let result = run(&cmd( + &fixture("night-moved.jsonl"), + Some(&fixture("history.jsonl")), + Some(&out), + )); + let Err(failure) = result else { + panic!("a moved refusal must be a regression"); + }; + assert_eq!(failure.exit, Exit::Gate, "{}", failure.message); + let written = std::fs::read_to_string(&out).unwrap(); + std::fs::remove_file(&out).ok(); + let lines: Vec = written + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect(); + assert_eq!(lines.len(), 1, "{written}"); + assert_eq!(lines[0]["kind"], "correctness"); + assert_eq!(lines[0]["metric"], "first_refused_tic"); + assert_eq!(lines[0]["before"], "275"); + assert_eq!(lines[0]["after"], "210"); + assert_eq!( + lines[0]["commit"], + "2222222222222222222222222222222222222222" + ); + } + + #[test] + fn a_night_that_held_steady_exits_0_and_writes_no_finding() { + let out = scratch("steady.jsonl"); + let result = run(&cmd( + &fixture("night-steady.jsonl"), + Some(&fixture("history.jsonl")), + Some(&out), + )); + let written = std::fs::read_to_string(&out).unwrap(); + std::fs::remove_file(&out).ok(); + assert!(matches!(result, Ok(Exit::Ok))); + assert_eq!(written, ""); + } + + /// A night that judged nothing is a failure, not a pass. + #[test] + fn an_empty_night_fails() { + let empty = scratch("empty.jsonl"); + std::fs::write(&empty, "\n").unwrap(); + let result = run(&cmd(&empty, None, None)); + std::fs::remove_file(&empty).ok(); + let Err(failure) = result else { + panic!("an empty file judged nothing"); + }; + assert_eq!(failure.exit, Exit::Failed); + assert!( + failure.message.contains("nothing was judged"), + "{}", + failure.message + ); + } + + #[test] + fn a_missing_history_fails_rather_than_judging_against_nothing() { + let result = run(&cmd( + &fixture("night-steady.jsonl"), + Some(&fixture("absent.jsonl")), + None, + )); + assert!(matches!( + result, + Err(Failure { + exit: Exit::Failed, + .. + }) + )); + } +} diff --git a/driver/src/native/mod.rs b/driver/src/native/mod.rs index 3f198540..4f3bb8a0 100644 --- a/driver/src/native/mod.rs +++ b/driver/src/native/mod.rs @@ -12,7 +12,8 @@ //! run renders and what each one draws from. [`refusal`] reads back the //! tic a run stopped at, where `unresolved` or `unimplemented` said one //! could not be produced exactly. [`record`] writes what a differential -//! run found as one JSON line. [`schema`] stands between a database an +//! run found as one JSON line, and [`regress`] judges those lines against +//! the ones before them. [`schema`] stands between a database an //! older binary loaded and one this binary's own statements can read: a //! load without `--fresh` refuses a column that moved, and //! [`Session::open`] refuses a database whose schema hash is not this @@ -27,6 +28,7 @@ pub mod plan; pub mod probe; pub mod record; pub mod refusal; +pub mod regress; pub mod schedule; pub mod schema; pub mod session; diff --git a/driver/src/native/regress.rs b/driver/src/native/regress.rs new file mode 100644 index 00000000..0149721a --- /dev/null +++ b/driver/src/native/regress.rs @@ -0,0 +1,394 @@ +//! Whether a recorded differential run regressed. +//! +//! [`judge`] walks new [`Record`]s in order, each against what came before +//! it. Correctness is judged against the latest line with no error, in the +//! history or earlier in the new lines. Cost is judged only against the +//! line directly before it in the new lines, and only when both were +//! measured in the same run on the same CPU model and server, because an +//! analysis time taken on another machine says nothing about this change. + +use std::collections::HashSet; + +use serde::Serialize; + +use super::record::Record; + +/// How much slower a child may be than its parent before it counts. +#[derive(Clone, Copy, Debug)] +pub struct Limits { + /// Child over parent for `stage1_analysis_s`. + pub analysis_ratio: f64, + /// Child over parent for `tic_ms_p50`. + pub tic_ratio: f64, +} + +/// Which kind of regression a [`Finding`] is. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum Kind { + Correctness, + Cost, +} + +/// One metric that got worse from one line to the next. +#[derive(Clone, Debug, PartialEq, Serialize)] +pub struct Finding { + pub kind: Kind, + /// The line that got worse. + pub commit: String, + /// The line it was judged against. + pub against: String, + /// The [`Record`] field that moved. + pub metric: &'static str, + pub before: String, + pub after: String, +} + +impl std::fmt::Display for Finding { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let kind = match self.kind { + Kind::Correctness => "correctness", + Kind::Cost => "cost", + }; + write!( + f, + "{kind}: {} {} against {} at {}", + self.metric, + self.after, + self.before, + short(&self.against) + ) + } +} + +/// What [`judge`] made of one new line. +#[derive(Debug, PartialEq)] +pub enum Verdict { + /// The commit already has a line in the history, or earlier in the new + /// lines. It is not judged again, and serves as the next line's parent. + Recorded { commit: String }, + /// The line carries an error, so there is nothing to judge. + Error { commit: String, error: String }, + Judged { + commit: String, + /// The line correctness was judged against, if any came before. + against: Option, + /// The parent cost was judged against, if one was measured in the + /// same run on the same machine. + cost_against: Option, + findings: Vec, + }, +} + +/// Judges every line of `new`, in order, against `history` and the lines +/// of `new` before it. +pub fn judge(history: &[Record], new: &[Record], limits: Limits) -> Vec { + let mut seen: HashSet<&str> = history.iter().map(|line| line.commit.as_str()).collect(); + let mut baseline = history.iter().rev().find(|line| line.error.is_none()); + let mut parent: Option<&Record> = None; + let mut verdicts = Vec::with_capacity(new.len()); + for line in new { + if let Some(error) = &line.error { + verdicts.push(Verdict::Error { + commit: line.commit.clone(), + error: error.clone(), + }); + parent = None; + continue; + } + let cost_parent = parent.filter(|parent| same_machine(parent, line)); + if !seen.insert(line.commit.as_str()) { + verdicts.push(Verdict::Recorded { + commit: line.commit.clone(), + }); + } else { + let mut findings = baseline.map_or_else(Vec::new, |base| correctness(base, line)); + if let Some(parent) = cost_parent { + findings.extend(cost(parent, line, limits)); + } + verdicts.push(Verdict::Judged { + commit: line.commit.clone(), + against: baseline.map(|base| base.commit.clone()), + cost_against: cost_parent.map(|parent| parent.commit.clone()), + findings, + }); + } + baseline = Some(line); + parent = Some(line); + } + verdicts +} + +/// Both lines were measured by one run on one CPU model and one server. +fn same_machine(a: &Record, b: &Record) -> bool { + a.run_id.is_some() + && a.run_id == b.run_id + && a.runner_cpu.is_some() + && a.runner_cpu == b.runner_cpu + && a.clickhouse == b.clickhouse +} + +/// The first refusal moves earlier, appears, or names other bits at the +/// same tic; or a divergence appears or moves earlier. A refusal that +/// moves later is progress, and the bits it names then are new ones. +fn correctness(base: &Record, line: &Record) -> Vec { + let finding = |metric, before: String, after: String| Finding { + kind: Kind::Correctness, + commit: line.commit.clone(), + against: base.commit.clone(), + metric, + before, + after, + }; + let mut findings = Vec::new(); + match (base.first_refused_tic, line.first_refused_tic) { + (b, Some(l)) if b.is_none_or(|b| l < b) => { + findings.push(finding("first_refused_tic", show(b), l.to_string())); + } + (Some(b), Some(l)) if b == l && base.first_refused_bits != line.first_refused_bits => { + findings.push(finding( + "first_refused_bits", + show(base.first_refused_bits.as_deref()), + show(line.first_refused_bits.as_deref()), + )); + } + _ => {} + } + // A line that compared nothing says nothing about divergence. + if base.compared_through.is_some() && line.compared_through.is_some() { + match (base.first_divergent_tic, line.first_divergent_tic) { + (b, Some(l)) if b.is_none_or(|b| l < b) => findings.push(finding( + "first_divergent_tic", + divergence(b, base), + divergence(Some(l), line), + )), + _ => {} + } + } + findings +} + +/// `stage1_analysis_s` or `tic_ms_p50` over its limit against the parent. +fn cost(parent: &Record, line: &Record, limits: Limits) -> Vec { + let metrics = [ + ( + "stage1_analysis_s", + parent.stage1_analysis_s, + line.stage1_analysis_s, + limits.analysis_ratio, + ), + ( + "tic_ms_p50", + parent.tic_ms_p50, + line.tic_ms_p50, + limits.tic_ratio, + ), + ]; + metrics + .into_iter() + .filter_map(|(metric, before, after, ratio)| { + let (before, after) = (before?, after?); + (before > 0.0 && after > before * ratio).then(|| Finding { + kind: Kind::Cost, + commit: line.commit.clone(), + against: parent.commit.clone(), + metric, + before: format!("{before:.3}"), + after: format!("{after:.3} ({:.2}x)", after / before), + }) + }) + .collect() +} + +fn divergence(tic: Option, line: &Record) -> String { + match (tic, &line.first_divergent_field) { + (Some(tic), Some(field)) => format!("{tic} ({field})"), + (Some(tic), None) => tic.to_string(), + (None, _) => format!("none through {}", show(line.compared_through)), + } +} + +fn show(value: Option) -> String { + value.map_or_else(|| "none".to_owned(), |value| value.to_string()) +} + +/// The first 12 hex digits of a commit, as the summary prints it. +pub fn short(commit: &str) -> &str { + commit.get(..12).unwrap_or(commit) +} + +#[cfg(test)] +mod tests { + use super::*; + + const LIMITS: Limits = Limits { + analysis_ratio: 1.25, + tic_ratio: 1.3, + }; + + /// History and new lines as the nightly writes them: JSON, one per line. + fn lines(text: &str) -> Vec { + text.lines() + .filter(|line| !line.trim().is_empty()) + .map(|line| serde_json::from_str(line).expect("a fixture line parses")) + .collect() + } + + const HISTORY: &str = r#" +{"commit":"a1","clickhouse":"26.8.2.7","runner_cpu":"EPYC","run_id":"1","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","compared_through":274,"compared_tics":273,"stage1_analysis_s":180.0,"tic_ms_p50":500.0} +{"commit":"a2","error":"cargo build failed"} +"#; + + fn findings(verdict: &Verdict) -> &[Finding] { + match verdict { + Verdict::Judged { findings, .. } => findings, + other => panic!("not judged: {other:?}"), + } + } + + #[test] + fn a_refusal_that_moves_earlier_is_a_regression() { + let new = lines( + r#"{"commit":"b1","clickhouse":"26.8.2.7","runner_cpu":"EPYC","run_id":"2","first_refused_tic":210,"first_refused_bits":"unresolved: PL_ACTION_NEEDED","compared_through":209,"stage1_analysis_s":180.0,"tic_ms_p50":500.0}"#, + ); + let verdicts = judge(&lines(HISTORY), &new, LIMITS); + assert_eq!(verdicts.len(), 1); + let Verdict::Judged { + against, + cost_against, + findings, + .. + } = &verdicts[0] + else { + panic!("{verdicts:?}"); + }; + assert_eq!(against.as_deref(), Some("a1"), "error lines are skipped"); + assert_eq!(*cost_against, None, "no parent measured in this run"); + assert_eq!(findings.len(), 1, "{findings:?}"); + assert_eq!(findings[0].kind, Kind::Correctness); + assert_eq!(findings[0].metric, "first_refused_tic"); + assert_eq!( + (findings[0].before.as_str(), findings[0].after.as_str()), + ("275", "210") + ); + } + + #[test] + fn a_refusal_that_moves_later_is_progress_whatever_bits_it_names() { + let new = lines( + r#"{"commit":"b1","first_refused_tic":300,"first_refused_bits":"unresolved: PX_CROSSED","compared_through":299}"#, + ); + assert_eq!(findings(&judge(&lines(HISTORY), &new, LIMITS)[0]), &[]); + } + + #[test] + fn other_bits_at_the_same_tic_are_a_regression() { + let new = lines( + r#"{"commit":"b1","first_refused_tic":275,"first_refused_bits":"unresolved: PX_CROSSED","compared_through":274}"#, + ); + let verdicts = judge(&lines(HISTORY), &new, LIMITS); + let found = findings(&verdicts[0]); + assert_eq!(found.len(), 1, "{found:?}"); + assert_eq!(found[0].metric, "first_refused_bits"); + assert_eq!(found[0].after, "unresolved: PX_CROSSED"); + } + + #[test] + fn a_divergence_that_appears_or_moves_earlier_is_a_regression() { + let new = lines( + r#" +{"commit":"b1","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","compared_through":274,"first_divergent_tic":206,"first_divergent_field":"mobj slot 1 m_momx"} +{"commit":"b2","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","compared_through":274,"first_divergent_tic":206,"first_divergent_field":"mobj slot 1 m_momy"} +{"commit":"b3","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","compared_through":274,"first_divergent_tic":100,"first_divergent_field":"player mo m_x"} +"#, + ); + let verdicts = judge(&lines(HISTORY), &new, LIMITS); + let appears = findings(&verdicts[0]); + assert_eq!(appears.len(), 1, "{appears:?}"); + assert_eq!(appears[0].metric, "first_divergent_tic"); + assert_eq!(appears[0].before, "none through 274"); + assert_eq!(appears[0].after, "206 (mobj slot 1 m_momx)"); + assert_eq!(findings(&verdicts[1]), &[], "the same tic is not earlier"); + let earlier = findings(&verdicts[2]); + assert_eq!(earlier.len(), 1, "{earlier:?}"); + assert_eq!(earlier[0].against, "b2", "judged against the line before"); + } + + /// Cost is judged only against a parent measured by the same run on the + /// same machine, never against history from another runner. + #[test] + fn cost_is_judged_against_the_parent_measured_in_the_same_run() { + let same = r#""clickhouse":"26.8.2.7","runner_cpu":"EPYC","run_id":"2","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","compared_through":274"#; + let new = lines(&format!( + "{{\"commit\":\"a1\",{same},\"stage1_analysis_s\":100.0,\"tic_ms_p50\":400.0}}\n\ + {{\"commit\":\"b1\",{same},\"stage1_analysis_s\":130.0,\"tic_ms_p50\":510.0}}\n\ + {{\"commit\":\"b2\",{same},\"stage1_analysis_s\":140.0,\"tic_ms_p50\":520.0}}\n\ + {{\"commit\":\"b3\",\"clickhouse\":\"26.8.2.7\",\"runner_cpu\":\"Xeon\",\"run_id\":\"2\",\"first_refused_tic\":275,\"first_refused_bits\":\"unresolved: CHASE_STUCK\",\"compared_through\":274,\"stage1_analysis_s\":900.0,\"tic_ms_p50\":900.0}}" + )); + let verdicts = judge(&lines(HISTORY), &new, LIMITS); + assert_eq!( + verdicts[0], + Verdict::Recorded { + commit: "a1".into() + }, + "a commit the history holds is the next one's parent, not judged again" + ); + let slower = findings(&verdicts[1]); + assert_eq!(slower.len(), 1, "{slower:?}"); + assert_eq!(slower[0].kind, Kind::Cost); + assert_eq!(slower[0].metric, "stage1_analysis_s"); + assert_eq!(slower[0].after, "130.000 (1.30x)"); + assert_eq!(findings(&verdicts[2]), &[], "within both limits of b1"); + let Verdict::Judged { cost_against, .. } = &verdicts[3] else { + panic!("{verdicts:?}"); + }; + assert_eq!(*cost_against, None, "another CPU model is not a parent"); + } + + #[test] + fn an_error_line_is_reported_and_breaks_the_cost_chain() { + let same = + r#""clickhouse":"26.8.2.7","runner_cpu":"EPYC","run_id":"2","compared_through":274"#; + let new = lines(&format!( + "{{\"commit\":\"b1\",{same},\"stage1_analysis_s\":100.0}}\n\ + {{\"commit\":\"b2\",\"error\":\"the diff exited 1\"}}\n\ + {{\"commit\":\"b3\",{same},\"stage1_analysis_s\":900.0}}" + )); + let verdicts = judge(&[], &new, LIMITS); + assert_eq!( + verdicts[1], + Verdict::Error { + commit: "b2".into(), + error: "the diff exited 1".into() + } + ); + let Verdict::Judged { + against, + cost_against, + findings, + .. + } = &verdicts[2] + else { + panic!("{verdicts:?}"); + }; + assert_eq!(against.as_deref(), Some("b1")); + assert_eq!(*cost_against, None, "b3's parent errored"); + assert_eq!(findings, &[]); + } + + #[test] + fn a_line_that_compared_nothing_says_nothing_about_divergence() { + let new = lines( + r#" +{"commit":"b1","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK"} +{"commit":"b2","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","compared_through":274,"first_divergent_tic":5} +"#, + ); + let verdicts = judge(&[], &new, LIMITS); + let Verdict::Judged { against, .. } = &verdicts[0] else { + panic!("{verdicts:?}"); + }; + assert_eq!(*against, None, "an empty history judges nothing"); + assert_eq!(findings(&verdicts[1]), &[]); + } +} diff --git a/driver/tests/fixtures/regress/history.jsonl b/driver/tests/fixtures/regress/history.jsonl new file mode 100644 index 00000000..89e7c60e --- /dev/null +++ b/driver/tests/fixtures/regress/history.jsonl @@ -0,0 +1,2 @@ +{"commit":"0000000000000000000000000000000000000000","clickhouse":"26.8.2.7","runner_cpu":"AMD EPYC 7763 64-Core Processor","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","first_divergent_tic":null,"first_divergent_field":null,"compared_through":274,"compared_tics":273,"stage1_analysis_s":180.5,"stage2_analysis_s":20.1,"tic_ms_p50":540.0,"tic_ms_p95":700.0,"tics":274,"run_id":"100","error":null} +{"commit":"1111111111111111111111111111111111111111","clickhouse":"26.8.2.7","runner_cpu":"AMD EPYC 7763 64-Core Processor","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","first_divergent_tic":null,"first_divergent_field":null,"compared_through":274,"compared_tics":273,"stage1_analysis_s":182.0,"stage2_analysis_s":20.3,"tic_ms_p50":545.0,"tic_ms_p95":710.0,"tics":274,"run_id":"100","error":null} diff --git a/driver/tests/fixtures/regress/night-moved.jsonl b/driver/tests/fixtures/regress/night-moved.jsonl new file mode 100644 index 00000000..d08667f0 --- /dev/null +++ b/driver/tests/fixtures/regress/night-moved.jsonl @@ -0,0 +1,2 @@ +{"commit":"1111111111111111111111111111111111111111","clickhouse":"26.8.2.7","runner_cpu":"AMD EPYC 7763 64-Core Processor","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","first_divergent_tic":null,"first_divergent_field":null,"compared_through":274,"compared_tics":273,"stage1_analysis_s":170.0,"stage2_analysis_s":19.0,"tic_ms_p50":520.0,"tic_ms_p95":690.0,"tics":274,"run_id":"200","error":null} +{"commit":"2222222222222222222222222222222222222222","clickhouse":"26.8.2.7","runner_cpu":"AMD EPYC 7763 64-Core Processor","first_refused_tic":210,"first_refused_bits":"unresolved: PL_ACTION_NEEDED","first_divergent_tic":null,"first_divergent_field":null,"compared_through":209,"compared_tics":208,"stage1_analysis_s":175.0,"stage2_analysis_s":19.5,"tic_ms_p50":530.0,"tic_ms_p95":695.0,"tics":209,"run_id":"200","error":null} diff --git a/driver/tests/fixtures/regress/night-steady.jsonl b/driver/tests/fixtures/regress/night-steady.jsonl new file mode 100644 index 00000000..77ff63ed --- /dev/null +++ b/driver/tests/fixtures/regress/night-steady.jsonl @@ -0,0 +1,3 @@ +{"commit":"1111111111111111111111111111111111111111","clickhouse":"26.8.2.7","runner_cpu":"AMD EPYC 7763 64-Core Processor","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","first_divergent_tic":null,"first_divergent_field":null,"compared_through":274,"compared_tics":273,"stage1_analysis_s":170.0,"stage2_analysis_s":19.0,"tic_ms_p50":520.0,"tic_ms_p95":690.0,"tics":274,"run_id":"200","error":null} +{"commit":"2222222222222222222222222222222222222222","clickhouse":"26.8.2.7","runner_cpu":"AMD EPYC 7763 64-Core Processor","first_refused_tic":275,"first_refused_bits":"unresolved: CHASE_STUCK","first_divergent_tic":null,"first_divergent_field":null,"compared_through":274,"compared_tics":273,"stage1_analysis_s":175.0,"stage2_analysis_s":19.5,"tic_ms_p50":530.0,"tic_ms_p95":695.0,"tics":274,"run_id":"200","error":null} +{"commit":"3333333333333333333333333333333333333333","error":"native diff 2000 exited 1"}