test: [M3-7124] - Add Cypress integration tests for VPC assign/unassign flows #9818

cliu-akamai · 2023-10-19T15:54:10Z

Description 📝

Add new cypress tests for VPC assign/unassign flows.

Major Changes 🔄

Add e2e tests to verify assigning/unassigning Linode(s) to/from a VPC.

How to test 🧪

yarn cy:run -s "cypress/e2e/core/vpc/vpc-linodes-update.spec.ts"

Known Issues

When there are multiple Linodes in the select list, users type the label to select one of them, then it will deselect all when users type another label and select it.
When users click the Unassign Linode button, it doesn’t send any request to tell the VPC to unassign Linodes.

jdamore-linode · 2023-10-19T20:41:07Z

packages/manager/cypress/e2e/core/vpc/vpc-linodes-update.spec.ts

+    cy.visitWithLogin('/linodes');
+    cy.wait(['@getFeatureFlags', '@getClientStream']);
+
+    ui.nav.findItemByTitle('VPC').should('be.visible').click();
+    cy.wait('@getVPCs');
+
+    cy.url().should('endWith', '/vpcs');
+
+    cy.visitWithLogin(`/vpcs/${mockVPC.id}`);
+    cy.wait(['@getVPC', '@getSubnets']);


Do we have to navigate to the VPCs page manually? We might be able to save some time by navigating to /vpcs/${mockVPC.id} directly (I didn't test though!)

Suggested change

cy.visitWithLogin('/linodes');

cy.wait(['@getFeatureFlags', '@getClientStream']);

ui.nav.findItemByTitle('VPC').should('be.visible').click();

cy.wait('@getVPCs');

cy.url().should('endWith', '/vpcs');

cy.visitWithLogin(`/vpcs/${mockVPC.id}`);

cy.wait(['@getVPC', '@getSubnets']);

cy.visitWithLogin(`/vpcs/${mockVPC.id}`);

cy.wait(['@getFeatureFlags', '@getClientStream', '@getVPC', '@getSubnets']);

jdamore-linode

Thanks @cliu-akamai, this looks great!

When there are multiple Linodes in the select list, users type the label to select one of them, then it will deselect all when users type another label and select it.

I'm seeing this too -- I'm guessing this has to do with the issue fixed by #9820 (though I haven't had a chance to look too closely at that PR yet).

When users click the Unassign Linode button, it doesn’t send any request to tell the VPC to unassign Linodes.

Also observing no HTTP request getting fired off upon submitting the drawer, but curious if this might be a data mocking issue -- for example, maybe we have to mock the GET request to the Linode config endpoint with the VPC interface data set up correctly in order for the frontend to make a PUT to remove the VPC.

Would a developer be able to offer any insight here?

jaalah-akamai · 2023-10-20T13:24:01Z

I think both those issues will be resolved in #9820

mjac0bs · 2023-10-26T15:35:29Z

@cliu-akamai #9820 was just merged! Can we get the latest develop merged into this branch?

cliu-akamai · 2023-10-27T14:44:08Z

@cliu-akamai #9820 was just merged! Can we get the latest develop merged into this branch?

Sure thing! I will rebase the code.

cliu-akamai · 2023-10-27T15:15:53Z

After I rebase the code, the Unassign Linodes button is always disabled even though I select a Linode.

coliu-akamai · 2023-10-30T19:19:05Z

Hey Cassie! Could you send a screenshot of what you're seeing for the Unassign Linodes drawer? I'm not seeing the button grey'd out

mjac0bs

Hey Cassie! Could you send a screenshot of what you're seeing for the Unassign Linodes drawer? I'm not seeing the button grey'd out

Looks like this test is failing consistently in the can unassign Linode(s) from a VPC test case once a Linode is selected to be unassigned. The test checks for

        ui.button
          .findByTitle('Unassign Linodes')
          .should('be.visible')
          .should('be.enabled')
          .click();

and fails because it's not enabled. Can't replicate this in the UI, though, like @coliu-akamai mentioned so it seems like there's something in the test. Based on SubnetUnassignLinodesDrawer code, the Unassign Linodes button is only disabled when configInterfacesToDelete.length === 0, so are we not mocking something correctly?

mjac0bs · 2023-10-30T19:42:11Z

packages/manager/cypress/e2e/core/vpc/vpc-linodes-update.spec.ts

+  /*
+   * - Confirms that can assign a Linode to the VPC when feature is enabled.
+   */
+  it.skip('can assign Linode(s) to a VPC', () => {


Let's not forget to unskip this!

Suggested change

it.skip('can assign Linode(s) to a VPC', () => {

it('can assign Linode(s) to a VPC', () => {

jdamore-linode · 2023-10-31T19:26:23Z

Looks like this test is failing consistently in the can unassign Linode(s) from a VPC test case once a Linode is selected to be unassigned. The test checks for
        ui.button
          .findByTitle('Unassign Linodes')
          .should('be.visible')
          .should('be.enabled')
          .click();
and fails because it's not enabled.

Thanks, @mjac0bs!

@cliu-akamai It looks like the root of the issue is that the Linode's mocked config doesn't contain the corresponding VPC interface. Defining the config mock like this:

  // (...Somewhere around line 214...)
  const mockLinodeConfig = configFactory.build({
    interfaces: [LinodeConfigInterfaceFactoryWithVPC.build({
      vpc_id: mockVPC.id,
      subnet_id: mockSubnet.id,
    })],
  });

And then using it on line 259 (instead of mockConfig, which you may be able to remove) allows the unassign button to become enabled!

carrillo-erik · 2023-11-01T06:23:38Z

Don't forget to add a changeset to this PR.

mjac0bs

Thanks, Cassie! This passed locally and seems to accomplish the goals of the flows.

We'll want a Test changeset for this PR (yarn changeset) and I spotted one typo we could fix with the new delete config interface function we're mocking.

mjac0bs · 2023-11-02T21:06:27Z

packages/manager/cypress/support/intercepts/configs.ts

+ *
+ * @returns Cypress chainable.
+ */
+export const mockDeleteLinodeConfignInterface = (


Suggested change

export const mockDeleteLinodeConfignInterface = (

export const mockDeleteLinodeConfigInterface = (

I think this is a small typo. 🔍

coliu-akamai

Just some import conflicts but after these changes the test passes!

coliu-akamai · 2023-11-07T22:49:23Z

packages/manager/cypress/e2e/core/vpc/vpc-linodes-update.spec.ts

+import {
+  mockCreateLinodeConfigInterfaces,
+  mockGetLinodeConfigs,
+  mockDeleteLinodeConfignInterface,


Suggested change

mockDeleteLinodeConfignInterface,

mockDeleteLinodeConfigInterface,

coliu-akamai · 2023-11-07T22:49:57Z

packages/manager/cypress/e2e/core/vpc/vpc-linodes-update.spec.ts

+        );
+        cy.findByText(mockSecondLinode.label).should('be.visible');
+
+        mockDeleteLinodeConfignInterface(


Suggested change

mockDeleteLinodeConfignInterface(

mockDeleteLinodeConfigInterface(

coliu-akamai · 2023-11-07T22:50:09Z

packages/manager/cypress/e2e/core/vpc/vpc-linodes-update.spec.ts

+          mockLinodeConfig.id,
+          mockLinodeConfig.interfaces[0].id
+        ).as('deleteLinodeConfigInterface1');
+        mockDeleteLinodeConfignInterface(


Suggested change

mockDeleteLinodeConfignInterface(

mockDeleteLinodeConfigInterface(

coliu-akamai

Confirmed test passed! Approving pending one small comment regarding the changeset 🎉

coliu-akamai · 2023-11-09T16:15:14Z

packages/manager/.changeset/pr-9818-added-1699024035326.md

@@ -0,0 +1,5 @@
+---
+"@linode/manager": Added


Could we change this to Tests? You'll also need to update the file name itself from pr-9818-added-.. to pr-9818-tests-...

Suggested change

"@linode/manager": Added

"@linode/manager": Tests

jdamore-linode

Nice work @cliu-akamai! One small suggestion to wrap the mocked interfaces in a makeResponse call (doesn't impact your test but could cause issues in other places), but otherwise this looks great!

jdamore-linode · 2023-11-09T16:52:27Z

packages/manager/cypress/support/intercepts/configs.ts

+  return cy.intercept(
+    'POST',
+    apiMatcher(`linode/instances/${linodeId}/configs/${config.id}/interfaces`),
+    config.interfaces


Suggested change

config.interfaces

makeResponse(config.interfaces)

jdamore-linode · 2023-11-09T17:40:10Z

(@cliu-akamai we can also disregard the e2e failure for this, it'll be fixed by PR #9890)

cliu-akamai requested a review from jdamore-linode October 19, 2023 15:54

cliu-akamai requested a review from a team as a code owner October 19, 2023 15:54

cliu-akamai requested review from mjac0bs and jaalah-akamai and removed request for a team October 19, 2023 15:54

mjac0bs assigned cliu-akamai Oct 19, 2023

mjac0bs added the VPC Relating to VPC project label Oct 19, 2023

cliu-akamai changed the title ~~test: [EW-7124] - Add Cypress integration tests for VPC assign/unassign flows~~ test: [M3-7124] - Add Cypress integration tests for VPC assign/unassign flows Oct 19, 2023

jdamore-linode reviewed Oct 19, 2023

View reviewed changes

cpathipa requested review from jdamore-linode and removed request for jaalah-akamai October 23, 2023 17:52

cliu-akamai force-pushed the feature/M3-7124 branch from b327f7b to d6e5c7c Compare October 27, 2023 15:14

coliu-akamai mentioned this pull request Oct 30, 2023

fix: [M3-7348] - Only unassign linodes in the 'Linodes to be Unassigned from Subnet' list for Subnet Unassign Drawer #9851

Merged

12 tasks

mjac0bs reviewed Oct 30, 2023

View reviewed changes

carrillo-erik added the Missing Changeset label Nov 1, 2023

cliu-akamai force-pushed the feature/M3-7124 branch from d6e5c7c to 05834cf Compare November 2, 2023 15:34

mjac0bs approved these changes Nov 2, 2023

View reviewed changes

mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Nov 2, 2023

cliu-akamai force-pushed the feature/M3-7124 branch from 05834cf to 583e39c Compare November 3, 2023 15:11

coliu-akamai reviewed Nov 7, 2023

View reviewed changes

coliu-akamai removed the Missing Changeset label Nov 7, 2023

EW-7124 Add Cypress integration tests for VPC assign/unassign flows

12f2a4a

Added changeset: Add integration tests for VPC assign/unassign flows

9fe8f46

cliu-akamai force-pushed the feature/M3-7124 branch from 583e39c to 9fe8f46 Compare November 9, 2023 15:34

coliu-akamai approved these changes Nov 9, 2023

View reviewed changes

jdamore-linode approved these changes Nov 9, 2023

View reviewed changes

coliu-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Nov 9, 2023

cliu-akamai merged commit c5e1324 into linode:develop Nov 10, 2023
11 checks passed

cliu-akamai deleted the feature/M3-7124 branch November 10, 2023 18:44

mjac0bs mentioned this pull request Dec 11, 2023

Release v1.108.0 - staging → master #9986

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: [M3-7124] - Add Cypress integration tests for VPC assign/unassign flows #9818

test: [M3-7124] - Add Cypress integration tests for VPC assign/unassign flows #9818

cliu-akamai commented Oct 19, 2023

jdamore-linode Oct 19, 2023

jdamore-linode left a comment

jaalah-akamai commented Oct 20, 2023

mjac0bs commented Oct 26, 2023

cliu-akamai commented Oct 27, 2023

cliu-akamai commented Oct 27, 2023

coliu-akamai commented Oct 30, 2023

mjac0bs left a comment

mjac0bs Oct 30, 2023

jdamore-linode commented Oct 31, 2023 •

edited

Loading

carrillo-erik commented Nov 1, 2023

mjac0bs left a comment

mjac0bs Nov 2, 2023

coliu-akamai left a comment

coliu-akamai Nov 7, 2023

coliu-akamai Nov 7, 2023

coliu-akamai Nov 7, 2023

coliu-akamai left a comment

coliu-akamai Nov 9, 2023

jdamore-linode left a comment

jdamore-linode Nov 9, 2023

jdamore-linode commented Nov 9, 2023

	it.skip('can assign Linode(s) to a VPC', () => {
	it('can assign Linode(s) to a VPC', () => {

	export const mockDeleteLinodeConfignInterface = (
	export const mockDeleteLinodeConfigInterface = (

	mockDeleteLinodeConfignInterface,
	mockDeleteLinodeConfigInterface,

	mockDeleteLinodeConfignInterface(
	mockDeleteLinodeConfigInterface(

test: [M3-7124] - Add Cypress integration tests for VPC assign/unassign flows #9818

test: [M3-7124] - Add Cypress integration tests for VPC assign/unassign flows #9818

Conversation

cliu-akamai commented Oct 19, 2023

Description 📝

Major Changes 🔄

How to test 🧪

Known Issues

Choose a reason for hiding this comment

jdamore-linode left a comment

Choose a reason for hiding this comment

jaalah-akamai commented Oct 20, 2023

mjac0bs commented Oct 26, 2023

cliu-akamai commented Oct 27, 2023

cliu-akamai commented Oct 27, 2023

coliu-akamai commented Oct 30, 2023

mjac0bs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdamore-linode commented Oct 31, 2023 • edited Loading

carrillo-erik commented Nov 1, 2023

mjac0bs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coliu-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coliu-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdamore-linode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdamore-linode commented Nov 9, 2023

jdamore-linode commented Oct 31, 2023 •

edited

Loading