test: [M3-8395] - Fix Linode delete test flake by fixing deletion assertion

[jdamore-linode]

Description 📝

This PR attempts to address or improve the test flakiness we've been seeing in smoke-delete-linode.spec.ts deleting multiple linodes with action menu.

What initially caused this test to become flaky is still unclear, but the majority of the failures I've observed follow this sequence of events:

1. Cypress deletes the first Linode
2. After the deletion request succeeds, but before Cloud has updated the landing page list to reflect the deletion, Cypress opens the action menu for the second Linode
3. Before Cypress is able to click the "Delete" menu item, the Linode landing page updates to reflect the deletion of the initial Linode, causing the opened action menu to close
4. Cypress is unable to click "Delete" because the action menu item is no longer on-screen, and the test fails

We had an assertion that was intended to make Cypress wait for the landing page to refresh before proceeding with the second deletion:

cy.wait('@deleteLinode').its('response.statusCode').should('eq', 200);
cy.findByText(linodeLabel).should('not.exist');

However, I was observing that frequently Cypress would proceed with the test even while the initial Linode was still present in the landing page. My understanding is that this is the result of a strange interaction between Testing Library (which provides the findByText() command and similar commands) and Cypress (which provides the should('not.exist') assertion and others).

When calling cy.findByText() with an element that appears more than once on-screen, the command normally fails with an error explaining that multiple items were matched. When using the should('not.exist') assertion, however, Cypress appears to interpret this state (that multiple elements exist) as equivalent to there being no elements, causing the assertion to succeed before it's expected to -- perhaps this is a result of some naive logic where should('not.exist') treats any command/element selection error as equivalent to being unable to find/select any element.

Minimal reproduction:

it.only('weird cypress assertion issue', () => {
    // Assumes empty Linode landing page.
    cy.visitWithLogin('/linodes');
    // Wait for landing page to load by waiting for blurb to appear...
    cy.findByText('Cloud-based virtual machines').should('be.visible');

    // Observe that this assertion succeeds when it should not:
    // "Linodes" is visible in the sidebar navigation and in the empty landing page,
    // yet the assertion succeeds anyway.
    cy.findByText('Linodes').should('not.exist');
  });

it.only('weird cypress assertion issue', () => {
    // Assumes empty Linode landing page.
    cy.visitWithLogin('/linodes');
    // Wait for landing page to load by waiting for blurb to appear...
    cy.findByText('Cloud-based virtual machines').should('be.visible');

    // Achieves the desired behavior:
    // The test fails because "Linodes" is present.
    cy.findAllByText('Linodes').should('not.exist');
  });

In the case of this test, the Linode label being searched for appears twice on screen: once in the deletion modal dialog which is still present in the DOM when the assertion executes, and again in the landing page table itself.

This could have been fixed in a few ways (narrowing our selection to the Linode landing page table before asserting that the label doesn't exist would've been one alternative), but in this case I opted to fix it by using cy.findAllByText(...) instead of cy.findByText(...).

Changes 🔄

• Ensure that Cypress waits for Linode to be deleted by using cy.findAllByText() instead of cy.findByText()

Target release date 🗓️

N/A

How to test 🧪

Check out this branch, run yarn && yarn build && yarn start:manager:ci, and then run the test a few times to gain confidence that it's stable:

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

(I ran the test locally in isolation and it passed 50/50 times, then I ran the entire spec and it passed 10/10 times)

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

[hkhalil-akamai]

Thank you @jdamore-linode for continuing to improve the reliability of our test suite! 🙌🏽

Tested and passed 50x locally:

I'm almost tempted not to ruin the perfect but should we add a changeset?

[github-actions]

Coverage Report: ✅
Base Coverage: 87.13%
Current Coverage: 87.13%

[hkhalil-akamai]

Thanks for the interesting context! I wonder if it would be good to document this somewhere?

[jdamore-linode]

Thank you @jdamore-linode for continuing to improve the reliability of our test suite! 🙌🏽

Tested and passed 50x locally:

(Screenshot)

I'm almost tempted not to ruin the perfect but should we add a changeset?

Pushed the changeset! And the nice tidy +1/-1 diff can be forever memorialized in your comment 😀

Edit: And yes! This should definitely be documented, although it's not clear to me if this is a bug in Cypress or Testing Library of if it's the expected behavior. I can make sure that gets included in the docs improvements that I'm already working on 👍

[abailly-akamai]

Thanks for the details explanation!

One less flaky test, confirmed things passing consistently on my end ✅