Repository navigation
Make the download cancel test deterministic - #503
Merged
Merged
Conversation
The test waited up to 10 s on wall-clock timers: for the server to start answering, then for the partial file to appear. On a busy Windows runner the first wait timed out (run 39965468348). The second never really held either: the server sent 16 bytes, fewer than the 64 the downloader sniffs before it creates the file, so WaitUntil ran out its deadline silently and the test cancelled before any partial file existed. Now the server sends a first chunk past the sniffed head and then blocks until the test is over. The test cancels on the first progress report, which comes right after the file is created and written to, checks the file is there, and after the cancellation reaches it checks the file is gone; the downloader deletes it before rethrowing, so no polling is needed. A download that fails before writing fails the test with its own reason instead of a timeout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WebDownloaderTests.Cancelling_stops_the_download_and_removes_the_partial_filefailed intermittently on busy runners: on the Windowsbuildjob of run 39965468348 (PR #360, which doesn't touch the downloader) it hit aTimeoutExceptionat line 238, the 10-second wait for the fake server to start answering.The test also didn't check what it said it did. The server sent 16 bytes, but the downloader reads 64 (
WebFormats.SniffLength) before it creates the file. SoWaitUntil(file exists)ran out its 10 s deadline without saying so, and the test cancelled before any partial file existed.Changes
Task.WhenAny(written, download), so a download that fails before writing fails the test with its own error, not a timeout.WaitUntilhelper is gone.Testing
FILEE_DATA_DIRpointed at a temp folder: 20/20 passed, about 0.5 s each. The wholeWebDownloaderTestsclass passes (12/12).WebDownloader.SaveAsynccommented out, the test fails (reverted afterwards).dotnet format Filee.slnx --verify-no-changesis clean.🤖 Generated with Claude Code