From 6fb144520fdc5ec460eb283de1573c8936719bb8 Mon Sep 17 00:00:00 2001 From: Iban Eguia Date: Tue, 29 Mar 2022 03:50:07 +0000 Subject: [PATCH] Added better error handling for the Boa tester (#1984) Trying to fix the issue in #1982, I noticed that we didn't have a proper error handling for the boa tester. This adds the `anyhow` dependency to the tester, which makes it much easier to handle errors and bubble them up with attached context. Thanks to this I was able to easily find out the issue, and I think it could be useful to have it. It gives errors such as this one: ``` Error: could not read the suite test caused by: error reading sub-suite ./test262/test/built-ins caused by: error reading sub-suite ./test262/test/built-ins/ShadowRealm caused by: error reading sub-suite ./test262/test/built-ins/ShadowRealm/WrappedFunction caused by: error reading test ./test262/test/built-ins/ShadowRealm/WrappedFunction/throws-typeerror-on-revoked-proxy.js caused by: while scanning a block scalar, found a tab character where an indentation space is expected at line 4 column 3 caused by: while scanning a block scalar, found a tab character where an indentation space is expected at line 4 column 3 ``` --- Cargo.lock | 7 ++++++ boa_tester/Cargo.toml | 1 + boa_tester/src/main.rs | 37 +++++++++++++++++++++---------- boa_tester/src/read.rs | 50 +++++++++++++++++++++++------------------- 4 files changed, 62 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b71bca5b63..c690e05fb4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -31,6 +31,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "anyhow" +version = "1.0.56" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4361135be9122e0870de935d7c439aef945b9f9ddd4199a553b5270b49c82a27" + [[package]] name = "arrayvec" version = "0.4.12" @@ -148,6 +154,7 @@ dependencies = [ name = "boa_tester" version = "0.14.0" dependencies = [ + "anyhow", "bitflags", "boa_engine", "boa_interner", diff --git a/boa_tester/Cargo.toml b/boa_tester/Cargo.toml index f148ffe659..4a68eeb73b 100644 --- a/boa_tester/Cargo.toml +++ b/boa_tester/Cargo.toml @@ -27,3 +27,4 @@ hex = "0.4.3" num-format = "0.4.0" gc = { version = "0.4.1", features = ["derive"] } rayon = "1.5.1" +anyhow = "1.0.56" diff --git a/boa_tester/src/main.rs b/boa_tester/src/main.rs index d8895830ef..b4a7b69b3c 100644 --- a/boa_tester/src/main.rs +++ b/boa_tester/src/main.rs @@ -71,6 +71,7 @@ use self::{ read::{read_harness, read_suite, read_test, MetaData, Negative, TestFlag}, results::{compare_results, write_json}, }; +use anyhow::{bail, Context}; use bitflags::bitflags; use colored::Colorize; use fxhash::{FxHashMap, FxHashSet}; @@ -242,13 +243,21 @@ fn main() { output, disable_parallelism, } => { - run_test_suite( + if let Err(e) = run_test_suite( verbose, !disable_parallelism, test262_path.as_path(), suite.as_path(), output.as_deref(), - ); + ) { + eprintln!("Error: {e}"); + let mut src = e.source(); + while let Some(e) = src { + eprintln!(" caused by: {e}"); + src = e.source(); + } + std::process::exit(1); + } } Cli::Compare { base, @@ -265,25 +274,27 @@ fn run_test_suite( test262_path: &Path, suite: &Path, output: Option<&Path>, -) { +) -> anyhow::Result<()> { if let Some(path) = output { if path.exists() { if !path.is_dir() { - eprintln!("The output path must be a directory."); - std::process::exit(1); + bail!("the output path must be a directory."); } } else { - fs::create_dir_all(path).expect("could not create the output directory"); + fs::create_dir_all(path).context("could not create the output directory")?; } } if verbose != 0 { println!("Loading the test suite..."); } - let harness = read_harness(test262_path).expect("could not read initialization bindings"); + let harness = read_harness(test262_path).context("could not read harness")?; if suite.to_string_lossy().ends_with(".js") { - let test = read_test(&test262_path.join(suite)).expect("could not get the test to run"); + let test = read_test(&test262_path.join(suite)).with_context(|| { + let suite = suite.display(); + format!("could not read the test {suite}") + })?; if verbose != 0 { println!("Test loaded, starting..."); @@ -292,8 +303,10 @@ fn run_test_suite( println!(); } else { - let suite = - read_suite(&test262_path.join(suite)).expect("could not get the list of tests to run"); + let suite = read_suite(&test262_path.join(suite)).with_context(|| { + let suite = suite.display(); + format!("could not read the suite {suite}") + })?; if verbose != 0 { println!("Test suite loaded, starting tests..."); @@ -318,8 +331,10 @@ fn run_test_suite( ); write_json(results, output, verbose) - .expect("could not write the results to the output JSON file"); + .context("could not write the results to the output JSON file")?; } + + Ok(()) } /// All the harness include files. diff --git a/boa_tester/src/read.rs b/boa_tester/src/read.rs index ba7595fc7d..092737ed19 100644 --- a/boa_tester/src/read.rs +++ b/boa_tester/src/read.rs @@ -1,6 +1,7 @@ //! Module to read the list of test suites from disk. use super::{Harness, Locale, Phase, Test, TestSuite, IGNORED}; +use anyhow::Context; use fxhash::FxHashMap; use serde::Deserialize; use std::{fs, io, path::Path, str::FromStr}; @@ -73,10 +74,12 @@ impl FromStr for TestFlag { } /// Reads the Test262 defined bindings. -pub(super) fn read_harness(test262_path: &Path) -> io::Result { +pub(super) fn read_harness(test262_path: &Path) -> anyhow::Result { let mut includes = FxHashMap::default(); - for entry in fs::read_dir(test262_path.join("harness"))? { + for entry in + fs::read_dir(test262_path.join("harness")).context("error reading the harness directory")? + { let entry = entry?; let file_name = entry.file_name(); let file_name = file_name.to_string_lossy(); @@ -85,15 +88,20 @@ pub(super) fn read_harness(test262_path: &Path) -> io::Result { continue; } - let content = fs::read_to_string(entry.path())?; + let content = fs::read_to_string(entry.path()) + .with_context(|| format!("error reading the harnes/{file_name}"))?; includes.insert( file_name.into_owned().into_boxed_str(), content.into_boxed_str(), ); } - let assert = fs::read_to_string(test262_path.join("harness/assert.js"))?.into_boxed_str(); - let sta = fs::read_to_string(test262_path.join("harness/sta.js"))?.into_boxed_str(); + let assert = fs::read_to_string(test262_path.join("harness/assert.js")) + .context("error reading harnes/assert.js")? + .into_boxed_str(); + let sta = fs::read_to_string(test262_path.join("harness/sta.js")) + .context("error reading harnes/sta.js")? + .into_boxed_str(); Ok(Harness { assert, @@ -103,32 +111,26 @@ pub(super) fn read_harness(test262_path: &Path) -> io::Result { } /// Reads a test suite in the given path. -pub(super) fn read_suite(path: &Path) -> io::Result { +pub(super) fn read_suite(path: &Path) -> anyhow::Result { let name = path .file_name() - .ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidInput, - format!("test suite with no name found: {}", path.display()), - ) - })? + .with_context(|| format!("test suite with no name found: {}", path.display()))? .to_str() - .ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidInput, - format!("non-UTF-8 suite name found: {}", path.display()), - ) - })?; + .with_context(|| format!("non-UTF-8 suite name found: {}", path.display()))?; let mut suites = Vec::new(); let mut tests = Vec::new(); // TODO: iterate in parallel - for entry in path.read_dir()? { + for entry in path.read_dir().context("retrieving entry")? { let entry = entry?; - if entry.file_type()?.is_dir() { - suites.push(read_suite(entry.path().as_path())?); + if entry.file_type().context("retrieving file type")?.is_dir() { + suites.push(read_suite(entry.path().as_path()).with_context(|| { + let path = entry.path(); + let suite = path.display(); + format!("error reading sub-suite {suite}") + })?); } else if entry.file_name().to_string_lossy().contains("_FIXTURE") { continue; } else if IGNORED.contains_file(&entry.file_name().to_string_lossy()) { @@ -136,7 +138,11 @@ pub(super) fn read_suite(path: &Path) -> io::Result { test.set_name(entry.file_name().to_string_lossy()); tests.push(test); } else { - tests.push(read_test(entry.path().as_path())?); + tests.push(read_test(entry.path().as_path()).with_context(|| { + let path = entry.path(); + let suite = path.display(); + format!("error reading test {suite}") + })?); } }