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

Tolerate both CancellationError and URLError in CancellationTests #45

Merged

Conversation

simonjbeaumont
Copy link
Contributor

Motivation

When cancelling a Swift concurrency task during a streaming request then the error returned might be URLError with .cancelled code or CancellationError. The tests tried to be smart and expect just one of these depending on which stage of the request we were at, but there are still some races, and this test fails very rarely because a CancellationError was thrown instead of a URLError.

Modifications

This patch updates the test to tolerate both kinds of error at this stage of the request.

Result

The test will pass if the error is either URLError with .cancelled or CancellationError, and continue to fail if there is any other kind of error or no error.

Test Plan

Existing tests, which failed when run repeatedly for 1k runs, now pass when run for 10k runs.

When cancelling a Swift concurrency task during a streaming request
then the error returned might be `URLError` with `.cancelled` code or
`CancellationError`. The tests tried to be smart and expect just one of
these depending on which stage of the request we were at, but there are
still some races, and this test fails very rarely because
a `CancellationError` was thrown instead of a `URLError`.

This patch updates the test to tolerate both kinds of error at this
stage of the request.

Now the tests do not fail when run repeatedly.
@simonjbeaumont simonjbeaumont enabled auto-merge (squash) January 16, 2024 08:29
@simonjbeaumont simonjbeaumont merged commit 3d1f6e7 into apple:main Jan 16, 2024
5 checks passed
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