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

ci: Fix URLSessionClientTest tests #524

Merged
merged 3 commits into from
Oct 30, 2024

Conversation

calvincestari
Copy link
Member

We've been getting frequent failures of the test__request__cancellingTaskThroughClient_shouldNotCallCompletion test in the Main Branch Health nightly build. I fixed the build logs not being archived which lead to the following:

  • We shouldn't be using fatalError for a missing request handler. I think it's a bad practice and the behaviour obscures the actual test failure by crashing instead.
  • Improves the test__request__cancellingTaskThroughClient_shouldNotCallCompletion test by invoking the same delegate callback that happens when a task is cancelled. This means we don't need to wait an arbitrary amount of time for the request callback to be called and can know for certain that the test succeeded.
  • Improves the test__request__taskDescription test by not having to wait an arbitrary amount of time for the test to exit. The test has nothing to do with the completion handler and the task description can be checked and the test satisfied immediately.

@svc-apollo-docs
Copy link
Collaborator

svc-apollo-docs commented Oct 29, 2024

✅ Docs Preview Ready

No new or changed pages found.

Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for eclectic-pie-88a2ba canceled.

Name Link
🔨 Latest commit 72f9ff5
🔍 Latest deploy log https://app.netlify.com/sites/eclectic-pie-88a2ba/deploys/67214b950d0c8400081b3a68

Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for apollo-ios-docc canceled.

Name Link
🔨 Latest commit 72f9ff5
🔍 Latest deploy log https://app.netlify.com/sites/apollo-ios-docc/deploys/67214b95339d6c000862fec4

@calvincestari calvincestari changed the title ci: Fix URLSessionClientTest tests ci: Fix URLSessionClientTest tests Oct 29, 2024
Copy link
Contributor

@AnthonyMDev AnthonyMDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for these improvements!

@calvincestari calvincestari merged commit c5bfeb9 into main Oct 30, 2024
38 checks passed
@calvincestari calvincestari deleted the fix/urlsessionclienttest-underlying-error branch October 30, 2024 20:52
BobaFetters pushed a commit that referenced this pull request Oct 30, 2024
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.

3 participants