Skip to content

test: Resolve Cypress test flake for Linode config edit test#9781

Merged
jdamore-linode merged 3 commits intolinode:developfrom
jdamore-linode:cypress-config-test-flake
Oct 16, 2023
Merged

test: Resolve Cypress test flake for Linode config edit test#9781
jdamore-linode merged 3 commits intolinode:developfrom
jdamore-linode:cypress-config-test-flake

Conversation

@jdamore-linode
Copy link
Contributor

Description 📝

This addresses the flake and failures we've been having with the Edits an existing config test in linodes/linode-config.spec.ts.

The tests were failing because the API would respond with a 400 upon editing a config while a Linode was still provisioning; specifically, the failure occurs when the disks listed in the Linode's config aren't ready when editing the config.

This fixes the issue by waiting for the Linode's disks to be ready before attempting to edit the configs.

Changes 🔄

List any change relevant to the reviewer.

  • Add pollLinodeDiskStatuses() polling utility
  • Wait for Linode disks to be ready before proceeding with config edit test

How to test 🧪

Without these changes, the Edit Config test fails relatively frequently (maybe 1 in 4 runs? Only estimating), but with these changes in place I was able to run the edit test 20 times in a row and they all passed.

yarn cy:run -s "cypress/e2e/core/linodes/linode-config.spec.ts"

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@jdamore-linode jdamore-linode self-assigned this Oct 11, 2023
@jdamore-linode jdamore-linode requested a review from a team as a code owner October 11, 2023 15:45
@jdamore-linode jdamore-linode requested review from carrillo-erik and cpathipa and removed request for a team October 11, 2023 15:45
Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Thanks Joe! The test was stable when I ran it locally and is passing in CI!

Screenshot 2023-10-11 at 2 05 07 PM

@mjac0bs mjac0bs self-requested a review October 11, 2023 20:43
Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Ran the spec locally 4 times and tests passed 100% of the time. 🚢

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Ready for Review labels Oct 11, 2023
@mjac0bs
Copy link
Contributor

mjac0bs commented Oct 12, 2023

Intentionally missing a changeset here?

As far as the failing e2es: longview/longview.spec.ts looks like an unrelated fluke failure. I believe the other runner seems to have run into the 504 issues with the account/ endpoint.

@jdamore-linode
Copy link
Contributor Author

Intentionally missing a changeset here?

As far as the failing e2es: longview/longview.spec.ts looks like an unrelated fluke failure. I believe the other runner seems to have run into the 504 issues with the account/ endpoint.

Haha, no more intentionally missing changesets from my end -- just slipped through the cracks!

The runner issue is unfortunate and has been impacting pretty much all of our runs today (I'm not sure what changed to cause this, but there was some discussion in the API frontend channel about it for more context). In the meantime, I just swapped in a new test account as a temporary workaround so runner #1 should stop failing prematurely now 🤞

I'm going to kick off a new run since we missed out on a quarter of the tests, but yeah, the longview one is another that can be disregarded because it's been generally troublesome lately :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants