From cd7902cb14e1e66f5437ce3658cfbce633d85f7a Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 19 Oct 2023 13:58:57 -0400 Subject: [PATCH] WIP: feat(cli): add `moz-webgpu-cts adjust-exps` subcmd. --- Cargo.lock | 45 +++++- Cargo.toml | 4 +- src/bin/moz-webgpu-cts/main.rs | 210 ++++++++++++++++++++++++++-- src/bin/moz-webgpu-cts/metadata.rs | 10 +- src/bin/moz-webgpu-cts/report.rs | 211 +++++++++++++++++++++++++++++ 5 files changed, 466 insertions(+), 14 deletions(-) create mode 100644 src/bin/moz-webgpu-cts/report.rs diff --git a/Cargo.lock b/Cargo.lock index 9dd949b..5ce819f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -389,6 +389,12 @@ dependencies = [ "either", ] +[[package]] +name = "itoa" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af150ab688ff2122fcef229be89cb50dd66af9e01a4ff320cc137eecc9bacc38" + [[package]] name = "joinery" version = "3.1.0" @@ -611,6 +617,12 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "ryu" +version = "1.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ad4cc8da4ef723ed60bced201181d83791ad433213d8c24efffda1eec85d741" + [[package]] name = "same-file" version = "1.0.6" @@ -640,6 +652,17 @@ dependencies = [ "syn 2.0.31", ] +[[package]] +name = "serde_json" +version = "1.0.107" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6b420ce6e3d8bd882e9b243c6eed35dbc9a6110c9769e74b584e0d68d1f20c65" +dependencies = [ + "itoa", + "ryu", + "serde", +] + [[package]] name = "similar" version = "2.3.0" @@ -721,6 +744,15 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tardar" +version = "0.1.0" +source = "git+https://github.com/ErichDonGubler/tardar?branch=static-diags#8dddc68b9f1ad730f3a97b1819333e2a6769ccb7" +dependencies = [ + "miette", + "vec1", +] + [[package]] name = "termcolor" version = "1.2.0" @@ -801,6 +833,12 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" +[[package]] +name = "vec1" +version = "1.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2bda7c41ca331fe9a1c278a9e7ee055f4be7f5eb1c2b72f079b4ff8b5fce9d5c" + [[package]] name = "version_check" version = "0.9.4" @@ -820,14 +858,15 @@ dependencies = [ [[package]] name = "wax" version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d12a78aa0bab22d2f26ed1a96df7ab58e8a93506a3e20adb47c51a93b4e1357" +source = "git+https://github.com/ErichDonGubler/wax?branch=static-miette-diags#b606968c386f98dba23c15f681d8afdc40142b11" dependencies = [ "const_format", "itertools", + "miette", "nom", "pori", "regex", + "tardar", "thiserror", "walkdir", ] @@ -849,6 +888,8 @@ dependencies = [ "natord", "path-dsl", "regex", + "serde", + "serde_json", "thiserror", "wax", ] diff --git a/Cargo.toml b/Cargo.toml index 1f94113..2f9b433 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,8 +24,10 @@ miette = { version = "5.10.0", features = ["fancy"] } natord = "1.0.9" path-dsl = "0.6.1" regex = "1.9.5" +serde = { version = "1.0.188", features = ["derive"] } +serde_json = "1.0.107" thiserror = "1.0.49" -wax = "0.6.0" +wax = { version = "0.6.0", features = ["miette"], git = "https://github.com/ErichDonGubler/wax", branch = "static-miette-diags"} [dev-dependencies] insta = "1.33.0" diff --git a/src/bin/moz-webgpu-cts/main.rs b/src/bin/moz-webgpu-cts/main.rs index da86d79..2feb7a3 100644 --- a/src/bin/moz-webgpu-cts/main.rs +++ b/src/bin/moz-webgpu-cts/main.rs @@ -1,14 +1,18 @@ mod metadata; +mod report; -use self::metadata::{ - AnalyzeableProps, Applicability, Expectation, Platform, SubtestOutcome, Test, TestOutcome, +use self::{ + metadata::{ + AnalyzeableProps, Applicability, Expectation, Platform, SubtestOutcome, Test, TestOutcome, + }, + report::ExecutionReport, }; use std::{ collections::{BTreeMap, BTreeSet}, fmt::Display, fs, - io::{self, BufWriter}, + io::{self, BufReader, BufWriter}, path::{Path, PathBuf}, process::ExitCode, sync::Arc, @@ -16,7 +20,7 @@ use std::{ use clap::Parser; use indexmap::{IndexMap, IndexSet}; -use miette::{miette, Diagnostic, NamedSource, Report, SourceSpan, WrapErr}; +use miette::{miette, Diagnostic, IntoDiagnostic, NamedSource, Report, SourceSpan, WrapErr}; use path_dsl::path; use regex::Regex; @@ -39,6 +43,12 @@ struct Cli { #[derive(Debug, Parser)] enum Subcommand { + #[clap(name = "adjust-exps")] + AdjustExpectations { + #[clap(long = "glob", value_name = "REPORT_GLOB")] + report_globs: Vec, + report_paths: Vec, + }, #[clap(name = "fmt")] Format, Triage, @@ -111,6 +121,181 @@ fn run(cli: Cli) -> ExitCode { } match subcommand { + Subcommand::AdjustExpectations { + report_globs, + report_paths, + } => { + let report_globs = { + let mut found_glob_parse_err = false; + // TODO: I think these can just be args. directly? + let globs = report_globs + .into_iter() + .filter_map(|glob| match Glob::diagnosed(&glob) { + Ok((glob, _diagnostics)) => Some(glob.into_owned().partition()), + Err(diagnostics) => { + found_glob_parse_err = true; + let error_reports = diagnostics + .into_iter() + .filter(|diag| { + // N.B.: There should be at least one of these! + diag.severity() + .map_or(true, |sev| sev == miette::Severity::Error) + }) + .map(Report::new_boxed); + for report in error_reports { + eprintln!("{report:?}"); + } + todo!("render glob parse diagnostics in hard error") + } + }) + .collect::>(); + + if found_glob_parse_err { + log::error!("failed to parse one or more WPT report globs; bailing"); + return ExitCode::FAILURE; + } + + globs + }; + + if report_paths.is_empty() && report_globs.is_empty() { + log::error!("no report paths specified, bailing"); + return ExitCode::FAILURE; + } + + let exec_report_paths = { + let mut found_glob_walk_err = false; + let files = report_globs + .iter() + .flat_map(|(base_path, glob)| { + glob.walk(base_path) + .filter_map(|entry| match entry { + // TODO: warn when not ending in `wptreport.json` + Ok(entry) => Some(entry.into_path()), + Err(e) => { + found_glob_walk_err = true; + let ctx_msg = if let Some(path) = e.path() { + format!( + "failed to enumerate files for glob `{}` at path {}", + glob, + path.display() + ) + } else { + format!("failed to enumerate files for glob `{glob}`") + }; + let e = Report::msg(e).wrap_err(ctx_msg); + eprintln!("{e:?}"); + None + } + }) + .collect::>() // OPT: Can we get rid of this somehow? + }) + .chain(report_paths) + .collect::>(); + + if found_glob_walk_err { + log::error!("failed to enumerate files with WPT report globs; bailing"); + return ExitCode::FAILURE; + } + + files + }; + + if exec_report_paths.is_empty() { + log::error!("no WPT report files found, bailing"); + return ExitCode::FAILURE; + } + + log::trace!("working with the following WPT report files: {exec_report_paths:#?}"); + log::info!("working with {} WPT report files", exec_report_paths.len()); + + let meta_files_by_path = { + let raw_meta_files_by_path = match read_metadata() { + Ok(paths) => paths, + Err(()) => return ExitCode::FAILURE, + }; + + log::info!("parsing metadata…"); + let mut parse_err_found = false; + + let files = raw_meta_files_by_path + .into_iter() + .filter_map(|(path, file_contents)| { + match chumsky::Parser::parse(&metadata::File::parser(), &*file_contents) + .into_result() + { + Err(errors) => { + parse_err_found = true; + render_metadata_parse_errors(&path, &file_contents, errors); + None + } + Ok(file) => Some((path, file)), + } + }) + .collect::>(); + + if parse_err_found { + log::error!(concat!( + "found one or more failures while parsing metadata, ", + "see above for more details" + )); + return ExitCode::FAILURE; + } + + files + }; + + let exec_reports = { + let mut found_read_err = false; + let reports = exec_report_paths + .into_iter() + .filter_map(|path| { + let res = fs::File::open(&path) + .map(BufReader::new) + .map_err(Report::msg) + .wrap_err("failed to open file") + .and_then(|reader| { + serde_json::from_reader(reader) + .into_diagnostic() + .wrap_err("failed to parse JSON") + }) + .wrap_err_with(|| { + format!( + "failed to read WPT execution report from {}", + path.display() + ) + }); + match res { + Ok(parsed) => Some((path, parsed)), + Err(e) => { + found_read_err = true; + log::error!("{e:?}"); + None + } + } + }) + .collect::>(); + + if found_read_err { + log::error!("failed to read one or more"); + return ExitCode::FAILURE; + } + + reports + }; + + // TODO: What if there are no reports? + + log::info!("printing report…"); + println!("{:#?}", exec_reports); + + log::info!("comparing report(s) results to metadata…"); + + todo!(concat!( + "compare reports with expectations, ", + "write out new metadata", + )); + } Subcommand::Format => { let raw_test_files_by_path = match read_metadata() { Ok(paths) => paths, @@ -221,7 +406,7 @@ fn run(cli: Cli) -> ExitCode { #[derive(Clone, Debug, Default)] struct PerPlatformAnalysis { tests_with_runner_errors: BTreeSet>, - tests_with_disabled: BTreeSet>, + tests_with_disabled_or_skip: BTreeSet>, tests_with_crashes: BTreeSet>, subtests_with_failures_by_test: BTreeMap, IndexSet>>, @@ -308,7 +493,9 @@ fn run(cli: Cli) -> ExitCode { if is_disabled { analysis.for_each_platform_mut(|analysis| { - analysis.tests_with_disabled.insert(test_name.clone()); + analysis + .tests_with_disabled_or_skip + .insert(test_name.clone()); }) } @@ -340,6 +527,11 @@ fn run(cli: Cli) -> ExitCode { TestOutcome::Error => receiver(&mut |analysis| { analysis.tests_with_runner_errors.insert(test_name.clone()); }), + TestOutcome::Skip => receiver(&mut |analysis| { + analysis + .tests_with_disabled_or_skip + .insert(test_name.clone()); + }), } } @@ -396,7 +588,7 @@ fn run(cli: Cli) -> ExitCode { if is_disabled { analysis .windows - .tests_with_disabled + .tests_with_disabled_or_skip .insert(test_name.clone()); } @@ -482,7 +674,7 @@ fn run(cli: Cli) -> ExitCode { analysis.for_each_platform(|platform, analysis| { let PerPlatformAnalysis { tests_with_runner_errors, - tests_with_disabled, + tests_with_disabled_or_skip: tests_with_disabled, tests_with_crashes, subtests_with_failures_by_test, subtests_with_timeouts_by_test, @@ -586,7 +778,7 @@ fn read_gecko_files_at( None => &"", }; log::error!( - "failed to enumerate `cts.https.html` files{}\n caused by: {e}", + "failed to enumerate {glob_pattern:?} files{}\n caused by: {e}", path_disp ); found_read_err = true; diff --git a/src/bin/moz-webgpu-cts/metadata.rs b/src/bin/moz-webgpu-cts/metadata.rs index 574ccf6..e0248f9 100644 --- a/src/bin/moz-webgpu-cts/metadata.rs +++ b/src/bin/moz-webgpu-cts/metadata.rs @@ -13,6 +13,7 @@ use chumsky::{ use format::lazy_format; use indexmap::IndexSet; use joinery::JoinableIterator; +use serde::Deserialize; use whippit::metadata::{ self, properties::{ @@ -407,12 +408,14 @@ where } } -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq)] +#[serde(rename_all = "UPPERCASE")] pub enum TestOutcome { Ok, Timeout, Crash, Error, + Skip, } impl Default for TestOutcome { @@ -431,6 +434,7 @@ impl Display for TestOutcome { Self::Timeout => "TIMEOUT", Self::Crash => "CRASH", Self::Error => "ERROR", + Self::Skip => "SKIP", } ) } @@ -448,6 +452,7 @@ impl<'a> Properties<'a> for AnalyzeableProps { keyword("CRASH").to(TestOutcome::Crash), keyword("TIMEOUT").to(TestOutcome::Timeout), keyword("ERROR").to(TestOutcome::Error), + keyword("SKIP").to(TestOutcome::Skip), )), ) .boxed() @@ -462,7 +467,8 @@ impl<'a> Properties<'a> for AnalyzeableProps { } } -#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq)] +#[serde(rename_all = "UPPERCASE")] pub enum SubtestOutcome { Pass, Fail, diff --git a/src/bin/moz-webgpu-cts/report.rs b/src/bin/moz-webgpu-cts/report.rs new file mode 100644 index 0000000..89a60ee --- /dev/null +++ b/src/bin/moz-webgpu-cts/report.rs @@ -0,0 +1,211 @@ +use std::{ + ffi::OsStr, + path::{self, Path}, +}; + +use serde::{ + de::{Deserializer, Error}, + Deserialize, +}; + +use crate::metadata::{BuildProfile, Platform, SubtestOutcome, TestOutcome}; + +#[derive(Debug, Deserialize)] +pub(crate) struct ExecutionReport { + pub run_info: RunInfo, + pub results: Vec, +} + +#[derive(Debug)] +pub(crate) struct RunInfo { + pub platform: Platform, + pub build_profile: BuildProfile, +} + +impl<'de> Deserialize<'de> for RunInfo { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + #[derive(Debug, Deserialize)] + struct ActualRunInfo { + os: String, + processor: String, + win11_2009: bool, + debug: bool, + } + + let ActualRunInfo { + os, + processor, + win11_2009, + debug, + } = ActualRunInfo::deserialize(deserializer)?; + + let platform = match &*os { + "win" => { + if processor == "x86_64" && win11_2009 { + Platform::Windows + } else { + return Err(D::Error::custom("asdf")); + } + } + // TODO: Make sure we're not missing any important fields to validate for + // Mac + "mac" => Platform::MacOs, + // TODO: Make sure we're not missing any important fields to validate for + // Linux + "linux" => Platform::Linux, + other => return Err(D::Error::custom(format!("unrecognized platform {other:?}"))), + }; + + let build_profile = if debug { + BuildProfile::Debug + } else { + BuildProfile::Optimized + }; + + Ok(RunInfo { + platform, + build_profile, + }) + } +} + +#[derive(Debug, Deserialize)] +pub(crate) struct TestExecutionEntry { + #[serde(rename = "test")] + pub test_name: TestEntryName, + #[serde(flatten)] + pub result: TestExecutionResult, +} + +// TODO: make sure this can only be a valid test path +#[derive(Debug, Deserialize)] +pub(crate) struct TestEntryName(pub String); + +impl TestEntryName { + pub fn matches_metadata(&self, rel_meta_path: &Path, test_name: &str) -> bool { + let Self(test_run_path) = self; + + if rel_meta_path.extension() != Some(OsStr::new("ini")) { + return false; + } + + let rel_meta_path = rel_meta_path.with_extension(""); + + let (test_run_path, meta_prefix): (_, &[_]) = + if let Some(stripped) = test_run_path.strip_prefix("/_mozilla") { + (stripped, &["testing", "web-platform", "mozilla", "meta"]) + } else { + (test_run_path, &["testing", "web-platform", "meta"]) + }; + + let mut test_run_path_components = test_run_path.strip_prefix('/').unwrap().split('/'); + let (expected_test_run_file_name, expected_test_name) = + match test_run_path_components.next_back() { + Some(path_and_maybe_variants) => match path_and_maybe_variants.find('?') { + Some(query_params_start_idx) => ( + &path_and_maybe_variants[..query_params_start_idx], + path_and_maybe_variants, + ), + None => (path_and_maybe_variants, path_and_maybe_variants), + }, + None => return false, + }; + let mut test_run_path_components = meta_prefix + .iter() + .copied() + .chain(test_run_path_components) + .chain([expected_test_run_file_name]); + + let mut rel_meta_path_components = rel_meta_path.components(); + + test_run_path_components.all(|c1| { + rel_meta_path_components.next().map_or( + false, + |c| matches!(c, path::Component::Normal(c2) if c2.to_str() == Some(c1)), + ) + }) && expected_test_name == test_name + } +} + +#[test] +fn report_meta_match() { + macro_rules! assert_test_matches_meta { + ($test_run_path:expr, $rel_meta_path:expr, $test_section_header:expr) => { + assert!(TestEntryName($test_run_path.to_owned()) + .matches_metadata(Path::new($rel_meta_path), $test_section_header)) + }; + } + + assert_test_matches_meta!( + "/_mozilla/blarg/cts.https.html?stuff=things", + "testing/web-platform/mozilla/meta/blarg/cts.https.html.ini", + "cts.https.html?stuff=things" + ); + + assert_test_matches_meta!( + "/blarg/cts.https.html?stuff=things", + "testing/web-platform/meta/blarg/cts.https.html.ini", + "cts.https.html?stuff=things" + ); +} + +#[test] +fn report_meta_reject() { + macro_rules! assert_test_rejects_meta { + ($test_run_path:expr, $rel_meta_path:expr, $test_section_header:expr) => { + assert!(!TestEntryName($test_run_path.to_owned()) + .matches_metadata(Path::new($rel_meta_path), $test_section_header)) + }; + } + + assert_test_rejects_meta!( + "/blarg/cts.https.html?stuff=things", + // Wrong: the `mozilla` component shouldn't be after `web-platform` + "testing/web-platform/mozilla/meta/blarg/cts.https.html.ini", + "cts.https.html?stuff=things" + ); + + assert_test_rejects_meta!( + "/_mozilla/blarg/cts.https.html?stuff=things", + // Wrong: missing the `mozilla` component after `web-platform` + "testing/web-platform/meta/blarg/cts.https.html.ini", + "cts.https.html?stuff=things" + ); +} + +#[derive(Debug, Deserialize)] +#[serde(untagged)] +pub(crate) enum TestExecutionResult { + Complete { + duration: TextExecutionDuration, + #[serde(rename = "status")] + outcome: TestOutcome, + subtests: Vec, + }, + JobMaybeTimedOut { + status: String, + // TODO: It'd be nice to validate that `status` is still there (but set to + // `""`) without keeping them around. 🤔 Custom `Deserialize` impl. or + // a mono-state structure, I suppose? + subtests: Vec, + }, +} + +#[derive(Debug, Deserialize)] +#[serde(untagged)] +pub(crate) enum TextExecutionDuration { + Valid(u64), + /// Only a thing because of + Invalid(f64), +} + +#[derive(Debug, Deserialize)] +pub(crate) struct SubtestExecutionResult { + #[serde(rename = "name")] + subtest_name: String, + #[serde(rename = "status")] + outcome: SubtestOutcome, +}