From fa8c3ec6e2638b89d329a7390b0525b9e9b4da6f Mon Sep 17 00:00:00 2001
From: Erich Gubler <erichdongubler@gmail.com>
Date: Sat, 29 Mar 2025 18:19:30 -0400
Subject: [PATCH] feat(process_reports): add `--on-skip-only`

---
 moz-webgpu-cts/src/main.rs            | 26 ++++++++++++++++++++++++++
 moz-webgpu-cts/src/process_reports.rs | 18 ++++++++++++++++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/moz-webgpu-cts/src/main.rs b/moz-webgpu-cts/src/main.rs
index a6694386..a5f94fe7 100644
--- a/moz-webgpu-cts/src/main.rs
+++ b/moz-webgpu-cts/src/main.rs
@@ -122,6 +122,9 @@ enum Subcommand {
         /// `implementation-status`es that changes should be applied to.
         #[clap(value_enum, long, default_value = "backlog")]
         implementation_status: Vec<ImplementationStatus>,
+        /// What do when only `SKIP` outcomes are found for tests and subtests.
+        #[clap(value_enum, long, default_value_t = OnSkipOnly::Reconcile)]
+        on_skip_only: OnSkipOnly,
     },
     /// Parse test metadata, apply automated fixups, and re-emit it in normalized form.
     #[clap(name = "fixup", alias = "fmt")]
@@ -196,6 +199,24 @@ impl From<UpdateExpectedPreset> for process_reports::ReportProcessingPreset {
     }
 }
 
+/// See [`Subcommand::UpdateExpected::on_skip_only`].
+#[derive(Clone, Copy, Debug, ValueEnum)]
+pub(crate) enum OnSkipOnly {
+    /// Use reconcilation from the provided `--preset` with `SKIP` outcomes.
+    Reconcile,
+    /// Do not change metadata.
+    Ignore,
+}
+
+impl From<OnSkipOnly> for process_reports::OnSkipOnly {
+    fn from(value: OnSkipOnly) -> Self {
+        match value {
+            OnSkipOnly::Ignore => Self::Ignore,
+            OnSkipOnly::Reconcile => Self::Reconcile,
+        }
+    }
+}
+
 #[derive(Clone, Copy, Debug, Default, ValueEnum)]
 enum OnZeroItem {
     Show,
@@ -263,6 +284,7 @@ fn run(cli: Cli) -> ExitCode {
             exec_report_spec,
             process_reports::ReportProcessingPreset::MigrateTestStructure,
             &mut should_update_expected::NeverUpdateExpected,
+            OnSkipOnly::Reconcile.into(),
         ) {
             Ok(()) => ExitCode::SUCCESS,
             Err(AlreadyReportedToCommandline) => ExitCode::FAILURE,
@@ -271,6 +293,7 @@ fn run(cli: Cli) -> ExitCode {
             exec_report_spec,
             preset,
             implementation_status,
+            on_skip_only,
         } => {
             assert!(
                 !implementation_status.is_empty(),
@@ -288,6 +311,7 @@ fn run(cli: Cli) -> ExitCode {
                 &mut should_update_expected::ImplementationStatusFilter {
                     allowed: allowed_implementation_statuses,
                 },
+                on_skip_only.into(),
             ) {
                 Ok(()) => ExitCode::SUCCESS,
                 Err(AlreadyReportedToCommandline) => ExitCode::FAILURE,
@@ -1322,6 +1346,7 @@ fn process_reports(
     exec_report_spec: ExecReportSpec,
     preset: process_reports::ReportProcessingPreset,
     should_update_expected: &mut dyn ShouldUpdateExpected,
+    on_skip_only: process_reports::OnSkipOnly,
 ) -> Result<(), AlreadyReportedToCommandline> {
     let exec_report_paths = exec_report_spec.paths()?;
 
@@ -1335,6 +1360,7 @@ fn process_reports(
         preset,
         should_update_expected,
         meta_files_by_path,
+        on_skip_only,
     })?;
 
     log::debug!("processing complete, writing new metadata to file system…");
diff --git a/moz-webgpu-cts/src/process_reports.rs b/moz-webgpu-cts/src/process_reports.rs
index 95f10cb0..b858d2cf 100644
--- a/moz-webgpu-cts/src/process_reports.rs
+++ b/moz-webgpu-cts/src/process_reports.rs
@@ -57,6 +57,7 @@ pub(crate) struct ProcessReportsArgs<'a> {
     pub preset: ReportProcessingPreset,
     pub should_update_expected: &'a mut dyn should_update_expected::ShouldUpdateExpected,
     pub meta_files_by_path: IndexMap<Arc<PathBuf>, File>,
+    pub on_skip_only: OnSkipOnly,
 }
 
 #[derive(Clone, Copy, Debug)]
@@ -67,6 +68,14 @@ pub(crate) enum ReportProcessingPreset {
     MigrateTestStructure,
 }
 
+#[derive(Clone, Copy, Debug)]
+pub(crate) enum OnSkipOnly {
+    /// Do not change metadata.
+    Ignore,
+    /// Apply reconcilation with `SKIP` outcomes.
+    Reconcile,
+}
+
 #[derive(Debug, Default)]
 struct EntryByCtsPath<'a> {
     metadata_path: Option<TestEntryPath<'a>>,
@@ -161,6 +170,7 @@ pub(crate) fn process_reports(
         preset,
         should_update_expected,
         meta_files_by_path,
+        on_skip_only,
     } = args;
 
     if exec_report_paths.is_empty() {
@@ -469,6 +479,7 @@ pub(crate) fn process_reports(
 
             let mut properties = properties.unwrap_or_default();
 
+            let skip = TestOutcome::Skip;
             for (platform, build_profile, reported) in
                 test_reported.iter_mut().flat_map(|(p, by_bp)| {
                     by_bp
@@ -476,7 +487,6 @@ pub(crate) fn process_reports(
                         .map(move |(bp, reported)| (p, bp, reported))
                 })
             {
-                let skip = TestOutcome::Skip;
                 // Ignore `SKIP` outcomes if we have non-`SKIP` outcomes here.
                 //
                 // Do this so that test runs whose coverage _in aggregate_ includes actual
@@ -509,7 +519,11 @@ pub(crate) fn process_reports(
                 test_reported,
                 preset,
                 &mut |meta_props, reported, key| {
-                    should_update_expected.test(meta_props, reported, key)
+                    let (platform, build_profile) = key;
+                    (match on_skip_only {
+                        OnSkipOnly::Ignore => reported[&platform][&build_profile] != skip,
+                        OnSkipOnly::Reconcile => true,
+                    }) && should_update_expected.test(meta_props, reported, key)
                 },
             );