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

Disabling flaky test #429

Merged
merged 2 commits into from
Jul 16, 2024
Merged

Disabling flaky test #429

merged 2 commits into from
Jul 16, 2024

Conversation

BobaFetters
Copy link
Member

Disabling the testCancellingTaskThroughClientDoesNotCallCompletion for now because it is flaky and failing inconsistently.

Disabling the `testCancellingTaskThroughClientDoesNotCallCompletion` for now because it is flaky and failing inconsistently.
@BobaFetters BobaFetters self-assigned this Jul 16, 2024
Copy link

netlify bot commented Jul 16, 2024

Deploy Preview for eclectic-pie-88a2ba canceled.

Name Link
🔨 Latest commit 2b2fec2
🔍 Latest deploy log https://app.netlify.com/sites/eclectic-pie-88a2ba/deploys/6696cba4bc000d0008405647

Copy link

netlify bot commented Jul 16, 2024

Deploy Preview for apollo-ios-docc canceled.

Name Link
🔨 Latest commit 2b2fec2
🔍 Latest deploy log https://app.netlify.com/sites/apollo-ios-docc/deploys/6696cba423ec010009538fc5

Copy link
Member

@calvincestari calvincestari left a comment

Choose a reason for hiding this comment

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

We currently have two ways of skipping tests:

  1. Using XCTSkip().
  2. Disabling the test in the test configuration.

This PR does 2 but I think we should switch it to using 1 instead. It looks like we use that method (1) for tests that should be skipped everywhere and the configuration-based method (2) when differentiating between tests that should be run on CI vs. local. XCTSkip() is also easily searchable when we go back to resolve them.

Maybe we shouldn't be doing things this way; worth discussing?

@AnthonyMDev
Copy link
Contributor

Is the test flaky locally as well, or just on CI? If it's just flaky on CI, we should just disable it in the CI test config but not in the unit test config.

@calvincestari
Copy link
Member

Is the test flaky locally as well, or just on CI? If it's just flaky on CI, we should just disable it in the CI test config but not in the unit test config.

So that's a good point. I haven't had it fail for me locally so I guess the intention of this PR is just to skip it for CI then - my bad.

Copy link
Member

@calvincestari calvincestari left a comment

Choose a reason for hiding this comment

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

We should just disable the test in the Apollo-CITestPlan config then.

@BobaFetters BobaFetters merged this pull request into main Jul 16, 2024
30 checks passed
@BobaFetters BobaFetters deleted the ci/disable-flaky-test branch July 16, 2024 19:43
BobaFetters added a commit that referenced this pull request Jul 16, 2024
BobaFetters pushed a commit that referenced this pull request Jul 16, 2024
git-subtree-dir: apollo-ios
git-subtree-mainline: 304b53d
git-subtree-split: 06c1a89127ba5655243a6a2cd014e418899cd5c7
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