fix(test): Improve reliability of deno test's op sanitizer with tim…

…ers (denoland#12908)

Although not easy to replicate in the wild, the `deno test` op sanitizer
can fail when there are intervals that started before a test runs, since
the op sanitizer can end up running in the time between the timer op for
an interval's run resolves and the op for the next run starts.

This change fixes that by adding a new macrotask callback that will run
after the timer macrotask queue has drained. This ensures that there is
a timer op if there are any timers which are unresolved by the time the
op sanitizer runs.

cli/tests/integration/test_tests.rs

@@ -151,6 +151,11 @@ itest!(ops_sanitizer_unstable {
   output: "test/ops_sanitizer_unstable.out",
 });
 
+itest!(ops_sanitizer_timeout_failure {
+  args: "test test/ops_sanitizer_timeout_failure.ts",
+  output: "test/ops_sanitizer_timeout_failure.out",
+});
+
 itest!(exit_sanitizer {
   args: "test test/exit_sanitizer.ts",
   output: "test/exit_sanitizer.out",

cli/tests/testdata/test/ops_sanitizer_timeout_failure.out

@@ -0,0 +1,6 @@
+Check [WILDCARD]/testdata/test/ops_sanitizer_timeout_failure.ts
+running 1 test from [WILDCARD]/testdata/test/ops_sanitizer_timeout_failure.ts
+test wait ... ok ([WILDCARD])
+
+test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out ([WILDCARD])
+

cli/tests/testdata/test/ops_sanitizer_timeout_failure.ts

@@ -0,0 +1,22 @@
+let intervalHandle: number;
+let firstIntervalPromise: Promise<void>;
+
+addEventListener("load", () => {
+  firstIntervalPromise = new Promise((resolve) => {
+    let firstIntervalCalled = false;
+    intervalHandle = setInterval(() => {
+      if (!firstIntervalCalled) {
+        resolve();
+        firstIntervalCalled = true;
+      }
+    }, 5);
+  });
+});
+
+addEventListener("unload", () => {
+  clearInterval(intervalHandle);
+});
+
+Deno.test("wait", async function () {
+  await firstIntervalPromise;
+});

runtime/js/40_testing.js

@@ -30,6 +30,37 @@
   } = window.__bootstrap.primordials;
   let testStepsEnabled = false;
 
+  let opSanitizerDelayResolve = null;
+
+  // Even if every resource is closed by the end of a test, there can be a delay
+  // until the pending ops have all finished. This function returns a promise
+  // that resolves when it's (probably) fine to run the op sanitizer.
+  //
+  // This is implemented by adding a macrotask callback that runs after the
+  // timer macrotasks, so we can guarantee that a currently running interval
+  // will have an associated op. An additional `setTimeout` of 0 is needed
+  // before that, though, in order to give time for worker message ops to finish
+  // (since timeouts of 0 don't queue tasks in the timer queue immediately).
+  function opSanitizerDelay() {
+    return new Promise((resolve, reject) => {
+      setTimeout(() => {
+        if (opSanitizerDelayResolve !== null) {
+          reject(new Error("There is an op sanitizer delay already."));
+        } else {
+          opSanitizerDelayResolve = resolve;
+        }
+      }, 0);
+    });
+  }
+
+  function handleOpSanitizerDelayMacrotask() {
+    if (opSanitizerDelayResolve !== null) {
+      opSanitizerDelayResolve();
+      opSanitizerDelayResolve = null;
+    }
+    return true;
+  }
+
   // Wrap test function in additional assertion that makes sure
   // the test case does not leak async "ops" - ie. number of async
   // completed ops after the test is the same as number of dispatched
@@ -45,7 +76,7 @@
         // Defer until next event loop turn - that way timeouts and intervals
         // cleared can actually be removed from resource table, otherwise
         // false positives may occur (https://github.com/denoland/deno/issues/4591)
-        await new Promise((resolve) => setTimeout(resolve, 0));
+        await opSanitizerDelay();
       }
 
       if (step.shouldSkipSanitizers) {
@@ -466,6 +497,8 @@ finishing test case.`;
     filter = null,
     shuffle = null,
   } = {}) {
+    core.setMacrotaskCallback(handleOpSanitizerDelayMacrotask);
+
     const origin = getTestOrigin();
     const originalConsole = globalThis.console;