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.

Sign up for GitHub

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

Jump to bottom

test: [M3-7071] - Add Cypress integration tests for NodeBalancer Create DC-specific pricing #9765

Merged
merged 1 commit into from
Oct 19, 2023
Merged

test: [M3-7071] - Add Cypress integration tests for NodeBalancer Create DC-specific pricing #9765

merged 1 commit into from
Oct 19, 2023

Conversation

cliu-akamai
Copy link
Contributor

Description 📝

Add new cypress tests for NodeBalancer creation tiered pricing.

Major Changes 🔄

  • Add e2e tests to verify the DC pricing information in NodeBalancer creation flow.

How to test 🧪

yarn cy:run -s "cypress/e2e/core/nodebalancers/smoke-create-nodebal.spec.ts"

@cliu-akamai cliu-akamai requested a review from a team as a code owner October 5, 2023 20:55
@cliu-akamai cliu-akamai requested review from mjac0bs and hana-akamai and removed request for a team October 5, 2023 20:55
abailly-akamai
abailly-akamai reviewed Oct 9, 2023
View reviewed changes
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.

This test is currently not passing for me

Screenshot 2023-10-09 at 11 12 14

@mjac0bs mjac0bs changed the title test: [M3-7071] - Add Cypress integration tests for NodeBalancer Create tiered … test: [M3-7071] - Add Cypress integration tests for NodeBalancer Create DC-specific pricing Oct 11, 2023
mjac0bs
mjac0bs approved these changes Oct 17, 2023
View reviewed changes
Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Confirmed this passed locally and covers DC-pricing changes for NodeBalancers. Thanks for the updates, @cliu-akamai! 🚢

image

packages/manager/cypress/e2e/core/nodebalancers/smoke-create-nodebal.spec.ts
// Confirms that the summary updates to reflect price changes if the user changes their region.
cy.get(`[value="${regionName}"]`).click().type(`${newRegion.label}{enter}`);
cy.get('[data-qa-summary="true"]').within(() => {
cy.findByText(`$14.00/month`).should('be.visible');
Copy link
Contributor

Choose a reason for hiding this comment

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

A note to self: once this PR is merged, #9660, which makes a change to the NodeBalancer summary section to display the monthly total in integers (consistent with Linode and LKE) when the value is not decimal, will need to update this test here and L47 so this test doesn't fail.

packages/manager/cypress/e2e/core/nodebalancers/smoke-create-nodebal.spec.ts Show resolved Hide resolved
@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Oct 17, 2023
hana-akamai
hana-akamai reviewed Oct 17, 2023
View reviewed changes
Copy link
Contributor

@hana-akamai hana-akamai left a comment

Choose a reason for hiding this comment

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

Tests are failing for me 🤔

image

abailly-akamai
abailly-akamai reviewed Oct 18, 2023
View reviewed changes
Copy link
Contributor

@abailly-akamai abailly-akamai left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tests are passing for me ✅

It would be nice to test the submission of multiple nodes and multiple configs since this form is complex. It will help us when refactoring it from a class component.

@jdamore-linode - any thoughts?

@mjac0bs
Copy link
Contributor

mjac0bs commented Oct 19, 2023 •
edited
Loading

It would be nice to test the submission of multiple nodes and multiple configs since this form is complex. It will help us when refactoring it from a class component.

@abailly-akamai Fair, but this sounds like a good candidate for a follow up ticket to me, because it extends beyond DC-specific pricing. I think we can merge this as is, and get something in the backlog for further NodeBalancer Create test coverage. (Edit: M3-7287)

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Oct 19, 2023
@mjac0bs
Copy link
Contributor

mjac0bs commented Oct 19, 2023

@cliu-akamai - Thanks for your work on this! I'm going to go ahead and merge since this test passes in CI and locally for at least two of us. (Not quite sure what @hana-linode was experiencing, but can't replicate it.) I'll then merge develop into #9660 and make the adjustment we need in that PR to avoid a test failure.

@mjac0bs mjac0bs merged commit 2847116 into linode:develop Oct 19, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hana-akamai hana-akamai hana-akamai left review comments

@abailly-akamai abailly-akamai abailly-akamai approved these changes

@mjac0bs mjac0bs mjac0bs approved these changes

@jdamore-linode jdamore-linode Awaiting requested review from jdamore-linode

@bnussman-akamai bnussman-akamai Awaiting requested review from bnussman-akamai

Assignees

@cliu-akamai cliu-akamai

Labels
Approved Multiple approvals and ready to merge! DC-Specific Pricing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cliu-akamai @mjac0bs @hana-akamai @abailly-akamai