From b4dac80edf41e59b734bef8a047255950b4b84c5 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 1 Aug 2024 13:19:35 -0400 Subject: [PATCH] WIP: refactor(update_expected): hoist logic for `expected` filtering to callers of `process_reports` --- moz-webgpu-cts/src/main.rs | 15 +++-- moz-webgpu-cts/src/process_reports.rs | 57 +++++++++-------- .../process_reports/should_update_expected.rs | 62 +++++++++++++++++++ moz-webgpu-cts/src/wpt/metadata/properties.rs | 4 +- 4 files changed, 103 insertions(+), 35 deletions(-) create mode 100644 moz-webgpu-cts/src/process_reports/should_update_expected.rs diff --git a/moz-webgpu-cts/src/main.rs b/moz-webgpu-cts/src/main.rs index 182c295..f3cd9c1 100644 --- a/moz-webgpu-cts/src/main.rs +++ b/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, + 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, })?; diff --git a/moz-webgpu-cts/src/process_reports.rs b/moz-webgpu-cts/src/process_reports.rs index 787a015..ecdbdcf 100644 --- a/moz-webgpu-cts/src/process_reports.rs +++ b/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}; @@ -23,15 +23,17 @@ use crate::{ taint_subtest_timeouts_by_suspicion, wpt::{ metadata::{ - properties::{ExpandedPropertyValue, Expected}, - BuildProfile, File, FileProps, ImplementationStatus, Platform, Subtest, SubtestOutcome, - Test, TestOutcome, TestProps, + properties::{ExpandedPropertyValue, Expected, NonNormalizedPropertyValue}, + 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 where @@ -53,7 +55,7 @@ pub(crate) struct ProcessReportsArgs<'a> { pub checkout: &'a Path, pub exec_report_paths: Vec, pub preset: ReportProcessingPreset, - pub implementation_status: EnumSet, + pub should_update_expected: &'a mut dyn should_update_expected::ShouldUpdateExpected, pub meta_files_by_path: IndexMap, File>, } @@ -106,27 +108,18 @@ fn accumulate( /// For subtests, `parent_implementation_status` should be specified so the /// parent test's implementation status can be used for filtering. fn reconcile( - parent_implementation_status: Option<&ExpandedPropertyValue>, meta_props: &mut TestProps, - reported: BTreeMap>>, + reported: &NonNormalizedPropertyValue>, preset: ReportProcessingPreset, - implementation_status_filter: EnumSet, + should_update_expected: &mut dyn FnMut( + &TestProps, + &NonNormalizedPropertyValue>, + (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>) -> _ = match preset { @@ -142,8 +135,12 @@ fn reconcile( 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, + &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, + &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); diff --git a/moz-webgpu-cts/src/process_reports/should_update_expected.rs b/moz-webgpu-cts/src/process_reports/should_update_expected.rs new file mode 100644 index 0000000..b7453d6 --- /dev/null +++ b/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, + reported: &NonNormalizedPropertyValue>, + key: (Platform, BuildProfile), + ) -> bool; + fn subtest( + &mut self, + meta_props: &TestProps, + reported: &NonNormalizedPropertyValue>, + parent_meta_props: &TestProps, + key: (Platform, BuildProfile), + ) -> bool; +} + +#[derive(Debug)] +pub(crate) struct ImplementationStatusFilter { + pub allowed: EnumSet, +} + +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, + _reported: &NonNormalizedPropertyValue>, + 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, + _reported: &NonNormalizedPropertyValue>, + parent_meta_props: &TestProps, + key: (Platform, BuildProfile), + ) -> bool { + let status = meta_props + .implementation_status + .or(parent_meta_props.implementation_status) + .unwrap_or_default()[key]; + self.is_allowed(status) + } +} diff --git a/moz-webgpu-cts/src/wpt/metadata/properties.rs b/moz-webgpu-cts/src/wpt/metadata/properties.rs index 8e3eaeb..ee8bcb8 100644 --- a/moz-webgpu-cts/src/wpt/metadata/properties.rs +++ b/moz-webgpu-cts/src/wpt/metadata/properties.rs @@ -400,8 +400,10 @@ where } } -/// Data from a [`NormalizedPropertyValue`]. +/// Data from a [`NormalizedPropertyValue`]. A normalized form of [`NonNormalizedPropertyValue`]. pub type NormalizedPropertyValueData = Normalized>; /// A value that is either `V` or a set of `V`s branched on by `K`. pub type Normalized = MaybeCollapsed>; + +pub type NonNormalizedPropertyValue = BTreeMap>;