Repository: denoland/deno
Pull request: fix(test): throw when Deno.test() is called during test execution

Fixes #30787.

`Deno.test()` was being accepted even when called from inside a running test callback.  
That means nested registrations could silently be ignored and tests looked like they passed.

Now `Deno.test()` calls made from inside a running test are rejected immediately.  
Instead of silently registering too late, they fail with a clear error:

`Nested Deno.test() calls are not supported. Use t.step() for nested tests.`

This makes the misuse explicit and points users to `t.step()`.

<!--
Before submitting a PR, please read https://docs.deno.com/runtime/manual/references/contributing

1. Give the PR a descriptive title.

  Examples of good title:
    - fix(std/http): Fix race condition in server
    - docs(console): Update docstrings
    - feat(doc): Handle nested reexports

  Examples of bad title:
    - fix #7123
    - update docs
    - fix bugs

2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `cargo test` passes.
5. Ensure `./tools/format.js` passes without changing files.
6. Ensure `./tools/lint.js` passes.
7. Open as a draft PR if your work is still in progress. The CI won't run
   all steps, but you can add '[ci]' to a commit message to force it to.
8. If you would like to run the benchmarks on the CI, add the 'ci-bench' label.
-->


Changed files:
filename: cli/ops/testing.rs
patch: @@ -132,7 +132,7 @@ fn op_register_test(
   };
   state
     .borrow_mut::<TestContainer>()
-    .register(description, function);
+    .register(description, function)?;
   ret_buf.copy_from_slice(&(id as u32).to_le_bytes());
   Ok(())
 }

filename: cli/tools/test/mod.rs
patch: @@ -206,6 +206,7 @@ pub(crate) struct TestContainer {
   descriptions: TestDescriptions,
   test_functions: Vec<v8::Global<v8::Function>>,
   test_hooks: TestHooks,
+  registration_phase_ended: bool,
 }
 
 #[derive(Default)]
@@ -217,13 +218,26 @@ pub(crate) struct TestHooks {
 }
 
 impl TestContainer {
+  pub fn with_registration_phase_ended() -> Self {
+    Self {
+      registration_phase_ended: true,
+      ..Default::default()
+    }
+  }
+
   pub fn register(
     &mut self,
     description: TestDescription,
     function: v8::Global<v8::Function>,
-  ) {
+  ) -> Result<(), JsErrorBox> {
+    if self.registration_phase_ended {
+      return Err(JsErrorBox::type_error(
+        "Nested Deno.test() calls are not supported. Use t.step() for nested tests.",
+      ));
+    }
     self.descriptions.tests.insert(description.id, description);
-    self.test_functions.push(function)
+    self.test_functions.push(function);
+    Ok(())
   }
 
   pub fn register_hook(
@@ -913,22 +927,34 @@ pub async fn run_tests_for_worker(
   let state_rc = worker.js_runtime.op_state();
 
   // Take whatever tests have been registered
-  let container =
-    std::mem::take(&mut *state_rc.borrow_mut().borrow_mut::<TestContainer>());
+  let container = std::mem::replace(
+    &mut *state_rc.borrow_mut().borrow_mut::<TestContainer>(),
+    TestContainer::with_registration_phase_ended(),
+  );
 
   let descriptions = Arc::new(container.descriptions);
-  event_tracker.register(descriptions.clone())?;
-  run_tests_for_worker_inner(
-    worker,
-    specifier,
-    descriptions,
-    container.test_functions,
-    container.test_hooks,
-    options,
-    event_tracker,
-    fail_fast_tracker,
-  )
-  .await
+  let result = async {
+    event_tracker.register(descriptions.clone())?;
+    run_tests_for_worker_inner(
+      worker,
+      specifier,
+      descriptions,
+      container.test_functions,
+      container.test_hooks,
+      options,
+      event_tracker,
+      fail_fast_tracker,
+    )
+    .await
+  }
+  .await;
+
+  // This worker can execute another discovery/run cycle (for example in REPL),
+  // so reset to a fresh container after the current run ends.
+  *state_rc.borrow_mut().borrow_mut::<TestContainer>() =
+    TestContainer::default();
+
+  result
 }
 
 fn compute_tests_to_run(

filename: cli/tsc/dts/lib.deno.ns.d.ts
patch: @@ -859,6 +859,11 @@ declare namespace Deno {
    *
    * `fn` can be async if required.
    *
+   * Tests are discovered before they are executed, so registrations must happen
+   * at module load time.
+   * Nested `Deno.test()` calls are not supported.
+   * Use `t.step()` for nested tests.
+   *
    * ```ts
    * import { assertEquals } from "jsr:@std/assert";
    *

filename: tests/integration/test_tests.rs
patch: @@ -119,3 +119,23 @@ fn conditionally_loads_type_graph() {
     .run();
   assert_not_contains!(output.combined_output(), "type_reference.d.ts");
 }
+
+#[test]
+fn nested_deno_test_registration_errors() {
+  let context = TestContextBuilder::new().use_temp_cwd().build();
+  let temp_dir = context.temp_dir();
+  temp_dir.write(
+    "nested_test.ts",
+    r#"
+Deno.test("outer", () => {
+  Deno.test("inner", () => {});
+});
+"#,
+  );
+
+  let output = context.new_command().args("test nested_test.ts").run();
+  output.assert_exit_code(1);
+  let combined = output.combined_output();
+  assert_contains!(combined, "Nested Deno.test() calls are not supported",);
+  assert_contains!(combined, "Use t.step()");
+}

filename: tests/unit_node/_fs/_fs_open_test.ts
patch: @@ -397,15 +397,13 @@ Deno.test("[std/node/fs] open callback isn't called twice if error is thrown", a
       await Deno.remove(tempFile);
     },
   });
+});
 
-  Deno.test({
-    name: "SYNC: open file with flag set to 0 (readonly)",
-    fn() {
-      const file = Deno.makeTempFileSync();
-      const fd = openSync(file, 0);
-      closeSync(fd);
-    },
-  });
+Deno.test("SYNC: open file with flag set to 0 (readonly)", async () => {
+  const file = Deno.makeTempFileSync();
+  const fd = openSync(file, 0);
+  closeSync(fd);
+  await Deno.remove(file);
 });
 
 Deno.test("[std/node/fs] openSync with custom flag", {