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

Add more tests around stopping fetchers with callbacks. #414

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

thomasvl
Copy link
Member

@thomasvl thomasvl commented Nov 7, 2024

Some of these cases are XCTSkipped as they reliable fail when run locally. This will be follow up work to fix them.

These tests are likely to be racy since they are just using delays, but they do catch some fail cause already. Some could be made non racy if the authorizer delay and the stop ordering was perfectly synchronized, but that is likely to be a fair amount of complexity to ensure it is done correctly and non blocking; so it is deferred for now.

I also haven't reasoned through why the Service versions currently all pass when the non service cases fail. That will get some attention during the followup to fix the existing failures.

Some of these cases are XCTSkipped as they reliable fail when run locally. This
will be follow up work to fix them.

These tests are likely to be racy since they are just using delays, but they do
catch some fail cause already. Some could be made non racy if the authorizer
delay and the stop ordering was perfectly synchronized, but that is likely to be
a fair amount of complexity to ensure it is done correctly and non blocking; so
it is deferred for now.

I also haven't reasoned through why the Service versions currently all pass when
the non service cases fail. That will get some attention during the followup to
fix the existing failures.
@thomasvl thomasvl requested a review from dmaclach November 7, 2024 19:34
@thomasvl thomasvl merged commit e18fe7c into google:main Nov 8, 2024
21 checks passed
@thomasvl thomasvl deleted the more_cancel_tests branch November 8, 2024 17:35
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