Skip to content

Conversation

@cliu-akamai
Copy link
Contributor

Description 📝

Add new Cypress tests for QEMU Upgrade Notice

Changes 🔄

List any change(s) relevant to the reviewer.

  • Add test suite QEMU reboot upgrade notification.
  • Verify the reboot banner and notifications.
  • Verify the QEMU reboot upgrade notice is displayed in the Linode landing page, Linode Details page, and Account Maintenance page.

How to test 🧪

pnpm cy:run -s "cypress/e2e/core/notificationsAndEvents/qemu-reboot-upgrade-notice.spec.ts"

@cliu-akamai cliu-akamai requested a review from a team as a code owner July 23, 2025 16:37
@cliu-akamai cliu-akamai requested review from jdamore-linode and removed request for a team July 23, 2025 16:37
@cliu-akamai cliu-akamai requested a review from a team as a code owner July 23, 2025 16:41
@cliu-akamai cliu-akamai requested review from bnussman-akamai and pmakode-akamai and removed request for a team July 23, 2025 16:41
@mjac0bs mjac0bs changed the title M3-10051 Add Cypress tests for QEMU Upgrade Notice test: [M3-10051] - Add Cypress tests for QEMU Upgrade Notice Jul 23, 2025
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 692 passing tests on test run #2 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing692 Passing4 Skipped118m 28s

Comment on lines 60 to 69
const mockLinodes = new Array(5)
.fill(null)
.map((item: null, index: number): Linode => {
return linodeFactory.build({
label: `Linode ${index}`,
region: chooseRegion({
capabilities: ['Linodes', 'Vlans'],
}).id,
});
});
Copy link
Member

Choose a reason for hiding this comment

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

Optional: We may be able to use linodeFactory.buildList here

Suggested change
const mockLinodes = new Array(5)
.fill(null)
.map((item: null, index: number): Linode => {
return linodeFactory.build({
label: `Linode ${index}`,
region: chooseRegion({
capabilities: ['Linodes', 'Vlans'],
}).id,
});
});
const mockLinodes = linodeFactory.buildList(5, {
region: chooseRegion({
capabilities: ['Linodes', 'Vlans'],
}).id,
});

* - Check that user gets notified and the notification is present in the notifications dropdown.
* - Check that a maintenance help button is displayed near the status of impacted Linodes.
*/
it(`should display maintenance banner in 'Linnode' landing page when one or more Linodes get impacted.`, () => {
Copy link
Member

Choose a reason for hiding this comment

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

Typo

Suggested change
it(`should display maintenance banner in 'Linnode' landing page when one or more Linodes get impacted.`, () => {
it(`should display maintenance banner in 'Linode' landing page when one or more Linodes get impacted.`, () => {

Comment on lines 319 to 320
cy.get('[data-testid="notice-warning"]')
.first()
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to worry about this causing test flake if there is a maintenance banner up or some other global notification?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could become an issue, good call @bnussman-akamai

@cliu-akamai a lot of times do get around this risk, we'll select by the text first and then if we need to we can get the notification banner from there (although a lot of times asserting that the text is visible is good enough), e.g.:

cy.findByText(NOTIFICATION_BANNER_TEXT)
    .should('be.visible')
    .closest('[data-testid="notice-warning"]')
    .within(() => { /* ... */ });

// or cy.contains(NOTIFICATION_BANNER_TEXT).[...]

Copy link
Contributor

@jdamore-linode jdamore-linode left a comment

Choose a reason for hiding this comment

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

Thanks @cliu-akamai! +1 to all of @bnussman-akamai's suggestions but overall this is looking good 👍

* - Check that user gets notified and the notification is present in the notifications dropdown.
* - Check that a maintenance help button is displayed near the status of impacted Linodes.
*/
it(`should display maintenance banner in 'Linnode' landing page when one or more Linodes get impacted.`, () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
it(`should display maintenance banner in 'Linnode' landing page when one or more Linodes get impacted.`, () => {
it(`should display maintenance banner in 'Linode' landing page when one or more Linodes get impacted.`, () => {

Comment on lines 319 to 320
cy.get('[data-testid="notice-warning"]')
.first()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could become an issue, good call @bnussman-akamai

@cliu-akamai a lot of times do get around this risk, we'll select by the text first and then if we need to we can get the notification banner from there (although a lot of times asserting that the text is visible is good enough), e.g.:

cy.findByText(NOTIFICATION_BANNER_TEXT)
    .should('be.visible')
    .closest('[data-testid="notice-warning"]')
    .within(() => { /* ... */ });

// or cy.contains(NOTIFICATION_BANNER_TEXT).[...]

@github-project-automation github-project-automation bot moved this from Review to Approved in Cloud Manager Jul 29, 2025
@cliu-akamai cliu-akamai merged commit 0ce357c into linode:develop Aug 1, 2025
34 of 35 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Merged in Cloud Manager Aug 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants