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.

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

test: [M3-8060, M3-8061] - Cypress tests for Linode Create v2 flows #10469

Merged
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0a7d2cf
Add comment to Linode Create spec file
May 1, 2024
e8061d5
Rename Linode Create tests to `legacy-create-linode.spec.ts`
May 1, 2024
f975764
Add `data-qa-` attributes, UI helper to select tab panels
May 1, 2024
3eee4f7
Format Linode summary plan type label
May 1, 2024
2c93978
Add data-qa-paper attribute to papers
May 1, 2024
8ba1012
Add Linode create timeout constant
May 1, 2024
0d41524
Add page utilities to ease interaction with Linode create and VPC cre…
May 1, 2024
ef1d6af
Add v2 Linode create end-to-end tests for each plan type
May 1, 2024
70d751e
Add v2 Linode create integration tests for VPC assignment
May 1, 2024
784cfa5
Refactor accordion helpers to account for ReactNode accordion titles
May 1, 2024
4abe2f3
Add WIP Linode Create w/ VLAN tests
May 1, 2024
030a7ed
Add WIP Linode Create w/ SSH Key and user data specs
May 1, 2024
da5d293
Merge branch 'develop' into new-linode-create-cypress-tests
May 2, 2024
9cc4abf
Add Linode Create w/ VLAN integration tests
May 2, 2024
52d6e8f
Create Linode create mobile screen size tests
May 2, 2024
5ed6fc2
Merge branch 'develop' into new-linode-create-cypress-tests
May 14, 2024
54c3838
Update Linode Create plan panel selection to account for recent refactor
May 14, 2024
c13bea7
Add WIP Linode Create user data UI tests
May 14, 2024
a9c3a04
Merge branch 'develop' into new-linode-create-cypress-tests
May 14, 2024
ad2dfc0
Restore user data payload assertion, remove unneeded comments from fi…
May 14, 2024
4724234
Refactor `mockGetImage` to be more flexible
May 14, 2024
053ebb0
Add user data tests for Images and Regions lacking cloud-init capability
May 14, 2024
93d36a7
Add tests and mock utils for Linode create SSH key flows
May 14, 2024
e8b88f8
Mock Linode create feature flag to be off for legacy create tests
May 14, 2024
e26594d
Add changesets
May 14, 2024
167ee2c
Undo `ui.tabList` changes and rework Linode create page interactions;…
May 14, 2024
a7160c0
Merge branch 'develop' into new-linode-create-cypress-tests
May 15, 2024
5d66f93
Only test iPhone portrait viewport for mobile tests as experiment
May 20, 2024
8e7bea7
Merge branch 'develop' into new-linode-create-cypress-tests
May 20, 2024
4a8773f
Merge branch 'develop' into new-linode-create-cypress-tests
May 21, 2024
0716396
Merge branch 'develop' into new-linode-create-cypress-tests
May 23, 2024
2a7d535
Added changeset: Add Cypress test coverage for Linode Create v2 flow
May 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10469-tests-1715717780880.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tests
---

