-
Notifications
You must be signed in to change notification settings - Fork 358
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
Changes from 27 commits
0a7d2cf
e8061d5
f975764
3eee4f7
2c93978
8ba1012
0d41524
ef1d6af
70d751e
784cfa5
4abe2f3
030a7ed
da5d293
9cc4abf
52d6e8f
5ed6fc2
54c3838
c13bea7
a9c3a04
ad2dfc0
4724234
053ebb0
93d36a7
e8b88f8
e26594d
167ee2c
a7160c0
5d66f93
8e7bea7
4a8773f
0716396
2a7d535
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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)) |
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)) |
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)) |
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)) |
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) => { | ||
['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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
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: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!
There was a problem hiding this comment.
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)
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:
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?)
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!