Skip to content

Conversation

lionel-rowe
Copy link
Contributor

@lionel-rowe lionel-rowe commented Sep 12, 2025

Bug was introduced in #6775, as setArbitraryLengthTimeout returns { valueOf(): number } rather than a primitive number, and Deno.unrefTimer throws if its argument isn't a number.

Previous type checking missed it due to already using a // @ts-ignore for the offending line, and the test also missed it due to stubbing Deno.unrefTimer with a no-op.

@lionel-rowe lionel-rowe requested a review from kt3k as a code owner September 12, 2025 10:06
@github-actions github-actions bot added the async label Sep 12, 2025
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.16%. Comparing base (91d1d55) to head (6d95f3c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6822      +/-   ##
==========================================
- Coverage   94.16%   94.16%   -0.01%     
==========================================
  Files         573      573              
  Lines       42400    42401       +1     
  Branches     6732     6732              
==========================================
- Hits        39928    39926       -2     
- Misses       2422     2424       +2     
- Partials       50       51       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lionel-rowe
Copy link
Contributor Author

I think (?) 368f865 should reliably fix the flaky test from this test (v1.x, macOS-latest) CI run:

delay() runs to completion with `ms` argument > 0x7fffffff => ./async/delay_test.ts:185:6
error: Leaks detected:
  - A timer was started before the test, but completed during the test. Intervals and timers should not complete in a test if they were not started in that test. This is often caused by not calling `clearTimeout`. The operation was started here:
    at Object.queueUserTimer (ext:core/01_core.js:792:9)
    at setTimeout (ext:deno_web/02_timers.js:74:15)
    at queueTimeout (file:///Users/runner/work/std/std/async/delay.ts:90:9)
    at setArbitraryLengthTimeout (file:///Users/runner/work/std/std/async/delay.ts:93:3)
    at file:///Users/runner/work/std/std/async/delay.ts:58:15
    at new Promise (<anonymous>)
    at delay (file:///Users/runner/work/std/std/async/delay.ts:49:10)
    at fn (file:///Users/runner/work/std/std/async/delay_test.ts:131:13)
    at innerWrapped (ext:cli/40_test.js:191:11)
    at exitSanitizer (ext:cli/40_test.js:107:33)

(Because try...catch...assert doesn't actually verify if an error was thrown)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant