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

test: [M3-7367] - Improve Cypress Rescue & Rebuild Test Flake #9867

Conversation

jdamore-linode
Copy link
Contributor

@jdamore-linode jdamore-linode commented Nov 2, 2023

Description 📝

This PR addresses some of the causes of flakiness and failures in our Linode rebuild and rescue Cypress tests.

Changes 🔄

  • Rescue test:
    • Fixed an issue on reattempts where the test would attempt to create a Linode with the same label as in the previous attempt, leading to a consistent Cypress failure stemming from a 400 response from the API on all reattempts
    • Added resource clean up
    • Refactored the Cannot reboot a provisioning Linode into rescue mode test to use mock API data
  • Rebuild test:
    • Addressed primary cause of flakiness by navigating directly to Linode details page and initiating rebuild from there
    • Improve resource clean up by deleting LKE clusters in addition to other resources
    • Refactor tests to use latest patterns, fewer markup-dependent selectors, etc.
    • Refactored the cannot rebuild a provisioning linode to use mock API data
  • Other supporting changes:
    • Mock user preference requests in Linode landing table checks summary view for linode table test to decrease the risk of user preference changes interfering with subsequent tests
    • Add intercept and mock utils, UI helpers

How to test 🧪

Prerequisites

Build and serve Cloud Manager:

yarn && yarn build && yarn start:manager:ci

Verification steps

Make sure that the tests that were modified by this PR continue to pass. We can rely on CI for this, but to run the tests manually you can use this command:

yarn cy:run -s "cypress/e2e/core/linodes/linode-config.spec.ts,cypress/e2e/core/linodes/rebuild-linode.spec.ts,cypress/e2e/core/linodes/rescue-linode.spec.ts,cypress/e2e/core/linodes/smoke-linode-landing-table.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 Nov 2, 2023
@jdamore-linode jdamore-linode requested a review from a team as a code owner November 2, 2023 17:00
@jdamore-linode jdamore-linode requested review from cliu-akamai and abailly-akamai and removed request for a team November 2, 2023 17:00
ui.dialog
.findByTitle(`Rescue Linode ${linode.label}`)
.should('be.visible')
.within(() => {
rebootInRescueMode();
});

// Check mocked response and make sure UI responded correctly.
// Check intercepted response and make sure UI responded correctly.
cy.wait('@rebootLinodeRescueMode')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is still one issue unaddressed by this PR that can cause the test to fail here.

The StandardRescueDialog component fetches the Linode's disks, volumes, etc.. It appears that if the user clicks "Reboot into Rescue Mode" before this data has been fetched, no API request is made (so the call to cy.wait('@rebootLinodeRescueMode') fails). This is unlikely to be encountered by users, but Cypress runs fast enough that it occasionally triggers this issue.

However, this PR addresses the secondary issue with this test, where it would fail consistently on reattempts when this unaddressed issue does occur.

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.

Everything was stable locally!

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.

Looks great! Suite passed locally ✅ - love fixes to the flakes!

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