Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(test): Improve reliability of deno test's op sanitizer with timers #12908

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cli/tests/integration/test_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 6 additions & 0 deletions cli/tests/testdata/test/ops_sanitizer_timeout_failure.out
Original file line number Diff line number Diff line change
@@ -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])

22 changes: 22 additions & 0 deletions cli/tests/testdata/test/ops_sanitizer_timeout_failure.ts
Original file line number Diff line number Diff line change
@@ -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;
});
35 changes: 34 additions & 1 deletion runtime/js/40_testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -466,6 +497,8 @@ finishing test case.`;
filter = null,
shuffle = null,
} = {}) {
core.setMacrotaskCallback(handleOpSanitizerDelayMacrotask);

const origin = getTestOrigin();
const originalConsole = globalThis.console;

Expand Down