Skip to content

Commit

Permalink
Merge pull request #32 from ErichDonGubler/fixups
Browse files Browse the repository at this point in the history 
Assorted fixups and refactors 😅
  • Loading branch information
@ErichDonGubler
ErichDonGubler authored Oct 27, 2023
2 parents f0bcc5e + a5758f8 commit dac82d5
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 53 deletions.
116 changes: 73 additions & 43 deletions src/bin/moz-webgpu-cts/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use miette::{miette, Diagnostic, NamedSource, Report, SourceSpan, WrapErr};
use path_dsl::path;

use regex::Regex;
use wax::Glob;
use whippit::{
metadata::{
properties::{ConditionalValue, PropertyValue},
Expand Down Expand Up @@ -63,10 +64,10 @@ fn run(cli: Cli) -> ExitCode {
.unwrap_or_else(search_for_moz_central_ckt)
{
Ok(ckt_path) => ckt_path,
Err(()) => return ExitCode::FAILURE,
Err(AlreadyReportedToCommandline) => return ExitCode::FAILURE,
};

let read_metadata = || {
let read_metadata = || -> Result<_, AlreadyReportedToCommandline> {
let webgpu_cts_meta_parent_dir = {
path!(
&gecko_checkout
Expand All @@ -79,10 +80,26 @@ fn run(cli: Cli) -> ExitCode {
)
};

read_gecko_files_at(&gecko_checkout, &webgpu_cts_meta_parent_dir, "**/*.ini")
let mut found_err = false;
let collected =
read_gecko_files_at(&gecko_checkout, &webgpu_cts_meta_parent_dir, "**/*.ini")?
.filter_map(|res| match res {
Ok(ok) => Some(ok),
Err(AlreadyReportedToCommandline) => {
found_err = true;
None
}
})
.map(|(p, fc)| (Arc::new(p), Arc::new(fc)))
.collect::<IndexMap<_, _>>();
if found_err {
Err(AlreadyReportedToCommandline)
} else {
Ok(collected)
}
};

fn render_parse_errors<'a>(
fn render_metadata_parse_errors<'a>(
path: &Arc<PathBuf>,
file_contents: &Arc<String>,
errors: impl IntoIterator<Item = Rich<'a, char>>,
Expand All @@ -104,7 +121,7 @@ fn run(cli: Cli) -> ExitCode {
inner: error.clone().into_owned(),
span: SourceSpan::new(span.start.into(), (span.end - span.start).into()),
};
let error = miette::Report::new(error);
let error = Report::new(error);
eprintln!("{error:?}");
}
}
Expand All @@ -113,7 +130,7 @@ fn run(cli: Cli) -> ExitCode {
Subcommand::Format => {
let raw_test_files_by_path = match read_metadata() {
Ok(paths) => paths,
Err(()) => return ExitCode::FAILURE,
Err(AlreadyReportedToCommandline) => return ExitCode::FAILURE,
};
log::info!("formatting metadata in-place…");
let mut fmt_err_found = false;
Expand All @@ -123,7 +140,7 @@ fn run(cli: Cli) -> ExitCode {
{
Err(errors) => {
fmt_err_found = true;
render_parse_errors(&path, &file_contents, errors);
render_metadata_parse_errors(&path, &file_contents, errors);
}
Ok(file) => {
let mut out =
Expand Down Expand Up @@ -173,7 +190,7 @@ fn run(cli: Cli) -> ExitCode {
let mut found_parse_err = false;
let raw_test_files_by_path = match read_metadata() {
Ok(paths) => paths,
Err(()) => return ExitCode::FAILURE,
Err(AlreadyReportedToCommandline) => return ExitCode::FAILURE,
};
let extracted = raw_test_files_by_path
.iter()
Expand All @@ -195,7 +212,7 @@ fn run(cli: Cli) -> ExitCode {
})),
Err(errors) => {
found_parse_err = true;
render_parse_errors(path, file_contents, errors);
render_metadata_parse_errors(path, file_contents, errors);
None
}
}
Expand All @@ -220,7 +237,7 @@ fn run(cli: Cli) -> ExitCode {
#[derive(Clone, Debug, Default)]
struct PerPlatformAnalysis {
tests_with_runner_errors: BTreeSet<Arc<SectionHeader>>,
tests_with_disabled: BTreeSet<Arc<SectionHeader>>,
tests_with_disabled_or_skip: BTreeSet<Arc<SectionHeader>>,
tests_with_crashes: BTreeSet<Arc<SectionHeader>>,
subtests_with_failures_by_test:
BTreeMap<Arc<SectionHeader>, IndexSet<Arc<SectionHeader>>>,
Expand Down Expand Up @@ -307,7 +324,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());
})
}

Expand Down Expand Up @@ -339,6 +358,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());
}),
}
}

