Skip to content

Commit

Permalink
[crater] Use timestamp to key crater results
Browse files Browse the repository at this point in the history
We were using the fontc rev as the key to a map of runs -> detailed
results, which doesn't work because it's possible for multiple runs to
share the same git rev (e.g. if the inputs or the installed python packages
have changed)
  • Loading branch information
cmyr committed Oct 31, 2024
1 parent b0745a8 commit 3cdee1c
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
2 changes: 1 addition & 1 deletion fontc_crater/src/ci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ fn run_crater_and_save_results(args: &CiArgs) -> Result<(), Error> {
let mut prev_runs: Vec<RunSummary> = load_json_if_exists_else_default(&summary_file)?;
// todo: fontc_repo should be checked out by us, and have a known path
let fontc_rev = super::get_git_rev(None).unwrap();
let pip_freeze_sha = super::pip_freeze_sha();
let pip_freeze_sha = dbg!(super::pip_freeze_sha());
if let Some(last_run) = prev_runs.last() {
if last_run.fontc_rev == fontc_rev
&& Some(last_run.input_file.as_os_str()) == args.to_run.file_name()
Expand Down
9 changes: 5 additions & 4 deletions fontc_crater/src/ci/html.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
ttx_diff_runner::{CompilerFailure, DiffError, DiffOutput, DiffValue},
TargetId,
};
use chrono::{DateTime, Utc};
use maud::{html, Markup, PreEscaped};

use super::{DiffResults, RunSummary};
Expand All @@ -27,7 +28,7 @@ pub(super) fn generate(target_dir: &Path) -> Result<(), Error> {
let details = summary
.iter()
.flat_map(|run| match run.try_load_results(target_dir) {
Some(Ok(results)) => Some(Ok((run.fontc_rev.clone(), results))),
Some(Ok(results)) => Some(Ok((run.began, results))),
None => None,
Some(Err(e)) => Some(Err(e)),
})
Expand All @@ -41,7 +42,7 @@ pub(super) fn generate(target_dir: &Path) -> Result<(), Error> {
fn make_html(
summary: &[RunSummary],
sources: &BTreeMap<PathBuf, String>,
results: &HashMap<String, DiffResults>,
results: &HashMap<DateTime<Utc>, DiffResults>,
) -> Result<String, Error> {
let table_body = make_table_body(summary);
let css = include_str!("../../resources/style.css");
Expand All @@ -65,8 +66,8 @@ fn make_html(
};
let detailed_report = match summary {
[.., prev, current] => make_detailed_report(
results.get(&current.fontc_rev).unwrap(),
results.get(&prev.fontc_rev).unwrap(),
results.get(&current.began).unwrap(),
results.get(&prev.began).unwrap(),
sources,
),
_ => html!(),
Expand Down

0 comments on commit 3cdee1c

Please sign in to comment.