From a4705fd1864ef7072fe0334257fca9657fe401de Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Mon, 7 Apr 2025 13:24:13 -0400 Subject: [PATCH 1/2] refactor!: use `IndexMap`s instead of `BTreeMap`s for test and subtest order --- Cargo.lock | 1 + moz-webgpu-cts/Cargo.toml | 2 +- moz-webgpu-cts/src/process_reports.rs | 4 ++-- moz-webgpu-cts/src/wpt/metadata.rs | 10 +++++----- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1d788291..503414ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -452,6 +452,7 @@ checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26" dependencies = [ "equivalent", "hashbrown", + "serde", ] [[package]] diff --git a/moz-webgpu-cts/Cargo.toml b/moz-webgpu-cts/Cargo.toml index 5c83e0b3..77c39dea 100644 --- a/moz-webgpu-cts/Cargo.toml +++ b/moz-webgpu-cts/Cargo.toml @@ -24,7 +24,7 @@ enum-map = { version = "2.7.3", features = ["serde"] } enumset = { version = "1.1.3", features = ["serde"] } env_logger = { workspace = true } format = { workspace = true } -indexmap = { workspace = true } +indexmap = { workspace = true, features = ["serde"] } itertools = "0.11.0" joinery = "3.1.0" lets_find_up = "0.0.3" diff --git a/moz-webgpu-cts/src/process_reports.rs b/moz-webgpu-cts/src/process_reports.rs index 94695d23..0e89d625 100644 --- a/moz-webgpu-cts/src/process_reports.rs +++ b/moz-webgpu-cts/src/process_reports.rs @@ -46,7 +46,7 @@ where #[derive(Debug, Default)] pub(crate) struct TestEntry { pub entry: Entry, - pub subtests: BTreeMap>, + pub subtests: IndexMap>, } #[derive(Debug)] @@ -565,7 +565,7 @@ pub(crate) fn process_reports( }, )) }) - .collect::>(); + .collect::>(); Some((test_entry_path, (properties, subtests))) }, diff --git a/moz-webgpu-cts/src/wpt/metadata.rs b/moz-webgpu-cts/src/wpt/metadata.rs index 063a4649..e979232a 100644 --- a/moz-webgpu-cts/src/wpt/metadata.rs +++ b/moz-webgpu-cts/src/wpt/metadata.rs @@ -1,5 +1,4 @@ use std::{ - collections::BTreeMap, fmt::{self, Display, Formatter}, hash::Hash, }; @@ -8,6 +7,7 @@ use clap::ValueEnum; use enum_map::Enum; use enumset::EnumSetType; use format::lazy_format; +use indexmap::IndexMap; use joinery::JoinableIterator; use maybe_collapsed::MaybeCollapsed; use serde::{Deserialize, Serialize}; @@ -43,7 +43,7 @@ pub(crate) mod properties; #[derive(Clone, Debug, Default, Serialize)] pub struct File { pub properties: FileProps, - pub tests: BTreeMap, + pub tests: IndexMap, } impl File { @@ -647,7 +647,7 @@ impl ImplementationStatus { } #[derive(Debug, Default)] -pub struct Tests(BTreeMap); +pub struct Tests(IndexMap); impl<'a> metadata::Tests<'a> for Tests { type Test = Test; @@ -670,7 +670,7 @@ impl<'a> metadata::Tests<'a> for Tests { #[derive(Clone, Debug, Default, Serialize)] pub struct Test { pub properties: TestProps, - pub subtests: BTreeMap, + pub subtests: IndexMap, } #[cfg(test)] @@ -694,7 +694,7 @@ impl metadata::Test<'_> for Test { } #[derive(Default)] -pub struct Subtests(BTreeMap); +pub struct Subtests(IndexMap); impl<'a> metadata::Subtests<'a> for Subtests { type Subtest = Subtest; From c9e6119ff78847eafdcfcbaee5e4ab94c38e2c13 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Mon, 7 Apr 2025 13:26:37 -0400 Subject: [PATCH 2/2] refactor!: move(process_reports): read reports, _then_ metadata --- moz-webgpu-cts/src/process_reports.rs | 174 +++++++++++++------------- 1 file changed, 87 insertions(+), 87 deletions(-) diff --git a/moz-webgpu-cts/src/process_reports.rs b/moz-webgpu-cts/src/process_reports.rs index 0e89d625..95f10cb0 100644 --- a/moz-webgpu-cts/src/process_reports.rs +++ b/moz-webgpu-cts/src/process_reports.rs @@ -176,93 +176,6 @@ pub(crate) fn process_reports( let mut other_entries_by_test = IndexMap::, TestEntry>::default(); let old_meta_file_paths = meta_files_by_path.keys().cloned().collect::>(); - log::debug!("loading metadata for comparison to reports…"); - for (path, file) in meta_files_by_path { - let File { properties, tests } = file; - - let file_rel_path = path.strip_prefix(checkout).unwrap(); - - file_props_by_file.insert( - Utf8PathBuf::from(file_rel_path.to_str().unwrap()), - properties, - ); - - for (SectionHeader(name), test) in tests { - let Test { - properties, - subtests, - } = test; - - let test_entry_path = - match TestEntryPath::from_metadata_test(browser, file_rel_path, &name) { - Ok(ok) => ok, - Err(e) => { - log::error!("{e}"); - return Err(AlreadyReportedToCommandline); - } - }; - - let freak_out_do_nothing = - |what: &dyn Display| log::error!("hoo boy, not sure what to do yet: {what}"); - - let mut reported_dupe_already = false; - let mut dupe_err = || { - if !reported_dupe_already { - freak_out_do_nothing(&format_args!( - concat!( - "duplicate entry for {:?}", - "discarding previous entries with ", - "this and further dupes" - ), - test_entry_path - )) - } - reported_dupe_already = true; - }; - - let TestEntry { - entry: test_entry, - subtests: subtest_entries, - } = if let Some(cts_path) = cts_path(&test_entry_path) { - let entry = entries_by_cts_path.entry(cts_path).or_default(); - if let Some(_old) = entry - .metadata_path - .replace(test_entry_path.clone().into_owned()) - { - dupe_err(); - } - &mut entry.entry - } else { - other_entries_by_test - .entry(test_entry_path.clone().into_owned()) - .or_default() - }; - - let test_entry_path = &test_entry_path; - - if let Some(_old) = test_entry.meta_props.replace(properties) { - dupe_err(); - } - - for (SectionHeader(subtest_name), subtest) in subtests { - let Subtest { properties } = subtest; - let subtest_entry = subtest_entries.entry(subtest_name.clone()).or_default(); - if let Some(_old) = subtest_entry.meta_props.replace(properties) { - if !reported_dupe_already { - freak_out_do_nothing(&format_args!( - concat!( - "duplicate subtest in {:?} named {:?}, ", - "discarding previous entries with ", - "this and further dupes" - ), - test_entry_path, subtest_name - )); - } - } - } - } - } - log::debug!("gathering reported test outcomes for reconciliation with metadata…"); let (exec_reports_sender, exec_reports_receiver) = channel(); @@ -399,6 +312,93 @@ pub(crate) fn process_reports( } } + log::debug!("loading metadata for comparison to reports…"); + for (path, file) in meta_files_by_path { + let File { properties, tests } = file; + + let file_rel_path = path.strip_prefix(checkout).unwrap(); + + file_props_by_file.insert( + Utf8PathBuf::from(file_rel_path.to_str().unwrap()), + properties, + ); + + for (SectionHeader(name), test) in tests { + let Test { + properties, + subtests, + } = test; + + let test_entry_path = + match TestEntryPath::from_metadata_test(browser, file_rel_path, &name) { + Ok(ok) => ok, + Err(e) => { + log::error!("{e}"); + return Err(AlreadyReportedToCommandline); + } + }; + + let freak_out_do_nothing = + |what: &dyn Display| log::error!("hoo boy, not sure what to do yet: {what}"); + + let mut reported_dupe_already = false; + let mut dupe_err = || { + if !reported_dupe_already { + freak_out_do_nothing(&format_args!( + concat!( + "duplicate entry for {:?}", + "discarding previous entries with ", + "this and further dupes" + ), + test_entry_path + )) + } + reported_dupe_already = true; + }; + + let TestEntry { + entry: test_entry, + subtests: subtest_entries, + } = if let Some(cts_path) = cts_path(&test_entry_path) { + let entry = entries_by_cts_path.entry(cts_path).or_default(); + if let Some(_old) = entry + .metadata_path + .replace(test_entry_path.clone().into_owned()) + { + dupe_err(); + } + &mut entry.entry + } else { + other_entries_by_test + .entry(test_entry_path.clone().into_owned()) + .or_default() + }; + + let test_entry_path = &test_entry_path; + + if let Some(_old) = test_entry.meta_props.replace(properties) { + dupe_err(); + } + + for (SectionHeader(subtest_name), subtest) in subtests { + let Subtest { properties } = subtest; + let subtest_entry = subtest_entries.entry(subtest_name.clone()).or_default(); + if let Some(_old) = subtest_entry.meta_props.replace(properties) { + if !reported_dupe_already { + freak_out_do_nothing(&format_args!( + concat!( + "duplicate subtest in {:?} named {:?}, ", + "discarding previous entries with ", + "this and further dupes" + ), + test_entry_path, subtest_name + )); + } + } + } + } + } + log::debug!("metadata and reports gathered, now reconciling outcomes…"); let entries_by_cts_path = entries_by_cts_path.into_iter().map(|(_name, entry)| {