Skip to content

Commit

Permalink
Small test ux improvements (#1704)
Browse files Browse the repository at this point in the history
<!---
Thank you for contributing to Boa! Please fill out the template below, and remove or add any
information as you feel neccesary.
--->

Hi, first time contributor :)

I was playing with getting the test262 test runner working and found a couple small things that I thought would've made my life a bit easier. I've split them into separate commits. Just let me know if you disagree with any and we can drop that commit. And of course, let me know of any changes you'd like to see :)

The changes are:
- Adds details to the documentation on the `--suite` command to `boa_tester`. I was trying to pass a path starting with `./test262/` for a bit before I looked at the code and saw what it was doing.
- Changes the individual test output when verbosity is > 1. Because the tests are run in parallel, the "Result" line for a given test was frequently not immediately after the "Started" line. This made it hard to determine which test had failed. The new output includes the test name in the result line, and also changes the format of all the individual-test-output lines to begin with the test name.
- Adds a `--disable-parallelism` flag. Even with the adjustments to the test output, it was still a bit hard to follow. Running serially for small sub-suites produces output that can be more easily scanned IMO. I added this as a "disable" flag (as opposed to "enable parallelism") in order to maintain the current default of running in parallel.


Co-authored-by: Grant Orndorff <grant@orndorff.me>
  • Loading branch information
orndorffgrant and orndorffgrant committed Nov 9, 2021
1 parent 009bd09 commit c8a5ff8
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 21 deletions.
13 changes: 11 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,17 @@ You can get some more verbose information that tells you the exact name of each
for debugging purposes by setting up the verbose flag twice, for example `-vv`. If you want to know the output of
each test that is executed, you can use the triple verbose (`-vvv`) flag.

Finally, if you want to only run one sub-suite or even one test (to just check if you fixed/broke something specific),
you can do it with the `-s` parameter, and then passing the path to the sub-suite or test that you want to run.
If you want to only run one sub-suite or even one test (to just check if you fixed/broke something specific),
you can do it with the `-s` parameter, and then passing the path to the sub-suite or test that you want to run. Note
that the `-s` parameter value should be a path relative to the `test262` directory. For example, to run the number
type tests, use `-s test/language/types/number`.

Finally, if you're using the verbose flag and running a sub suite with a small number of tests, then the output will
be more readable if you disable parallelism with the `-d` flag. All together it might look something like:

```
cargo run --release --bin boa_tester -- run -vv -d -s test/language/types/number 2> error.log
```

## Communication

Expand Down
56 changes: 40 additions & 16 deletions boa_tester/src/exec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,36 @@ use std::panic;

impl TestSuite {
/// Runs the test suite.
pub(crate) fn run(&self, harness: &Harness, verbose: u8) -> SuiteResult {
pub(crate) fn run(&self, harness: &Harness, verbose: u8, parallel: bool) -> SuiteResult {
if verbose != 0 {
println!("Suite {}:", self.name);
}

let suites: Vec<_> = self
.suites
.par_iter()
.map(|suite| suite.run(harness, verbose))
.collect();
let suites: Vec<_> = if parallel {
self.suites
.par_iter()
.map(|suite| suite.run(harness, verbose, parallel))
.collect()
} else {
self.suites
.iter()
.map(|suite| suite.run(harness, verbose, parallel))
.collect()
};

let tests: Vec<_> = self
.tests
.par_iter()
.map(|test| test.run(harness, verbose))
.flatten()
.collect();
let tests: Vec<_> = if parallel {
self.tests
.par_iter()
.map(|test| test.run(harness, verbose))
.flatten()
.collect()
} else {
self.tests
.iter()
.map(|test| test.run(harness, verbose))
.flatten()
.collect()
};

if verbose != 0 {
println!();
Expand Down Expand Up @@ -102,7 +115,7 @@ impl Test {
fn run_once(&self, harness: &Harness, strict: bool, verbose: u8) -> TestResult {
if verbose > 1 {
println!(
"Starting `{}`{}",
"`{}`{}: starting",
self.name,
if strict { " (strict mode)" } else { "" }
);
Expand Down Expand Up @@ -222,7 +235,9 @@ impl Test {

if verbose > 1 {
println!(
"Result: {}",
"`{}`{}: {}",
self.name,
if strict { " (strict mode)" } else { "" },
if matches!(result, (TestOutcomeResult::Passed, _)) {
"Passed".green()
} else if matches!(result, (TestOutcomeResult::Failed, _)) {
Expand All @@ -245,15 +260,24 @@ impl Test {
result
} else {
if verbose > 1 {
println!("Result: {}", "Ignored".yellow());
println!(
"`{}`{}: {}",
self.name,
if strict { " (strict mode)" } else { "" },
"Ignored".yellow()
);
} else {
print!("{}", ".".yellow());
}
(TestOutcomeResult::Ignored, String::new())
};

if verbose > 2 {
println!("Result text:");
println!(
"`{}`{}: result text",
self.name,
if strict { " (strict mode)" } else { "" },
);
println!("{}", result_text);
println!();
}
Expand Down
18 changes: 15 additions & 3 deletions boa_tester/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,17 @@ enum Cli {
#[structopt(long, parse(from_os_str), default_value = "./test262")]
test262_path: PathBuf,

/// Which specific test or test suite to run.
/// Which specific test or test suite to run. Should be a path relative to the Test262 directory: e.g. "test/language/types/number"
#[structopt(short, long, parse(from_os_str), default_value = "test")]
suite: PathBuf,

/// Optional output folder for the full results information.
#[structopt(short, long, parse(from_os_str))]
output: Option<PathBuf>,

/// Execute tests serially
#[structopt(short, long)]
disable_parallelism: bool,
},
Compare {
/// Base results of the suite.
Expand All @@ -204,9 +208,11 @@ fn main() {
test262_path,
suite,
output,
disable_parallelism,
} => {
run_test_suite(
verbose,
!disable_parallelism,
test262_path.as_path(),
suite.as_path(),
output.as_deref(),
Expand All @@ -221,7 +227,13 @@ fn main() {
}

/// Runs the full test suite.
fn run_test_suite(verbose: u8, test262_path: &Path, suite: &Path, output: Option<&Path>) {
fn run_test_suite(
verbose: u8,
parallel: bool,
test262_path: &Path,
suite: &Path,
output: Option<&Path>,
) {
if let Some(path) = output {
if path.exists() {
if !path.is_dir() {
Expand Down Expand Up @@ -254,7 +266,7 @@ fn run_test_suite(verbose: u8, test262_path: &Path, suite: &Path, output: Option
if verbose != 0 {
println!("Test suite loaded, starting tests...");
}
let results = suite.run(&harness, verbose);
let results = suite.run(&harness, verbose, parallel);

println!();
println!("Results:");
Expand Down

0 comments on commit c8a5ff8

Please sign in to comment.