Write tests in report order after processing reports (#163)

When we process reports, populate test entries in `IndexMaps`, instead
of `BTreeMap`s. Then, populate them from reports _before_ they get
populated from metadata. That way, when we iterate over them to write
back to disk, we do so in the order that tests were reported.

Author: ErichDonGubler

Cargo.lock

@@ -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"

moz-webgpu-cts/Cargo.toml

@@ -46,7 +46,7 @@ where
 #[derive(Debug, Default)]
 pub(crate) struct TestEntry {
     pub entry: Entry<TestOutcome>,
-    pub subtests: BTreeMap<String, Entry<SubtestOutcome>>,
+    pub subtests: IndexMap<String, Entry<SubtestOutcome>>,
 }
 
 #[derive(Debug)]
@@ -176,93 +176,6 @@ pub(crate) fn process_reports(
     let mut other_entries_by_test = IndexMap::<TestEntryPath<'_>, TestEntry>::default();
     let old_meta_file_paths = meta_files_by_path.keys().cloned().collect::<Vec<_>>();
 
-    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)| {
@@ -565,7 +565,7 @@ pub(crate) fn process_reports(
                         },
                     ))
                 })
-                .collect::<BTreeMap<_, _>>();
+                .collect::<IndexMap<_, _>>();
 
             Some((test_entry_path, (properties, subtests)))
         },

moz-webgpu-cts/src/process_reports.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<SectionHeader, Test>,
+    pub tests: IndexMap<SectionHeader, Test>,
 }
 
 impl File {
@@ -647,7 +647,7 @@ impl ImplementationStatus {
 }
 
 #[derive(Debug, Default)]
-pub struct Tests(BTreeMap<SectionHeader, Test>);
+pub struct Tests(IndexMap<SectionHeader, Test>);
 
 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<TestOutcome>,
-    pub subtests: BTreeMap<SectionHeader, Subtest>,
+    pub subtests: IndexMap<SectionHeader, Subtest>,
 }
 
 #[cfg(test)]
@@ -694,7 +694,7 @@ impl metadata::Test<'_> for Test {
 }
 
 #[derive(Default)]
-pub struct Subtests(BTreeMap<SectionHeader, Subtest>);
+pub struct Subtests(IndexMap<SectionHeader, Subtest>);
 
 impl<'a> metadata::Subtests<'a> for Subtests {
     type Subtest = Subtest;