Expand Down Expand Up @@ -395,7 +419,7 @@ fn run(cli: Cli) -> ExitCode {
if is_disabled {
analysis
.windows
.tests_with_disabled
.tests_with_disabled_or_skip
.insert(test_name.clone());
}

Expand Down Expand Up @@ -481,14 +505,14 @@ 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_crashes,
subtests_with_failures_by_test,
subtests_with_timeouts_by_test,
} = analysis;

let num_tests_with_runner_errors = tests_with_runner_errors.len();
let num_tests_with_disabled = tests_with_disabled.len();
let num_tests_with_disabled = tests_with_disabled_or_skip.len();
let num_tests_with_crashes = tests_with_crashes.len();
let num_tests_with_failures_somewhere = subtests_with_failures_by_test.len();
let num_subtests_with_failures_somewhere = subtests_with_failures_by_test
Expand Down Expand Up @@ -534,8 +558,20 @@ fn run(cli: Cli) -> ExitCode {
&webgpu_cts_test_parent_dir,
"**/cts.https.html",
) {
Ok(paths) => paths,
Err(()) => return ExitCode::FAILURE,
Ok(paths) => {
let mut found_err = false;
let collected = paths
.filter_map(|res| {
found_err |= res.is_ok();
res.ok()
})
.collect::<IndexMap<_, _>>();
if found_err {
return ExitCode::FAILURE;
}
collected
}
Err(AlreadyReportedToCommandline) => return ExitCode::FAILURE,
};

