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

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

Merged

Conversation

andreubotella
Copy link
Contributor

@andreubotella andreubotella commented Nov 29, 2021

This relands #12908 –which was reverted by #12929– now that #12933 has landed. It also adds @kt3k's tests from #12908 (comment)

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.
@bartlomieju
Copy link
Member

I verified that tests in deno_std no longer hang.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bartlomieju bartlomieju merged commit 9a10668 into denoland:main Nov 30, 2021
@andreubotella andreubotella deleted the op-sanitizer-after-timer-macrotasks branch November 30, 2021 00:27
bartlomieju pushed a commit to bartlomieju/deno that referenced this pull request Nov 30, 2021
…ers (denoland#12934)

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.
bnoordhuis pushed a commit that referenced this pull request Nov 30, 2021
…ers (#12934)

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.

Co-authored-by: Andreu Botella <abb@randomunok.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants