Skip to content

test(engine): hold the download attempt deadline so slow runners cannot trip it - #4190

Open
miguel-heygen wants to merge 1 commit into
mainfrom
fix/urldownloader-attempt-deadline
Open

miguel-heygen wants to merge 1 commit into
mainfrom
fix/urldownloader-attempt-deadline

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What changes

Main went red on Windows in urlDownloader.test.ts ("locally refetches a caller checksum mismatch...") with Download timeout after 1s where hash_mismatch was expected.

Cause: every download attempt starts a real 1 second deadline timer, and the tests all pass 1_000. That test runs two attempts (fetch, hash, refetch, hash); on a slow Windows runner one attempt can outlast the timer. The earlier fix (#4155) faked only Date, so this timer was still real.

  • The test file now holds that 1 second deadline timer instead of starting it. Other timers (cache-lock poll, test delays, the 20 ms stalled-body test) stay real. No timeout value is raised.
  • "does not let one caller cancellation abort another caller" only passed because the real deadline fired after about 1 s (1078 ms on main). It now fires the held deadline by hand once the stalled first fetch has started.
  • New test: a fetch that takes 1.2 s against a 1 s deadline still succeeds, and the stub rejects on abort, so an armed deadline would fail it.

No product code changes.

Verification

  • urlDownloader.test.ts: 81 pass, three runs in a row on a remote Linux runner, exit 0 each. Lint and format clean.
  • Deliberate mutation: turning the hold off makes the new test fail.
  • Not exercised: the hosted Windows job (the flake is Windows-only, so the next Windows run is the real check).

Review

An independent read-only review found no blocking or should-fix items. One note: the hold matches a delay of exactly 1_000 ms, so a future test using 1_000 for something else would be held too.

…ot trip it

urlDownloader tests passed a real 1s deadline that a slow Windows attempt outlasted.

The deadline timer is now held; the one test that needs it fires it by hand.
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.

1 participant