Skip to content

test: [M3-9178] - Update/Add integration tests for Create Linode - #12187

Merged
cliu-akamai merged 3 commits into
linode:developfrom
cliu-akamai:feature/M3-9178
Jun 3, 2025
Merged

cliu-akamai merged 3 commits into
linode:developfrom
cliu-akamai:feature/M3-9178

Conversation

@cliu-akamai

@cliu-akamai cliu-akamai commented May 9, 2025

Copy link
Copy Markdown
Contributor

Description 📝

Update / Add integration tests for Create Linode

Changes 🔄

List any change(s) relevant to the reviewer.

  • Add function checkLinodeInterfacesElements()
  • Update legacy integration tests for Create Linode
  • Add Linode Interfaces integration tests for Create Linode

How to test 🧪

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

Issues

  • M3-9955

@cliu-akamai
cliu-akamai requested a review from a team as a code owner May 9, 2025 17:46
@cliu-akamai
cliu-akamai requested review from dmcintyr-akamai and removed request for a team May 9, 2025 17:46
@cliu-akamai
cliu-akamai requested a review from a team as a code owner May 9, 2025 17:49
@cliu-akamai
cliu-akamai requested review from bill-akamai and hana-akamai and removed request for a team May 9, 2025 17:49
@coliu-akamai
coliu-akamai self-requested a review May 9, 2025 20:55

@coliu-akamai coliu-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.

Heads up, seeing a lot of failing Linode Create tests that look relevant

@jdamore-linode
jdamore-linode self-requested a review May 13, 2025 18:03

@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 start @cliu-akamai! I've mainly looked at create-linode-with-firewall.spec.ts so far and it's looking pretty good. I noticed some issues that could be fixed with some mocks, and I suspect that most if not all of the failures we're seeing in CI would be resolved with those applied across these 3 specs. Once that's fixed I'll take a closer look at the other two specs!

Comment on lines +346 to +389
beforeEach(() => {
mockAppendFeatureFlags({
linodeInterfaces: { enabled: true },
});
});

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.

I think we should also mock a couple other things here to get the tests passing reliably in CI since there's now a decent amount of behavior on this page that's impacted by account specific settings:

I think applying these mocks here and applying similar mocks in the other two specs will largely get the tests passing by ensuring that the legacy config interfaces option is selected on page load and that the compliant firewall notice gets shown when attempting to create a Linode without a firewall as an internal user. All put together, I think this block will look something like this:

    mockAppendFeatureFlags({
      linodeInterfaces: { enabled: true },
    });
    mockGetAccountSettings(
      accountSettingsFactory.build({
      interfaces_for_new_linodes: 'legacy_config_default_but_linode_allowed',
      })
    );
    mockGetFirewallSettings({
      default_firewall_ids: {
        linode: null,
        public_interface: null,
        vpc_interface: null,
        nodebalancer: null,
      }
    });

checkLinodeInterfacesElements();

// Switch to Linode Interfaces
cy.findByText(linodeInterfacesLabelText).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.

I think this would be a good opportunity to add a new function or pair of functions to our Linode create page object.

I'm imagining something like linodeCreatePage.selectInterfacesType("linode" | "legacy config") or maybe two functions like linodeCreatePage.selectLinodeInterfacesType() and linodeCreatePage.selectLegacyConfigInterfacesType().

Comment on lines +226 to +340

/**
* Check the elements of Linode Interfaces.
*
* @param linodeInterfacesEnabled - Indicator if Linode Interfaces feature is enabled.
*/
export const checkLinodeInterfacesElements = (
linodeInterfacesEnabled: boolean = true
): void => {
const expectedBehavior = linodeInterfacesEnabled ? 'be.visible' : 'not.exist';
cy.findByText(networkInterfaceTypeSectionText).should(expectedBehavior);
cy.findByText(linodeInterfacesLabelText).should(expectedBehavior);
cy.findByText(linodeInterfacesDescriptionText1).should(expectedBehavior);
cy.findByText(linodeInterfacesDescriptionText2).should(expectedBehavior);
cy.findByText(legacyInterfacesLabelText).should(expectedBehavior);
cy.findByText(legacyInterfacesDescriptionText1).should(expectedBehavior);
cy.findByText(legacyInterfacesDescriptionText2).should(expectedBehavior);
cy.findByText(networkConnectionSectionText).should(expectedBehavior);
cy.findByText(networkConnectionDescriptionText).should(expectedBehavior);
};

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.

Do you think we could rename this to something that describes its purpose a little more clearly? I think something like assertNewLinodeInterfacesIsAvailable or something like that

@dmcintyr-akamai dmcintyr-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.

looks good. i guess this pattern would be. agood way to avoid all the linting warnings about duplicate strings

@coliu-akamai coliu-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.

seeing these tests pass in the github checks, but for some reason when I run pnpm cy:debug or the provided commands, I get test failures:

This screenshot is from the first test in create-linode-with-firewall.spec.ts, where the Linode Interface feature flag is disabled (but I was seeing this happen with a bunch of the legacy tests):
image

any idea what the issue could be?

@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.

Thanks @cliu-akamai! This is looking good and the tests run well!

@github-project-automation github-project-automation Bot moved this from Review to Approved in Cloud Manager May 28, 2025
@linode-gh-bot

Copy link
Copy Markdown

Cloud Manager UI test results

🔺 2 failing tests on test run #6 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
2 Failing626 Passing4 Skipped131m 52s

Details

Failing Tests
SpecTest
lke-create.spec.tsCloud Manager Cypress Tests→LKE Cluster Creation » LKE Cluster Creation
lke-create.spec.tsCloud Manager Cypress Tests→LKE Cluster Creation with APL enabled » LKE Cluster Creation with APL enabled

Troubleshooting

Use this command to re-run the failing tests:

pnpm cy:run -s "cypress/e2e/core/kubernetes/lke-create.spec.ts"

@cliu-akamai
cliu-akamai merged commit c9f2652 into linode:develop Jun 3, 2025
@github-project-automation github-project-automation Bot moved this from Approved to Merged in Cloud Manager Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants