fix(test): race condition for cancelled tests (denoland#15233)

cli/tests/testdata/test/uncaught_errors.out

@@ -1,6 +1,9 @@
-running 2 tests from ./test/uncaught_errors_1.ts
+running 3 tests from ./test/uncaught_errors_1.ts
 foo 1 ... FAILED ([WILDCARD])
 foo 2 ... ok ([WILDCARD])
+foo 3 ...
+Uncaught error from ./test/uncaught_errors_1.ts FAILED
+foo 3 ... cancelled (0ms)
 running 3 tests from ./test/uncaught_errors_2.ts
 bar 1 ... ok ([WILDCARD])
 bar 2 ... FAILED ([WILDCARD])
@@ -15,6 +18,14 @@ error: Error: foo 1 message
  ^
  at [WILDCARD]/test/uncaught_errors_1.ts:2:9
 
+./test/uncaught_errors_1.ts (uncaught error)
+error: (in promise) Error: foo 3 message
+ Promise.reject(new Error("foo 3 message"));
+ ^
+ at [WILDCARD]/test/uncaught_errors_1.ts:8:18
+This error was not caught from a test and caused the test runner to fail on the referenced module.
+It most likely originated from a dangling promise, event/timeout handler or top-level code.
+
 bar 2 => ./test/uncaught_errors_2.ts:3:6
 error: Error: bar 2
  throw new Error("bar 2");
@@ -38,10 +49,11 @@ It most likely originated from a dangling promise, event/timeout handler or top-
  FAILURES 
 
 foo 1 => ./test/uncaught_errors_1.ts:1:6
+./test/uncaught_errors_1.ts (uncaught error)
 bar 2 => ./test/uncaught_errors_2.ts:3:6
 bar 3 => ./test/uncaught_errors_2.ts:6:6
 ./test/uncaught_errors_3.ts (uncaught error)
 
-FAILED | 2 passed | 4 failed ([WILDCARD])
+FAILED | 2 passed | 6 failed ([WILDCARD])
 
 error: Test failed

cli/tests/testdata/test/uncaught_errors_1.ts

@@ -3,3 +3,7 @@ Deno.test("foo 1", () => {
 });
 
 Deno.test("foo 2", () => {});
+
+Deno.test("foo 3", () => {
+ Promise.reject(new Error("foo 3 message"));
+});

cli/tools/test.rs

@@ -34,7 +34,6 @@ use deno_core::futures::stream;
 use deno_core::futures::FutureExt;
 use deno_core::futures::StreamExt;
 use deno_core::parking_lot::Mutex;
-use deno_core::parking_lot::RwLock;
 use deno_core::serde_json::json;
 use deno_core::url::Url;
 use deno_core::ModuleSpecifier;
@@ -1123,11 +1122,7 @@ async fn test_specifiers(
  let sender = TestEventSender::new(sender);
  let concurrent_jobs = options.concurrent_jobs;
  let fail_fast = options.fail_fast;
- let tests: Arc<RwLock<IndexMap<usize, TestDescription>>> =
- Arc::new(RwLock::new(IndexMap::new()));
- let mut test_steps = IndexMap::new();
 
- let tests_ = tests.clone();
  let join_handles =
  specifiers_with_mode.iter().map(move |(specifier, mode)| {
  let ps = ps.clone();
@@ -1136,7 +1131,6 @@ async fn test_specifiers(
  let mode = mode.clone();
  let mut sender = sender.clone();
  let options = options.clone();
- let tests = tests_.clone();
 
  tokio::task::spawn_blocking(move || {
  let origin = specifier.to_string();
@@ -1151,18 +1145,9 @@ async fn test_specifiers(
  if let Err(error) = file_result {
  if error.is::<JsError>() {
  sender.send(TestEvent::UncaughtError(
- origin.clone(),
+ origin,
  Box::new(error.downcast::<JsError>().unwrap()),
  ))?;
- for desc in tests.read().values() {
- if desc.origin == origin {
- sender.send(TestEvent::Result(
- desc.id,
- TestResult::Cancelled,
- 0,
- ))?
- }
- }
  } else {
  return Err(error);
  }
@@ -1181,6 +1166,8 @@ async fn test_specifiers(
  let handler = {
  tokio::task::spawn(async move {
  let earlier = Instant::now();
+ let mut tests = IndexMap::new();
+ let mut test_steps = IndexMap::new();
  let mut tests_with_result = HashSet::new();
  let mut summary = TestSummary::new();
  let mut used_only = false;
@@ -1189,7 +1176,7 @@ async fn test_specifiers(
  match event {
  TestEvent::Register(description) => {
  reporter.report_register(&description);
- tests.write().insert(description.id, description);
+ tests.insert(description.id, description);
  }
 
  TestEvent::Plan(plan) => {
@@ -1204,7 +1191,7 @@ async fn test_specifiers(
  }
 
  TestEvent::Wait(id) => {
- reporter.report_wait(tests.read().get(&id).unwrap());
+ reporter.report_wait(tests.get(&id).unwrap());
  }
 
  TestEvent::Output(output) => {
@@ -1213,7 +1200,7 @@ async fn test_specifiers(
 
  TestEvent::Result(id, result, elapsed) => {
  if tests_with_result.insert(id) {
- let description = tests.read().get(&id).unwrap().clone();
+ let description = tests.get(&id).unwrap().clone();
  match &result {
  TestResult::Ok => {
  summary.passed += 1;
@@ -1226,7 +1213,7 @@ async fn test_specifiers(
  summary.failures.push((description.clone(), error.clone()));
  }
  TestResult::Cancelled => {
- summary.failed += 1;
+ unreachable!("should be handled in TestEvent::UncaughtError");
  }
  }
  reporter.report_result(&description, &result, elapsed);
@@ -1236,7 +1223,13 @@ async fn test_specifiers(
  TestEvent::UncaughtError(origin, error) => {
  reporter.report_uncaught_error(&origin, &error);
  summary.failed += 1;
- summary.uncaught_errors.push((origin, error));
+ summary.uncaught_errors.push((origin.clone(), error));
+ for desc in tests.values() {
+ if desc.origin == origin && tests_with_result.insert(desc.id) {
+ summary.failed += 1;
+ reporter.report_result(desc, &TestResult::Cancelled, 0);
+ }
+ }
  }
 
  TestEvent::StepRegister(description) => {