Skip to content

test: [M3-7465] - Add Cypress test coverage for Firewall renaming - #10384

Merged
cliu-akamai merged 4 commits into
linode:developfrom
cliu-akamai:feature/M3-7465
May 1, 2024
Merged

cliu-akamai merged 4 commits into
linode:developfrom
cliu-akamai:feature/M3-7465

Conversation

@cliu-akamai

Copy link
Copy Markdown
Contributor

Description 📝

Add regression tests to rename firewall label on landing page.

Major Changes 🔄

  • Check user can update a Firewall's label from the Firewall details page
  • Check label shown on details page updates to reflect change

How to test 🧪

yarn cy:run -s "cypress/e2e/core/firewalls/update-firewall.spec.ts"

@cliu-akamai
cliu-akamai requested a review from a team as a code owner April 16, 2024 17:46
@github-actions

github-actions Bot commented Apr 16, 2024 •

Copy link
Copy Markdown

Coverage Report: ✅
Base Coverage: 81.82%
Current Coverage: 81.82%

@mjac0bs mjac0bs changed the title M3-7465 Add Cypress test coverage for Firewall renaming test: [M3-7465] - Add Cypress test coverage for Firewall renaming Apr 17, 2024
@jdamore-linode
jdamore-linode self-requested a review April 22, 2024 16:45

@jdamore-linode jdamore-linode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work @cliu-akamai! Approved pending changesets. Also posted a couple really minor suggestions but this is great as-is. Thanks!


cy.visitWithLogin(`/firewalls/${firewall.id}`);

cy.get(`[aria-label="Edit ${firewall.label}"]`).click();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
cy.get(`[aria-label="Edit ${firewall.label}"]`).click();
cy.findByLabelText(`Edit ${firewall.label}`).click();

Slightly more clear this way!

Comment on lines +446 to +454
cy.reload();

// Confirm firewall label is updated on details page.
cy.findByText(newFirewallLabel).should('be.visible');

cy.visitWithLogin('/firewalls');

// Confirm firewall label is updated on landing page.
cy.findByText(newFirewallLabel).closest('tr').should('be.visible');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
cy.reload();
// Confirm firewall label is updated on details page.
cy.findByText(newFirewallLabel).should('be.visible');
cy.visitWithLogin('/firewalls');
// Confirm firewall label is updated on landing page.
cy.findByText(newFirewallLabel).closest('tr').should('be.visible');
// Confirm Firewall label updates in breadcrumbs.
ui.entityHeader
.find()
.within(() => {
cy.findByText(newFirewallLabel).should('be.visible');
cy.findByText('firewalls').click();
});
// Confirm firewall label is updated on landing page without refresh.
cy.findByText(newFirewallLabel).should('be.visible');
// Confirm firewall label is updated on landing page after refresh.
cy.reload();
cy.findByText(newFirewallLabel).should('be.visible');

Just a really minor suggestion to slightly improve the coverage by confirming that the label update takes effect without a page refresh (i.e. the React Query cache was successfully updated) and then refreshes the page and confirms the label again (confirming that the label has been updated successfully on the backend, too).

@cliu-akamai
cliu-akamai requested a review from a team as a code owner April 24, 2024 16:36
@cliu-akamai
cliu-akamai requested review from carrillo-erik and dwiley-akamai and removed request for a team April 24, 2024 16:36

@dwiley-akamai dwiley-akamai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Test passes locally & remotely ✅

});

/*
* - Confirms that firewall's label can be updated on landing page'.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* - Confirms that firewall's label can be updated on landing page'.
* - Confirms that firewall's label can be updated on landing page.

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! Ready for Review labels Apr 26, 2024
@cliu-akamai
cliu-akamai merged commit c4b8259 into linode:develop May 1, 2024
@cliu-akamai
cliu-akamai deleted the feature/M3-7465 branch May 1, 2024 19:33
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.

5 participants