WIP: fix(process_reports): skip report entries with SKIP test outcomes

erichdongubler-mozilla · Apr 16, 2024 · b64a6c0 · b64a6c0
1 parent c884d69
commit b64a6c0
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 23 deletions.
diff --git a/moz-webgpu-cts/src/main.rs b/moz-webgpu-cts/src/main.rs
@@ -8,7 +8,7 @@ use self::{
         BuildProfile, File, FileProps, Platform, Subtest, SubtestOutcome, Test, TestOutcome,
         TestProps,
     },
-    process_reports::{Entry, TestEntry},
+    process_reports::{Entry, MaybeSkipped, TestEntry},
     report::{
         ExecutionReport, RunInfo, SubtestExecutionResult, TestExecutionEntry, TestExecutionResult,
     },
@@ -477,6 +477,24 @@ fn run(cli: Cli) -> ExitCode {
                     let TestExecutionEntry { test_name, result } = entry;
 
                     let test_path = TestPath::from_execution_report(&test_name).unwrap();
+
+                    let (reported_outcome, reported_subtests) = match result {
+                        TestExecutionResult::Complete { outcome, subtests } => (outcome, subtests),
+                        TestExecutionResult::JobMaybeTimedOut { status, subtests } => {
+                            if !status.is_empty() {
+                                log::warn!(
+                                    concat!(
+                                        "expected an empty `status` field for {:?}, ",
+                                        "but found the {:?} status"
+                                    ),
+                                    test_path,
+                                    status,
+                                )
+                            }
+                            (TestOutcome::Timeout, subtests)
+                        }
+                    };
+
                     let TestEntry {
                         entry: test_entry,
                         subtests: subtest_entries,
@@ -507,40 +525,35 @@ fn run(cli: Cli) -> ExitCode {
                             .or_default()
                     };
 
-                    let (reported_outcome, reported_subtests) = match result {
-                        TestExecutionResult::Complete { outcome, subtests } => (outcome, subtests),
-                        TestExecutionResult::JobMaybeTimedOut { status, subtests } => {
-                            if !status.is_empty() {
-                                log::warn!(
-                                    concat!(
-                                        "expected an empty `status` field for {:?}, ",
-                                        "but found the {:?} status"
-                                    ),
-                                    test_path,
-                                    status,
-                                )
-                            }
-                            (TestOutcome::Timeout, subtests)
-                        }
-                    };
-
                     fn accumulate<Out>(
-                        recorded: &mut BTreeMap<Platform, BTreeMap<BuildProfile, Expectation<Out>>>,
+                        recorded: &mut BTreeMap<
+                            Platform,
+                            BTreeMap<BuildProfile, MaybeSkipped<Expectation<Out>>>,
+                        >,
                         platform: Platform,
                         build_profile: BuildProfile,
-                        reported_outcome: Out,
+                        reported_outcome: MaybeSkipped<Out>,
                     ) where
                         Out: Default + EnumSetType + Hash,
                     {
+                        let reported_outcome = reported_outcome.map(Expectation::permanent);
                         match recorded.entry(platform).or_default().entry(build_profile) {
                             std::collections::btree_map::Entry::Vacant(entry) => {
-                                entry.insert(Expectation::permanent(reported_outcome));
+                                entry.insert(reported_outcome);
                             }
                             std::collections::btree_map::Entry::Occupied(mut entry) => {
                                 *entry.get_mut() |= reported_outcome
                             }
                         }
                     }
+
+                    let test_skipped = reported_outcome == TestOutcome::Skip;
+                    let reported_outcome = if test_skipped {
+                        MaybeSkipped::Skipped
+                    } else {
+                        MaybeSkipped::ButItWasnt(reported_outcome)
+                    };
+
                     accumulate(
                         &mut test_entry.reported,
                         platform,
@@ -554,6 +567,16 @@ fn run(cli: Cli) -> ExitCode {
                             outcome,
                         } = reported_subtest;
 
+                        let outcome = if test_skipped {
+                            if outcome == SubtestOutcome::NotRun {
+                                MaybeSkipped::Skipped
+                            } else {
+                                panic!("wtf") // TODO
+                            }
+                        } else {
+                            MaybeSkipped::ButItWasnt(outcome)
+                        };
+
                         accumulate(
                             &mut subtest_entries
                                 .entry(subtest_name.clone())
@@ -675,6 +698,7 @@ fn run(cli: Cli) -> ExitCode {
                         log::info!("new test entry: {test_path:?}")
                     }
 
+                    // TODO: Perhaps we should ignore skipped stuff?
                     if test_entry.reported.is_empty() && using_reports {
                         let test_path = &test_path;
                         let msg = lazy_format!("no entries found in reports for {:?}", test_path);

diff --git a/moz-webgpu-cts/src/process_reports.rs b/moz-webgpu-cts/src/process_reports.rs
@@ -1,6 +1,10 @@
-use std::collections::BTreeMap;
+use std::{
+    collections::BTreeMap,
+    ops::{BitOr, BitOrAssign},
+};
 
 use enumset::EnumSetType;
+use whippit::reexport::chumsky::util::Maybe;
 
 use crate::{
     metadata::{BuildProfile, Platform, SubtestOutcome, TestOutcome, TestProps},
@@ -13,7 +17,40 @@ where
     Out: EnumSetType,
 {
     pub meta_props: Option<TestProps<Out>>,
-    pub reported: BTreeMap<Platform, BTreeMap<BuildProfile, Expectation<Out>>>,
+    pub reported: BTreeMap<Platform, BTreeMap<BuildProfile, MaybeSkipped<Expectation<Out>>>>,
+}
+
+#[derive(Clone, Copy, Debug)]
+pub(crate) enum MaybeSkipped<T> {
+    ButItWasnt(T),
+    Skipped,
+}
+
+impl<T> MaybeSkipped<T> {
+    pub fn map<U, F>(self, f: F) -> MaybeSkipped<U>
+    where
+        F: FnOnce(T) -> U,
+    {
+        match self {
+            Self::ButItWasnt(t) => MaybeSkipped::ButItWasnt(f(t)),
+            Self::Skipped => MaybeSkipped::Skipped,
+        }
+    }
+}
+
+impl<T> BitOrAssign for MaybeSkipped<T>
+where
+    T: BitOr<Output = T>,
+{
+    fn bitor_assign(&mut self, rhs: Self) {
+        *self = match (self, rhs) {
+            (MaybeSkipped::Skipped, rhs) => rhs,
+            (MaybeSkipped::ButItWasnt(lhs), MaybeSkipped::ButItWasnt(rhs)) => {
+                MaybeSkipped::ButItWasnt(*lhs | rhs)
+            }
+            (_, MaybeSkipped::Skipped) => return,
+        }
+    }
 }
 
 #[derive(Debug, Default)]