Add Linode Create v2 end-to-end tests ([#10469](https://github.com/linode/manager/pull/10469))
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10469-tests-1715717804841.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tests
---

Add Linode Create v2 integration tests for VLAN flows ([#10469](https://github.com/linode/manager/pull/10469))
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10469-tests-1715717849362.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tests
---

Add Linode Create v2 integration tests for VPC flows ([#10469](https://github.com/linode/manager/pull/10469))
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10469-tests-1715717865422.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tests
---

Add Linode Create v2 integration tests for SSH key flows ([#10469](https://github.com/linode/manager/pull/10469))
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10469-tests-1715717889337.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tests
---

Add Linode Create v2 integration tests for cloud-init flows ([#10469](https://github.com/linode/manager/pull/10469))
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/**
* @file Smoke tests for Linode Create flow across common mobile viewport sizes.
*/

import { linodeFactory } from 'src/factories';
import { MOBILE_VIEWPORTS } from 'support/constants/environment';
import { linodeCreatePage } from 'support/ui/pages';
import { randomLabel, randomNumber, randomString } from 'support/util/random';
import { chooseRegion } from 'support/util/regions';
import {
mockAppendFeatureFlags,
mockGetFeatureFlagClientstream,
} from 'support/intercepts/feature-flags';
import { makeFeatureFlagData } from 'support/util/feature-flags';
import { ui } from 'support/ui';
import { mockCreateLinode } from 'support/intercepts/linodes';

describe('Linode create mobile smoke', () => {
// TODO Remove feature flag mocks when `linodeCreateRefactor` flag is retired.
beforeEach(() => {
mockAppendFeatureFlags({
linodeCreateRefactor: makeFeatureFlagData(true),
});
mockGetFeatureFlagClientstream();
});

MOBILE_VIEWPORTS.forEach((viewport) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the primary goal of testing different viewports in your opinion?

I usually find that feature quite useful for visual regression tests, however I am failing to find the tradeoff valuable for integration test (tradeoff being coverage VS suite time execution).

By looping here we're testing exactly the same functionality (we're also looping from inside the loop) and not that there's anything wrong with that, but our application is responsive out of the box, meaning that we usually only handle responsiveness via CSS media queries, which usually don't alter functionality.

Again, this is a very important flow so there is value here, but I wonder if there's concern in blindly duplicating runs when not needed in the long term (aka, lemme just use of MOBILE_VIEWPORTS forEach in my new cause it's really convenient and covers everything πŸ˜„). At the very least there may be value in build guard rails by making it a util to which we can add solid JSDocs warnings as to:

  • when it should be used
  • why it should be used
  • when it's not needed at all

Don't get me wrong, we NEED to get better at testing mobile sizes (esp when we conditionally render based on the viewport, ie: CTAs in a table row) and I can see this being really helpful, just wanted to point out potential scaling thoughts!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abailly-akamai This is a good question and all the points you raised here are extremely relevant (and I'll also add that in addition to the test run length considerations, I'm also kind of reluctant to do this because of how easily and dramatically it could increase our Cypress Cloud usage)

What is the primary goal of testing different viewports in your opinion?

You kinda touched on it, but my main goal here is to reduce the risk of there being some weird styling issue (or other component issue) that would prevent the Linode Create flow (or any other flow) from working on mobile devices/small screen sizes. My fear is that if something were to break for small screen sizes, however unlikely, we have such little visibility into it that an issue could sit around for a while before we even notice it.

Some (perhaps contrived) examples that come to mind:

  • Some element or button that's critical to a flow gets cut off or overflows in such a way that the user is prevented from interacting with it (and therefore can't complete the flow)
  • Some component that's only present on small screen sizes (e.g. plan selection cards) breaks
  • Some element or button behaves unexpectedly with touch interactions vs clicks
    • (This is a moot point right now since the mobile test on this PR still uses clicks, but we have had cases like this; one that comes to mind is a tooltip issue where desktop users could open the tooltip with a mouse hover, but mobile users were prevented from opening it because iirc the touch event was triggering the parent component and not the tooltip button inside of it)

I wonder if there's concern in blindly duplicating runs when not needed in the long term (aka, lemme just use of MOBILE_VIEWPORTS forEach in my new cause it's really convenient and covers everything πŸ˜„)

Yeah, this is a legitimate concern of mine, especially when it comes to our Cypress Cloud usage! And I think if we do go with an approach like this, we should definitely think about whether all these screen sizes in the MOBILE_VIEWPORTS array are actually necessary; we might be able to get away with having only 1 or 2 sizes covered. What's in there now is almost definitely overkill.

(I wonder if we have any analytics telling us what screen sizes are the most common among our users?)

Don't get me wrong, we NEED to get better at testing mobile sizes (esp when we conditionally render based on the viewport, ie: CTAs in a table row) and I can see this being really helpful, just wanted to point out potential scaling thoughts!

Absolutely! I think we should be doing something, but there's a good argument to be made that this might not be exactly the right solution (or perhaps requires some guard rails, like you said). This is something I'll keep thinking about, and I'd love to hear your thoughts if any other ideas come to mind!

['portrait', 'landscape'].forEach((orientation) => {
/*
* - Confirms Linode create flow can be completed on common mobile screen sizes
* - Creates a basic Nanode and confirms interactions succeed and outgoing request contains expected data.
*/
it(`can create Linode (${viewport.label}, ${orientation})`, () => {
const mockLinodeRegion = chooseRegion();
const mockLinode = linodeFactory.build({
id: randomNumber(),
label: randomLabel(),
region: mockLinodeRegion.id,
});

mockCreateLinode(mockLinode).as('createLinode');

cy.viewport(
orientation === 'portrait' ? viewport.width : viewport.height,
orientation === 'portrait' ? viewport.height : viewport.width
);
cy.visitWithLogin('/linodes/create');

linodeCreatePage.selectImage('Debian 11');
linodeCreatePage.selectRegionById(mockLinodeRegion.id);
linodeCreatePage.selectPlanCard('Shared CPU', 'Nanode 1 GB');
linodeCreatePage.setLabel(mockLinode.label);
linodeCreatePage.setRootPassword(randomString(32));

cy.get('[data-qa-linode-create-summary]')
.scrollIntoView()
.within(() => {
cy.findByText('Nanode 1 GB').should('be.visible');
cy.findByText('Debian 11').should('be.visible');
cy.findByText(mockLinodeRegion.label).should('be.visible');
});

ui.button
.findByTitle('Create Linode')
.should('be.visible')
.should('be.enabled')
.click();

cy.wait('@createLinode').then((xhr) => {
const requestBody = xhr.request.body;

expect(requestBody['image']).to.equal('linode/debian11');
expect(requestBody['label']).to.equal(mockLinode.label);
expect(requestBody['region']).to.equal(mockLinodeRegion.id);
expect(requestBody['type']).to.equal('g6-nanode-1');
});

cy.url().should('endWith', `/linodes/${mockLinode.id}`);
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
import {
accountUserFactory,
linodeFactory,
sshKeyFactory,
} from 'src/factories';
import {
mockAppendFeatureFlags,
mockGetFeatureFlagClientstream,
} from 'support/intercepts/feature-flags';
import { makeFeatureFlagData } from 'support/util/feature-flags';
import { randomLabel, randomNumber, randomString } from 'support/util/random';
import { chooseRegion } from 'support/util/regions';
import { mockGetUser, mockGetUsers } from 'support/intercepts/account';
import { mockCreateLinode } from 'support/intercepts/linodes';
import { linodeCreatePage } from 'support/ui/pages';
import { ui } from 'support/ui';
import { mockCreateSSHKey } from 'support/intercepts/profile';

describe('Create Linode with SSH Key', () => {
// TODO Remove feature flag mocks when `linodeCreateRefactor` flag is retired.
beforeEach(() => {
mockAppendFeatureFlags({
linodeCreateRefactor: makeFeatureFlagData(true),
});
mockGetFeatureFlagClientstream();
});
Comment on lines +21 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

Was looking for a PR to abstract/consolidate flag logic/imports but maybe it hasn't been done yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hasn't been done yet, but we do have a ticket! M3-7891


/*
* - Confirms UI flow when creating a Linode with an authorized SSH key.
* - Confirms that existing SSH keys are listed on page and can be selected.
* - Confirms that outgoing Linode create API request contains authorized user for chosen key.
*/
it('can add an existing SSH key during Linode create flow', () => {
const linodeRegion = chooseRegion();
const mockLinode = linodeFactory.build({
id: randomNumber(),
label: randomLabel(),
region: linodeRegion.id,
});

const mockSshKey = sshKeyFactory.build({
label: randomLabel(),
});

const mockUser = accountUserFactory.build({
username: randomLabel(),
ssh_keys: [mockSshKey.label],
});

mockGetUsers([mockUser]);
mockGetUser(mockUser);
mockCreateLinode(mockLinode).as('createLinode');

cy.visitWithLogin('/linodes/create');

linodeCreatePage.setLabel(mockLinode.label);
linodeCreatePage.selectImage('Debian 11');
linodeCreatePage.selectRegionById(linodeRegion.id);
linodeCreatePage.selectPlan('Shared CPU', 'Nanode 1 GB');
linodeCreatePage.setRootPassword(randomString(32));
Copy link
Contributor

Choose a reason for hiding this comment

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

This abstraction makes sense! Especially cause we often have to use the create flow to test adjacent/dependent features (just built one for PGs).

One thing I find the most difficult when working in e2e tests (that is also true in the src CM codebase but I know it better) is knowing the utils/abstractions available to me. I wonder how we could go around providing developers summaries of those. My first thought was to start building MD files in the feature root. Right now that could just be read, but eventually we could aggregate them into docs. ex:

// packages/manager/cypress/e2e/core/linodes/helpers.md

Linode Helpers

| Helper  | Description | Methods | Example |
| ------- | ------- | ------- | ------- |
| linodeCreatePage | a util to populate linode create flow data | setLabel, setImage, ...  | linodeCreatePage.setLabel('My Label') |

Obv it would be amazing to also find a way to self generate cause this would take quite a bit of work. Any idea on how to generate an API of sorts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discoverability is a huge issue! (Azure and I have even discussed this a bit as she's getting acquainted with the tests and Cloud Manager source)

Weeks back I did a little bit of googling and found vitepress-jsdoc, and thought it would be really cool if we could generate reference docs for our API-v4 client library and integrate them with the rest of our Vitepress docs, but doing the same for our utils could be really useful too. (Mind you I did absolutely no work to see if this is technically viable, it was just a spur of the moment idea I never followed up on)

Copy link
Contributor

Choose a reason for hiding this comment

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

ohhh that looks promising. I'll look into this one in a a new task ❗


// Confirm that SSH key is listed, then select it.
cy.findByText(mockSshKey.label)
.scrollIntoView()
.should('be.visible')
.closest('tr')
.within(() => {
cy.findByText(mockUser.username);
cy.findByLabelText(`Enable SSH for ${mockUser.username}`).click();
});

// Click "Create Linode" button and confirm outgoing request data.
ui.button
.findByTitle('Create Linode')
.should('be.visible')
.should('be.enabled')
.click();

// Confirm that outgoing Linode create request contains authorized user that
// corresponds to the selected SSH key.
cy.wait('@createLinode').then((xhr) => {
const requestPayload = xhr.request.body;
expect(requestPayload['authorized_users'][0]).to.equal(mockUser.username);
});
});

/*
* - Confirms UI flow when creating and selecting an SSH key during Linode create flow.
* - Confirms that new SSH key is automatically shown in Linode create page.
* - Confirms that outgoing Linode create API request contains authorized user for new key.
*/
it('can add a new SSH key during Linode create flow', () => {
const linodeRegion = chooseRegion();
const mockLinode = linodeFactory.build({
id: randomNumber(),
label: randomLabel(),
region: linodeRegion.id,
});

const mockSshKey = sshKeyFactory.build({
label: randomLabel(),
ssh_key: `ssh-rsa ${randomString(16)}`,
});

const mockUser = accountUserFactory.build({
username: randomLabel(),
ssh_keys: [],
});

const mockUserWithKey = {
...mockUser,
ssh_keys: [mockSshKey.label],
};

mockGetUser(mockUser);
mockGetUsers([mockUser]);
mockCreateLinode(mockLinode).as('createLinode');
mockCreateSSHKey(mockSshKey).as('createSSHKey');

cy.visitWithLogin('/linodes/create');

linodeCreatePage.setLabel(mockLinode.label);
linodeCreatePage.selectImage('Debian 11');
linodeCreatePage.selectRegionById(linodeRegion.id);
linodeCreatePage.selectPlan('Shared CPU', 'Nanode 1 GB');
linodeCreatePage.setRootPassword(randomString(32));

// Confirm that no SSH keys are listed for the mocked user.
cy.findByText(mockUser.username)
.scrollIntoView()
.should('be.visible')
.closest('tr')
.within(() => {
cy.findByText('None').should('be.visible');
cy.findByLabelText(`Enable SSH for ${mockUser.username}`).should(
'be.disabled'
);
});

// Click "Add an SSH Key" and enter a label and the public key, then submit.
ui.button
.findByTitle('Add an SSH Key')
.should('be.visible')
.should('be.enabled')
.click();

mockGetUsers([mockUserWithKey]).as('refetchUsers');
ui.drawer
.findByTitle('Add SSH Key')
.should('be.visible')
.within(() => {
cy.findByLabelText('Label').type(mockSshKey.label);
cy.findByLabelText('SSH Public Key').type(mockSshKey.ssh_key);
ui.button
.findByTitle('Add Key')
.should('be.visible')
.should('be.enabled')
.click();
});

cy.wait(['@createSSHKey', '@refetchUsers']);

// Confirm that the new SSH key is listed, and select it to be added to the Linode.
cy.findByText(mockSshKey.label)
.scrollIntoView()
.should('be.visible')
.closest('tr')
.within(() => {
cy.findByLabelText(`Enable SSH for ${mockUser.username}`).click();
});

// Click "Create Linode" button and confirm outgoing request data.
ui.button
.findByTitle('Create Linode')
.should('be.visible')
.should('be.enabled')
.click();

// Confirm that outgoing Linode create request contains authorized user that
// corresponds to the new SSH key.
cy.wait('@createLinode').then((xhr) => {
const requestPayload = xhr.request.body;
expect(requestPayload['authorized_users'][0]).to.equal(mockUser.username);
});
});
});
Loading
Loading