Skip to content

test: [M3-7069] - Add Cypress integration tests for Linode Create DC-specific pricing#9701

Merged
mjac0bs merged 4 commits intolinode:developfrom
cliu-akamai:feature/M3-7069
Sep 29, 2023
Merged

test: [M3-7069] - Add Cypress integration tests for Linode Create DC-specific pricing#9701
mjac0bs merged 4 commits intolinode:developfrom
cliu-akamai:feature/M3-7069

Conversation

@cliu-akamai
Copy link
Contributor

Description 📝

Add new cypress tests for Linode creation tiered pricing.

Major Changes 🔄

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

How to test 🧪

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

@cliu-akamai cliu-akamai requested a review from a team as a code owner September 19, 2023 20:58
@cliu-akamai cliu-akamai requested review from abailly-akamai and bnussman-akamai and removed request for a team September 19, 2023 20:58
@cliu-akamai cliu-akamai changed the title M3-7069 Add Cypress integration tests for Linode Create tiered pricing test: [M3-7069] - Add Cypress integration tests for Linode Create tiered pricing Sep 19, 2023
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 good! couple comments:

  • I would consider naming the file create-linode-dynamic-pricing.spec.ts so it's clear what the focus of the test is at first sight
  • I am getting "The selected region does not have any availability for the requested service plan." when runnign locally. Should we consider getting a much lower plan to run this test to avoid this kind of issue?

@mjac0bs mjac0bs changed the title test: [M3-7069] - Add Cypress integration tests for Linode Create tiered pricing test: [M3-7069] - Add Cypress integration tests for Linode Create DC-specific pricing Sep 20, 2023
@jdamore-linode
Copy link
Contributor

  • I would consider naming the file create-linode-dynamic-pricing.spec.ts so it's clear what the focus of the test is at first sight.

@abailly-akamai For DC pricing, we're keeping these tests side-by-side with the existing tests because when the feature launches and we clean up the feature flag, all of the new tests will be removed, and the DC-pricing-specific assertions will be refactored into the existing tests.

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.

Nice work @cliu-akamai! Everything looks good to me, just a quick suggestion for a couple of your getClick() calls!

authenticate();
describe('create linode', () => {
before(() => {
cleanUp('linodes');
Copy link
Contributor

Choose a reason for hiding this comment

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

Great catch 🔍! Thanks @cliu-akamai!

// Confirm that the checkout summary at the bottom of the page reflects the correct price.
containsClick(selectRegionString).type(`${initialRegion.label} {enter}`);
fbtClick('Shared CPU');
getClick('[id="g6-standard-8"]');
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
getClick('[id="g6-standard-8"]');
getClick(`[id="${dcPricingMockLinodeTypes[0].id}"]`);

Coincidentally, I just posted an explanation on another PR about this and why hardcoding the ID here might make the tests fragile in unexpected ways!

// Confirms that the summary updates to reflect price changes if the user changes their region and plan selection.
containsClick(initialRegion.label).type(`${newRegion.label} {enter}`);
fbtClick('Shared CPU');
getClick('[id="g6-standard-8"]');
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
getClick('[id="g6-standard-8"]');
getClick(`[id="${dcPricingMockLinodeTypes[0].id}"]`);

@jdamore-linode
Copy link
Contributor

jdamore-linode commented Sep 21, 2023

  • I am getting "The selected region does not have any availability for the requested service plan." when runnign locally. Should we consider getting a much lower plan to run this test to avoid this kind of issue?

@abailly-akamai, are you seeing this message when the test attempts to create the Linode or at another point during the create flow? Haven't been able to reproduce this and can't think of why this might be happening off the top of my head, but I'd like to get to the bottom of it before merging if possible

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.

Test passed and looked solid 👍🏼

@jaalah-akamai jaalah-akamai added the Approved Multiple approvals and ready to merge! label Sep 22, 2023
@jdamore-linode
Copy link
Contributor

@cliu-akamai Just circling back here to thank you for those additions! Took a look at those and confirmed they're passing locally.

I think you should be able to fix the automated test failures by merging in the latest changes from develop, but if it continues giving us trouble I'll run the suite locally and confirm they're passing

@abailly-akamai
Copy link
Contributor

@jdamore-linode sorry for the delay in answering. It happened to me when running the test locally - could it be because my regions list (locally) contains regions the production build CY test against wouldn't, and those regions have no availability (I have a bunch of flags on my account to that effect, ie: us-east-1b)?

@mjac0bs
Copy link
Contributor

mjac0bs commented Sep 28, 2023

@cliu-akamai I merged in the latest develop here and looks like all tests are passing. linodes/create-linode.spec.ts also passes locally and with two approvals, I think this is good to merge! 🚢

Edit: @jdamore-linode - Are we still concerned about the issue that Alban was seeing locally? I also can't reproduce.

@mjac0bs
Copy link
Contributor

mjac0bs commented Sep 29, 2023

I'm going to merge this now since only one of us seemed to have an issue with local runs, but make a note of this to revisit if it causes more problems with the tests in the future.

@mjac0bs mjac0bs merged commit 48eab72 into linode:develop Sep 29, 2023
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! DC-Specific Pricing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants