From 4a9f42950140b36b1916ab0e0320d721223b1095 Mon Sep 17 00:00:00 2001 From: Matt Mastracci Date: Tue, 5 Dec 2023 09:26:06 -0700 Subject: refactor(cli): refactor bench/test for future module changes (#21460) Extracting some refactorings for the module work that will land in https://github.com/denoland/deno_core/pull/359/ --- cli/tests/testdata/bench/no_check.out | 6 ++- cli/tests/testdata/bench/unhandled_rejection.out | 6 ++- cli/tests/testdata/bench/unresolved_promise.out | 4 ++ cli/tools/bench/mod.rs | 48 ++++++++++++++++++++ cli/tools/bench/reporters.rs | 14 ++++++ cli/tools/test/mod.rs | 57 ++++++++++++++++++------ 6 files changed, 119 insertions(+), 16 deletions(-) (limited to 'cli') diff --git a/cli/tests/testdata/bench/no_check.out b/cli/tests/testdata/bench/no_check.out index 1f90836eb..9548a4f3d 100644 --- a/cli/tests/testdata/bench/no_check.out +++ b/cli/tests/testdata/bench/no_check.out @@ -1,5 +1,9 @@ -error: Uncaught TypeError: Cannot read properties of undefined (reading 'fn') +error: TypeError: Cannot read properties of undefined (reading 'fn') Deno.bench(); ^ at [WILDCARD] at [WILDCARD]/bench/no_check.ts:1:6 +This error was not caught from a benchmark and caused the bench runner to fail on the referenced module. +It most likely originated from a dangling promise, event/timeout handler or top-level code. + +error: Bench failed diff --git a/cli/tests/testdata/bench/unhandled_rejection.out b/cli/tests/testdata/bench/unhandled_rejection.out index 98c9e68b5..dba6d9ed4 100644 --- a/cli/tests/testdata/bench/unhandled_rejection.out +++ b/cli/tests/testdata/bench/unhandled_rejection.out @@ -1,7 +1,11 @@ Check [WILDCARD]/bench/unhandled_rejection.ts -error: Uncaught (in promise) Error: rejection +error: (in promise) Error: rejection reject(new Error("rejection")); ^ at [WILDCARD]/bench/unhandled_rejection.ts:2:10 at new Promise () at [WILDCARD]/bench/unhandled_rejection.ts:1:1 +This error was not caught from a benchmark and caused the bench runner to fail on the referenced module. +It most likely originated from a dangling promise, event/timeout handler or top-level code. + +error: Bench failed diff --git a/cli/tests/testdata/bench/unresolved_promise.out b/cli/tests/testdata/bench/unresolved_promise.out index b601e9b70..e5c53836a 100644 --- a/cli/tests/testdata/bench/unresolved_promise.out +++ b/cli/tests/testdata/bench/unresolved_promise.out @@ -3,3 +3,7 @@ error: Top-level await promise never resolved await new Promise((_resolve, _reject) => {}); ^ at ([WILDCARD]bench/unresolved_promise.ts:1:1) +This error was not caught from a benchmark and caused the bench runner to fail on the referenced module. +It most likely originated from a dangling promise, event/timeout handler or top-level code. + +error: Bench failed diff --git a/cli/tools/bench/mod.rs b/cli/tools/bench/mod.rs index 70551a767..ed6192b3b 100644 --- a/cli/tools/bench/mod.rs +++ b/cli/tools/bench/mod.rs @@ -42,6 +42,7 @@ use serde::Serialize; use std::collections::HashSet; use std::path::Path; use std::sync::Arc; +use std::time::Duration; use tokio::sync::mpsc::unbounded_channel; use tokio::sync::mpsc::UnboundedSender; @@ -76,6 +77,7 @@ pub enum BenchEvent { Register(BenchDescription), Wait(usize), Result(usize, BenchResult), + UncaughtError(String, Box), } #[derive(Debug, Clone, Deserialize, Serialize)] @@ -166,6 +168,38 @@ async fn bench_specifier( specifier: ModuleSpecifier, sender: UnboundedSender, filter: TestFilter, +) -> Result<(), AnyError> { + match bench_specifier_inner( + worker_factory, + permissions, + specifier.clone(), + &sender, + filter, + ) + .await + { + Ok(()) => Ok(()), + Err(error) => { + if error.is::() { + sender.send(BenchEvent::UncaughtError( + specifier.to_string(), + Box::new(error.downcast::().unwrap()), + ))?; + Ok(()) + } else { + Err(error) + } + } + } +} + +/// Run a single specifier as an executable bench module. +async fn bench_specifier_inner( + worker_factory: Arc, + permissions: Permissions, + specifier: ModuleSpecifier, + sender: &UnboundedSender, + filter: TestFilter, ) -> Result<(), AnyError> { let mut worker = worker_factory .create_custom_worker( @@ -180,6 +214,10 @@ async fn bench_specifier( worker.execute_side_module_possibly_with_npm().await?; let mut worker = worker.into_main_worker(); + + // Ensure that there are no pending exceptions before we start running tests + worker.run_up_to_duration(Duration::from_millis(0)).await?; + worker.dispatch_load_event(located_script_name!())?; let benchmarks = { @@ -227,6 +265,11 @@ async fn bench_specifier( // event loop to continue beyond what's needed to await results. worker.dispatch_beforeunload_event(located_script_name!())?; worker.dispatch_unload_event(located_script_name!())?; + + // Ensure the worker has settled so we can catch any remaining unhandled rejections. We don't + // want to wait forever here. + worker.run_up_to_duration(Duration::from_millis(0)).await?; + Ok(()) } @@ -308,6 +351,11 @@ async fn bench_specifiers( } }; } + + BenchEvent::UncaughtError(origin, error) => { + report.failed += 1; + reporter.report_uncaught_error(&origin, error); + } } } diff --git a/cli/tools/bench/reporters.rs b/cli/tools/bench/reporters.rs index 35b4a229c..a3e3f85d1 100644 --- a/cli/tools/bench/reporters.rs +++ b/cli/tools/bench/reporters.rs @@ -12,6 +12,7 @@ pub trait BenchReporter { fn report_wait(&mut self, desc: &BenchDescription); fn report_output(&mut self, output: &str); fn report_result(&mut self, desc: &BenchDescription, result: &BenchResult); + fn report_uncaught_error(&mut self, origin: &str, error: Box); } #[derive(Debug, Serialize)] @@ -91,6 +92,8 @@ impl BenchReporter for JsonReporter { }); } } + + fn report_uncaught_error(&mut self, _origin: &str, _error: Box) {} } pub struct ConsoleReporter { @@ -301,4 +304,15 @@ impl BenchReporter for ConsoleReporter { fn report_end(&mut self, _: &BenchReport) { self.report_group_summary(); } + + fn report_uncaught_error(&mut self, _origin: &str, error: Box) { + println!( + "{}: {}", + colors::red_bold("error"), + format_test_error(&error) + ); + println!("This error was not caught from a benchmark and caused the bench runner to fail on the referenced module."); + println!("It most likely originated from a dangling promise, event/timeout handler or top-level code."); + println!(); + } } diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 836004f86..c69c3115c 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -410,6 +410,41 @@ pub async fn test_specifier( mut sender: TestEventSender, fail_fast_tracker: FailFastTracker, options: TestSpecifierOptions, +) -> Result<(), AnyError> { + match test_specifier_inner( + worker_factory, + permissions, + specifier.clone(), + &sender, + fail_fast_tracker, + options, + ) + .await + { + Ok(()) => Ok(()), + Err(error) => { + if error.is::() { + sender.send(TestEvent::UncaughtError( + specifier.to_string(), + Box::new(error.downcast::().unwrap()), + ))?; + Ok(()) + } else { + Err(error) + } + } + } +} + +/// Test a single specifier as documentation containing test programs, an executable test module or +/// both. +async fn test_specifier_inner( + worker_factory: Arc, + permissions: Permissions, + specifier: ModuleSpecifier, + sender: &TestEventSender, + fail_fast_tracker: FailFastTracker, + options: TestSpecifierOptions, ) -> Result<(), AnyError> { if fail_fast_tracker.should_stop() { return Ok(()); @@ -439,23 +474,13 @@ pub async fn test_specifier( } // We execute the main module as a side module so that import.meta.main is not set. - match worker.execute_side_module_possibly_with_npm().await { - Ok(()) => {} - Err(error) => { - if error.is::() { - sender.send(TestEvent::UncaughtError( - specifier.to_string(), - Box::new(error.downcast::().unwrap()), - ))?; - return Ok(()); - } else { - return Err(error); - } - } - } + worker.execute_side_module_possibly_with_npm().await?; let mut worker = worker.into_main_worker(); + // Ensure that there are no pending exceptions before we start running tests + worker.run_up_to_duration(Duration::from_millis(0)).await?; + worker.dispatch_load_event(located_script_name!())?; run_tests_for_worker(&mut worker, &specifier, &options, &fail_fast_tracker) @@ -466,6 +491,10 @@ pub async fn test_specifier( worker.dispatch_beforeunload_event(located_script_name!())?; worker.dispatch_unload_event(located_script_name!())?; + // Ensure the worker has settled so we can catch any remaining unhandled rejections. We don't + // want to wait forever here. + worker.run_up_to_duration(Duration::from_millis(0)).await?; + if let Some(coverage_collector) = coverage_collector.as_mut() { worker .js_runtime -- cgit v1.2.3