let meta_variant_re =
Expand Down Expand Up @@ -568,10 +604,13 @@ fn read_gecko_files_at(
gecko_checkout: &Path,
base: &Path,
glob_pattern: &str,
) -> Result<IndexMap<Arc<PathBuf>, Arc<String>>, ()> {
log::info!("reading {glob_pattern:?} files at {}", base.display());
) -> Result<
impl Iterator<Item = Result<(PathBuf, String), AlreadyReportedToCommandline>>,
AlreadyReportedToCommandline,
> {
log::info!("reading {glob_pattern} files at {}", base.display());
let mut found_read_err = false;
let mut paths = wax::Glob::new(glob_pattern)
let mut paths = Glob::new(glob_pattern)
.unwrap()
.walk(base)
.filter_map(|entry| match entry {
Expand All @@ -585,7 +624,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;
Expand All @@ -606,36 +645,25 @@ fn read_gecko_files_at(
);

if found_read_err {
return Err(());
return Err(AlreadyReportedToCommandline);
}

let files = paths
.into_iter()
.filter_map(|file| {
log::debug!("reading from {}…", file.display());
match fs::read_to_string(&file) {
Err(e) => {
log::error!("failed to read {file:?}: {e}");
found_read_err = true;
None
}
Ok(contents) => Some((Arc::new(file), Arc::new(contents))),
}
})
.collect::<IndexMap<_, _>>();

if found_read_err {
return Err(());
}

Ok(files)
Ok(paths.into_iter().map(|path| -> Result<_, _> {
log::debug!("reading from {}…", path.display());
fs::read_to_string(&path)
.map_err(|e| {
log::error!("failed to read {path:?}: {e}");
AlreadyReportedToCommandline
})
.map(|file_contents| (path, file_contents))
}))
}

/// Search for a `mozilla-central` checkout either via Mercurial or Git, iterating from the CWD to
/// its parent directories.
///
/// This function reports to `log` automatically, so no meaningful [`Err`] value is returned.
fn search_for_moz_central_ckt() -> miette::Result<PathBuf, ()> {
fn search_for_moz_central_ckt() -> Result<PathBuf, AlreadyReportedToCommandline> {
use lets_find_up::{find_up_with, FindUpKind, FindUpOptions};

let find_up_opts = || FindUpOptions {
Expand Down Expand Up @@ -671,7 +699,7 @@ fn search_for_moz_central_ckt() -> miette::Result<PathBuf, ()> {
log::warn!("{e:?}");
log::warn!("{e2:?}");
log::error!("failed to find a Gecko repository root");
Err(())
Err(AlreadyReportedToCommandline)
}
})?;

Expand All @@ -682,3 +710,5 @@ fn search_for_moz_central_ckt() -> miette::Result<PathBuf, ()> {

Ok(gecko_source_root)
}

struct AlreadyReportedToCommandline;
18 changes: 8 additions & 10 deletions src/bin/moz-webgpu-cts/metadata.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,14 @@ pub type Test = metadata::Test<AnalyzeableProps<TestOutcome>, AnalyzeableProps<S

pub type Subtest = metadata::Subtest<AnalyzeableProps<SubtestOutcome>>;

pub fn format_file<'a>(
file: &'a metadata::File<AnalyzeableProps<TestOutcome>, AnalyzeableProps<SubtestOutcome>>,
) -> impl Display + 'a {
pub fn format_file(file: &File) -> impl Display + '_ {
lazy_format!(|f| {
let metadata::File { tests } = file;
write!(f, "{}", tests.iter().map(format_test).join_with("\n\n"))
})
}

fn format_test<'a>(test: &'a Test) -> impl Display + 'a {
fn format_test(test: &Test) -> impl Display + '_ {
lazy_format!(|f| {
let Test {
name,
Expand Down Expand Up @@ -68,10 +66,7 @@ fn format_test<'a>(test: &'a Test) -> impl Display + 'a {
})
}

fn format_properties<'a, Out>(
indentation: u8,
property: &'a AnalyzeableProps<Out>,
) -> impl Display + 'a
fn format_properties<Out>(indentation: u8, property: &AnalyzeableProps<Out>) -> impl Display + '_
where
Out: Display,
{
Expand Down Expand Up @@ -125,7 +120,7 @@ where
"{indent} if {}: {}",
platform
.as_ref()
.map(|p| -> &dyn Display { &*p })
.map(|p| -> &dyn Display { p })
.into_iter()
.chain(
build_profile
Expand All @@ -149,7 +144,7 @@ where
})
}

fn format_exp<'a, Exp>(exp: &'a Expectation<Exp>) -> impl Display + 'a
fn format_exp<Exp>(exp: &Expectation<Exp>) -> impl Display + '_
where
Exp: Display,
{
Expand Down Expand Up @@ -418,6 +413,7 @@ pub enum TestOutcome {
Timeout,
Crash,
Error,
Skip,
}

impl Default for TestOutcome {
Expand All @@ -436,6 +432,7 @@ impl Display for TestOutcome {
Self::Timeout => "TIMEOUT",
Self::Crash => "CRASH",
Self::Error => "ERROR",
Self::Skip => "SKIP",
}
)
}
Expand All @@ -453,6 +450,7 @@ impl<'a> Properties<'a> for AnalyzeableProps<TestOutcome> {
keyword("CRASH").to(TestOutcome::Crash),
keyword("TIMEOUT").to(TestOutcome::Timeout),
keyword("ERROR").to(TestOutcome::Error),
keyword("SKIP").to(TestOutcome::Skip),
)),
)
.boxed()
Expand Down

0 comments on commit dac82d5

Please sign in to comment.