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

refactor: [M3-7650] - Refactor Cypress region utils, address region capacity-related flake #10242

Conversation

jdamore-linode
Copy link
Contributor

Description 📝

This refactors our Cypress region handling utilities and makes a few changes intended to eliminate test failures related to region capacity, and better position us for upcoming changes (e.g. GECKO, DC Get Well).

Changes 🔄

  • Disallows regions with limited capacity from being returned by the chooseRegion(s) utilities -- this is the most relevant change w.r.t. test stability, and is done using a simple blacklist (but could perhaps be done dynamically in the future)
  • Refactors the chooseRegion and chooseRegions utilities
    • Adds the ability to specify multiple required capabilities when choosing regions (previously only 1 capability could be specified)
    • Adds the ability to specify capabilities for chooseRegions (previously only able to be done for chooseRegion)
    • Adds the ability to specify regions from which to select (allowing the utils to be used when mocking regions)
    • Preemptively throws errors in certain circumstances that would otherwise result in test failures (this allows the tests to fail faster, rather than timing out waiting for some response or UI condition, and prevents certain API requests from being made in quick succession -- see point below regarding API rate limiting)
    • Reorganizes the region selection/filtering logic
  • Attempts to mitigate or resolve recent API 429/rate limiting issues
    • I think this issue is closely related to the regions issue (Cloud attempts to create resources using a bad region -> gets a 400 -> fails, tries again more 2 times in quick succession -> moves on to another test which does the same thing -> gets rate limited). Some of the changes made to the chooseRegion(s) utils mentioned above relate to this, and I've also added a 15 second timeout when Cypress (not Cloud) fires an API request that results in a 429. I'll keep an eye on this to see if makes any impact.

How to test 🧪

We can rely on the CI for this. Assuming there are no region-related failures (or other significant failures), I may run the tests multiple times just to build confidence in the changes before going forward.

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 Feb 29, 2024
@jdamore-linode jdamore-linode requested a review from a team as a code owner February 29, 2024 21:35
@jdamore-linode jdamore-linode requested review from cliu-akamai and removed request for a team February 29, 2024 21:35
Copy link

github-actions bot commented Feb 29, 2024

Coverage Report:
Base Coverage: 81.32%
Current Coverage: 81.32%

@jdamore-linode jdamore-linode requested a review from a team as a code owner February 29, 2024 22:09
@jdamore-linode jdamore-linode requested review from jaalah-akamai and abailly-akamai and removed request for a team February 29, 2024 22:09
Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

Nice and well self documented utils. thanks!

this is going to be useful ✅

@jdamore-linode jdamore-linode merged commit f138153 into linode:develop Mar 4, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants