refactor(update_expected): hoist logic for expected filtering to ca…

…llers of `process_reports`

moz-webgpu-cts/src/main.rs

@@ -37,7 +37,10 @@ use itertools::Itertools;
 use joinery::JoinableIterator;
 use miette::{miette, Diagnostic, IntoDiagnostic, NamedSource, Report, SourceSpan, WrapErr};
 use path_dsl::path;
-use process_reports::ProcessReportsArgs;
+use process_reports::{
+    should_update_expected::{self, ShouldUpdateExpected},
+    ProcessReportsArgs,
+};
 use wax::Glob;
 use whippit::{
     metadata::SectionHeader,
@@ -294,7 +297,7 @@ fn run(cli: Cli) -> ExitCode {
             preset,
             implementation_status,
         } => {
-            let implementation_status = if implementation_status.is_empty() {
+            let allowed_implementation_statuses = if implementation_status.is_empty() {
                 ImplementationStatus::default().into()
             } else {
                 EnumSet::from_iter(implementation_status)
@@ -304,7 +307,9 @@ fn run(cli: Cli) -> ExitCode {
                 &checkout,
                 exec_report_spec,
                 preset.into(),
-                implementation_status,
+                &mut should_update_expected::ImplementationStatusFilter {
+                    allowed: allowed_implementation_statuses,
+                },
             ) {
                 Ok(()) => ExitCode::SUCCESS,
                 Err(AlreadyReportedToCommandline) => ExitCode::FAILURE,
@@ -1312,7 +1317,7 @@ fn process_reports(
     checkout: &Path,
     exec_report_spec: ExecReportSpec,
     preset: process_reports::ReportProcessingPreset,
-    implementation_status: EnumSet<ImplementationStatus>,
+    should_update_expected: &mut dyn ShouldUpdateExpected,
 ) -> Result<(), AlreadyReportedToCommandline> {
     let exec_report_paths = exec_report_spec.paths()?;
 
@@ -1324,7 +1329,7 @@ fn process_reports(
         checkout,
         exec_report_paths,
         preset,
-        implementation_status,
+        should_update_expected,
         meta_files_by_path,
     })?;
 

moz-webgpu-cts/src/process_reports.rs

@@ -9,7 +9,7 @@ use std::{
 };
 
 use camino::Utf8PathBuf;
-use enumset::{EnumSet, EnumSetType};
+use enumset::EnumSetType;
 use format::lazy_format;
 use indexmap::IndexMap;
 use miette::{IntoDiagnostic, Report, WrapErr};
@@ -24,14 +24,16 @@ use crate::{
     wpt::{
         metadata::{
             properties::{ExpandedPropertyValue, Expected, NonNormalizedPropertyValue},
-            BuildProfile, File, FileProps, ImplementationStatus, Platform, Subtest, SubtestOutcome,
-            Test, TestOutcome, TestProps,
+            BuildProfile, File, FileProps, Platform, Subtest, SubtestOutcome, Test, TestOutcome,
+            TestProps,
         },
         path::{Browser, TestEntryPath},
     },
     AlreadyReportedToCommandline,
 };
 
+pub(crate) mod should_update_expected;
+
 #[derive(Debug, Default)]
 pub(crate) struct Entry<Out>
 where
@@ -53,7 +55,7 @@ pub(crate) struct ProcessReportsArgs<'a> {
     pub checkout: &'a Path,
     pub exec_report_paths: Vec<PathBuf>,
     pub preset: ReportProcessingPreset,
-    pub implementation_status: EnumSet<ImplementationStatus>,
+    pub should_update_expected: &'a mut dyn should_update_expected::ShouldUpdateExpected,
     pub meta_files_by_path: IndexMap<Arc<PathBuf>, File>,
 }
 
@@ -106,27 +108,18 @@ fn accumulate<Out>(
 /// For subtests, `parent_implementation_status` should be specified so the
 /// parent test's implementation status can be used for filtering.
 fn reconcile<Out>(
-    parent_implementation_status: Option<&ExpandedPropertyValue<ImplementationStatus>>,
     meta_props: &mut TestProps<Out>,
     reported: &NonNormalizedPropertyValue<Expected<Out>>,
     preset: ReportProcessingPreset,
-    implementation_status_filter: EnumSet<ImplementationStatus>,
+    should_update_expected: &mut dyn FnMut(
+        &TestProps<Out>,
+        &NonNormalizedPropertyValue<Expected<Out>>,
+        (Platform, BuildProfile),
+    ) -> bool,
 ) where
     Out: Debug + Default + EnumSetType,
 {
-    let implementation_status = meta_props
-        .implementation_status
-        .or(parent_implementation_status.cloned())
-        .unwrap_or_default();
-    let should_apply_changes =
-        |key| implementation_status_filter.contains(implementation_status[key]);
     let reconciled = {
-        let reported = |(platform, build_profile)| {
-            reported
-                .get(&platform)
-                .and_then(|rep| rep.get(&build_profile))
-                .copied()
-        };
         let meta_expected = meta_props.expected.unwrap_or_default();
 
         let resolve: fn(Expected<_>, Option<Expected<_>>) -> _ = match preset {
@@ -142,8 +135,12 @@ fn reconcile<Out>(
 
         ExpandedPropertyValue::from_query(|platform, build_profile| {
             let key = (platform, build_profile);
-            if should_apply_changes(key) {
-                resolve(meta_expected[key], reported(key))
+            if should_update_expected(meta_props, reported, key) {
+                let reported = reported
+                    .get(&platform)
+                    .and_then(|rep| rep.get(&build_profile))
+                    .copied();
+                resolve(meta_expected[key], reported)
             } else {
                 meta_expected[key]
             }
@@ -160,7 +157,7 @@ pub(crate) fn process_reports(
         checkout,
         exec_report_paths,
         preset,
-        implementation_status,
+        should_update_expected,
         meta_files_by_path,
     } = args;
 
@@ -490,11 +487,12 @@ pub(crate) fn process_reports(
             }
 
             reconcile(
-                None,
                 &mut properties,
                 &test_reported,
                 preset,
-                implementation_status,
+                &mut |meta_props, reported, key| {
+                    should_update_expected.test(meta_props, reported, key)
+                },
             );
 
             let subtests = subtest_entries
@@ -527,11 +525,12 @@ pub(crate) fn process_reports(
 
                     let mut subtest_properties = subtest_properties.unwrap_or_default();
                     reconcile(
-                        properties.implementation_status.as_ref(),
                         &mut subtest_properties,
                         &subtest_reported,
                         preset,
-                        implementation_status,
+                        &mut |meta_props, reported, key| {
+                            should_update_expected.subtest(meta_props, reported, &properties, key)
+                        },
                     );
                     for (_, expected) in subtest_properties.expected.as_mut().unwrap().iter_mut() {
                         taint_subtest_timeouts_by_suspicion(expected);

moz-webgpu-cts/src/process_reports/should_update_expected.rs

@@ -0,0 +1,62 @@
+use std::fmt::Debug;
+
+use enumset::EnumSet;
+
+use crate::wpt::metadata::{
+    properties::{Expected, NonNormalizedPropertyValue},
+    BuildProfile, ImplementationStatus, Platform, SubtestOutcome, TestOutcome, TestProps,
+};
+
+pub(crate) trait ShouldUpdateExpected: Debug {
+    fn test(
+        &mut self,
+        meta_props: &TestProps<TestOutcome>,
+        reported: &NonNormalizedPropertyValue<Expected<TestOutcome>>,
+        key: (Platform, BuildProfile),
+    ) -> bool;
+    fn subtest(
+        &mut self,
+        meta_props: &TestProps<SubtestOutcome>,
+        reported: &NonNormalizedPropertyValue<Expected<SubtestOutcome>>,
+        parent_meta_props: &TestProps<TestOutcome>,
+        key: (Platform, BuildProfile),
+    ) -> bool;
+}
+
+#[derive(Debug)]
+pub(crate) struct ImplementationStatusFilter {
+    pub allowed: EnumSet<ImplementationStatus>,
+}
+
+impl ImplementationStatusFilter {
+    fn is_allowed(&self, implementation_status: ImplementationStatus) -> bool {
+        let Self { allowed } = self;
+        allowed.contains(implementation_status)
+    }
+}
+
+impl ShouldUpdateExpected for ImplementationStatusFilter {
+    fn test(
+        &mut self,
+        meta_props: &TestProps<TestOutcome>,
+        _reported: &NonNormalizedPropertyValue<Expected<TestOutcome>>,
+        key: (Platform, BuildProfile),
+    ) -> bool {
+        let status = meta_props.implementation_status.unwrap_or_default()[key];
+        self.is_allowed(status)
+    }
+
+    fn subtest(
+        &mut self,
+        meta_props: &TestProps<SubtestOutcome>,
+        _reported: &NonNormalizedPropertyValue<Expected<SubtestOutcome>>,
+        parent_meta_props: &TestProps<TestOutcome>,
+        key: (Platform, BuildProfile),
+    ) -> bool {
+        let status = meta_props
+            .implementation_status
+            .or(parent_meta_props.implementation_status)
+            .unwrap_or_default()[key];
+        self.is_allowed(status)
+    }
+}