fix(test): race condition for cancelled tests (#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) => {