test: [DI-20585] - Add ACLP Cypress Test Coverage for Linode Dashboard Widgets #10891

agorthi-akamai · 2024-09-05T12:12:16Z

Description 📝

Added aclp e2e test cases

Changes 🔄

List any change relevant to the reviewer.

...
...

How to test 🧪

yarn cy:run -s cypress/e2e/core/cloudpulse/linode-widget-verification.spec.ts

As an Author I have considered 🤔

Check all that apply

venkymano-akamai · 2024-09-05T12:20:25Z

packages/manager/cypress/support/util/cloudpulse.ts

+ * @param {string} region - The name of the region to select.
+ */
+export const selectRegion = (region: string) => {
+  // ui.regionSelect.find().click();


These two steps are not working even for existing tests like create-linode and clone-linode

ui.regionSelect.find().click(); ui.autocompletePopper.findByTitle(region);

Since this is a common thing, Can we please check this?

venkymano-akamai · 2024-09-05T12:21:55Z

packages/manager/cypress/support/util/cloudpulse.ts

+ */
+export const selectAndVerifyServiceName = (service: string) => {
+  const resourceInput = ui.autocomplete.findByTitleCustom('Select Resources');
+  resourceInput.findByTitle('Open').click();


Aftert this (we opened the autocompletepopper now), the below step doesn't work

ui.autocompletePopper.findByTitle(service);

This resources is a multiselect component. Can you please point us if there are already places where we have test cases for autocomplete multi selections, so that we can follow the standards

For all other filters, this

ui.autocompletePopper.findByTitle(service);

works out of the box

@venkymano-akamai Happy to take a closer look once this test is passing, but can't offer much advice for now since I can't reproduce.

In the meantime, here's an example in our OBJ access keys test where we have a multi select and select multiple options:

https://github.com/linode/manager/blob/develop/packages/manager/cypress/e2e/core/objectStorage/access-keys.smoke.spec.ts#L225-L231

@venkymano-akamai still having trouble with this? Does my other comment shed any light / help you solve your problem?

venkymano-akamai · 2024-09-05T12:22:24Z

packages/manager/cypress/support/util/cloudpulse.ts

+    .findByTitleCustom('Select Dashboard')
+    .findByTitle('Open')
+    .click();
+  ui.autocompletePopper.findByTitle(serviceName).should('be.visible').click();


Here it works out of the box, for single selection component

jdamore-linode · 2024-09-05T13:00:09Z

@agorthi-akamai Taking this out of draft status so that the tests can run. I'll take a look soon! Thanks for the PR!

jdamore-linode · 2024-09-05T17:38:21Z

@agorthi-akamai Thanks for the PR! I'm seeing these fail to build in CI -- checking it out locally, it looks like there's a stray import in cypress/support/util/cloudpulse.ts that either needs to be installed if it's needed, or removed if it's not.

Aside from that, I'm unable to run any of the tests locally because they don't seem to be using any mocks. These tests run against our Production API so Cypress tests for upcoming features need to use mock API data.

agorthi-akamai · 2024-09-06T12:22:22Z

@agorthi-akamai Thanks for the PR! I'm seeing these fail to build in CI -- checking it out locally, it looks like there's a stray import in cypress/support/util/cloudpulse.ts that either needs to be installed if it's needed, or removed if it's not.

Aside from that, I'm unable to run any of the tests locally because they don't seem to be using any mocks. These tests run against our Production API so Cypress tests for upcoming features need to use mock API data.

I've addressed to your code review comments; kindly review

agorthi-akamai · 2024-10-01T15:59:55Z

Hi Joe!
I’ve addressed your code review feedback and added appropriate comments in the pull request. Additionally, I’ve removed the waits as per our preference since the PR has been merged. Could you please take a moment to review it?

jdamore-linode

Thanks Ananth! A few things I suggested before that still need to be addressed:

I think there may have been a misunderstanding: we should be preferring forEach over for .. of expressions, so we'll want to change all of these new for .. of expressions back to forEach. I apologize for any confusion.
These two tests still need some clean up:
- should allow users to select the desired aggregation and view the latest data from the API displayed in the graph
- should allow users to select their desired granularity and see the most recent data from the API reflected in the graph
Order of params in the mock utils in intercepts/cloudpulse.ts (I posted a new comment that goes into more detail about this, but feel free to reach out on Slack if you have any questions or if you're not sure what I'm asking for. It's a quick change.)

Beyond that, I noticed a couple small things that I failed to catch before (sorry!) and shared some suggestions to address them:

Stray cy.get() after cy.visitWithLogin() -- double check that this isn't required, but I'm pretty sure it's unnecessary. It doesn't hurt anything, though, so not a huge deal.
Couple naming suggestions for the functions in util/cloudpulse.ts to make them consistent with the rest of our mock and intercept utils

There's good news though:

The tests pass reliably: I ran them 25 times in a row, no failures. I'm very confident in their stability right now, and am very happy to see them running so smoothly (and quickly) without the need for cy.wait
Almost all of these remaining suggestions are about maintaining consistency with our codebase's conventions -- only the clean up for the 2 tests I mentioned above actually relate to test quality. In other words, we're pretty much there in terms of test quality.
Once these suggestions are addressed, as long as there are no regressions and the tests continue passing reliably, I'm ready to approve.

Thanks Ananth! We're just about there. Don't hesitate to reach out if you have any questions or could use a hand wrapping up these last few things.

packages/manager/cypress/e2e/core/cloudpulse/linode-widget-verification.spec.ts

packages/manager/cypress/support/util/cloudpulse.ts

packages/manager/cypress/e2e/core/cloudpulse/linode-widget-verification.spec.ts

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

jdamore-linode · 2024-10-02T19:09:37Z

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

@@ -16,7 +16,7 @@ import type {
 } from '@linode/api-v4';

 /**
- * Intercepts  GET requests for metric definitions.


Nice catch! 🦅👀

jdamore-linode

Nice work, thanks for the effort you put into this @agorthi-akamai.

I posted a couple small suggestions that I'd like to see applied, and think there might be a stray inclusion in queries/cloudpulse/metrics.ts that might not belong in this PR, but otherwise I'm quite happy with the state of this. Thanks again

packages/manager/cypress/e2e/core/cloudpulse/linode-widget-verification.spec.ts

packages/manager/src/components/LineGraph/LineGraph.tsx

packages/manager/src/queries/cloudpulse/metrics.ts

dwiley-akamai

Things look good to me, but there's a unit test failure for GlobalFilters.test.tsx -- should be able to resolve it by updating

manager/packages/manager/src/features/CloudPulse/Overview/GlobalFilters.test.tsx

Line 38 in 92a4643

screen.getByRole('combobox', { name: 'Select Time Duration' })

to have name: 'Select a Time Duration'

dwiley-akamai · 2024-10-04T18:03:17Z

packages/manager/src/factories/dashboards.ts

I echo both of the points above regarding the removal of the doc comments and using template string literals

agorthi-akamai · 2024-10-05T08:11:31Z

All comments are addressed.

dwiley-akamai

All checks pass now ✅

Thanks for all the changes and fixes!

cypress · 2024-10-07T18:24:09Z

Cloud Manager E2E Run #6631

Run Properties: Passed #6631 • f49d104f6c: test: [DI-20585] - Add ACLP Cypress Test Coverage for Linode Dashboard Widgets (...

Project	`Cloud Manager E2E`
Branch Review	`develop`
Run status	`Passed #6631`
Run duration	`26m 19s`
Commit	`f49d104f6c: test: [DI-20585] - Add ACLP Cypress Test Coverage for Linode Dashboard Widgets (...`
Committer	`agorthi-akamai`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`2`
Pending	`2`
Skipped	`0`
Passing	`425`
View all changes introduced in this branch ↗︎

…d Widgets (linode#10891) * upcoming:[DI-20585]- added aclp e2e test cases * upcoming:[DI-20585]- added aclp e2e test cases * upcoming:[DI-20585]- added missing message constants file * upcoming:[DI-20585]- added cloudpulse tsx file under ui/util files * upcoming:[DI-20585]- added findByPlaceholderCustom for selectServiceName * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming: [DI-20585] - Fix validation for feature flag test case * upcoming: [DI-20585] - Enable services call through mock * upcoming: [DI-20585] - Flow update for widget tests * upcoming:[DI-20585]- Added code review comments and fixing few mocking issues * upcoming:[DI-20585]- Added code review comments and fixing few mocking issues * upcoming:[DI-20585]- Added code review comments and fixing few mocking issues * upcoming:[DI-20585]- Added code factories to mock the data * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- renaming intercept methods to mocks * upcoming:[DI-20585]- renaming intercept methods to mocks * upcoming:[DI-20585]- renaming intercept methods to mocks * upcoming:[DI-20585]- renaming intercept methods to mocks * upcoming:[DI-20585]- fixing zoom-in/out canvas issue * upcoming:[DI-20585]- fixing code review coments * upcoming:[DI-20585]- fixing code review coments * upcoming:[DI-20585]- Added code review comments and adding placeholder values in CloudPulseCustomSelect.tsx * DI-20360: Added change set * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- fixing conflict * upcoming:[DI-20585]- fixing place holder values * tests: [DI-20585] - Use Icon button with aria label for svg icons and remove ui.cloudpulse * upcoming:[DI-20585]- Added code review comments * tests: [DI-20585] - Factories implementation instead of functions * upcoming: [DI-20800] - cypress changes * tests: [DI-20585] - Enabling auto highlight for selections * tests: [DI-20585] - Revert not needed changes * tests: [DI-20585] - Selection update for custom select component * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review commennts * tests: [DI-20585] - PR comments for aria label * upcoming:[DI-20585]- Added code review commennts * upcoming:[DI-20585]- Added code review commennts * tests: [DI-20585] - Code clean ups and refactoring * upcoming:[DI-20585]- Added code review commennts * tests: [DI-20585] - CamelCase for variables * tests: [DI-20585] - Title validations * upcoming:[DI-20585]- Added code review commennts * upcoming:[DI-20585]- Added code review commennts * tests: [DI-20585] - Code clean up and refactoring work * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * tests: [DI-20585] - Removed widget header title * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * yarn file reverting * Syncing file processesLanding.tsx with develop * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments * upcoming:[DI-20585]- Added code review comments --------- Co-authored-by: vmangalr <vmangalr@akamai.com> Co-authored-by: venkatmano-akamai <chk-Venkatesh@outlook.com>

upcoming:[DI-20585]- added aclp e2e test cases

a4d7132

agorthi-akamai requested a review from a team as a code owner September 5, 2024 12:12

agorthi-akamai requested review from cliu-akamai and removed request for a team September 5, 2024 12:12

agorthi-akamai marked this pull request as draft September 5, 2024 12:14

venkymano-akamai reviewed Sep 5, 2024

View reviewed changes

agorthi-akamai added 2 commits September 5, 2024 17:58

upcoming:[DI-20585]- added aclp e2e test cases

2b87944

upcoming:[DI-20585]- added missing message constants file

38aa268

jdamore-linode marked this pull request as ready for review September 5, 2024 12:59

jdamore-linode requested a review from a team as a code owner September 5, 2024 12:59

jdamore-linode requested review from dwiley-akamai and abailly-akamai and removed request for a team September 5, 2024 12:59

upcoming:[DI-20585]- added cloudpulse tsx file under ui/util files

1bc1a73

jdamore-linode self-requested a review September 5, 2024 13:13

jdamore-linode assigned agorthi-akamai Sep 5, 2024

upcoming:[DI-20585]- added findByPlaceholderCustom for selectServiceName

12c8743

agorthi-akamai removed their assignment Sep 5, 2024

jaalah-akamai changed the title ~~upcoming:[DI-20585]- added aclp e2e test cases~~ test: [DI-20585] - Added ACLP E2E Test Coverage Sep 5, 2024

jaalah-akamai assigned agorthi-akamai Sep 5, 2024

jaalah-akamai added Ready for Review CloudPulse e2e Indicates that a PR touches Cypress tests in some way labels Sep 5, 2024

abailly-akamai marked this pull request as draft September 6, 2024 01:23

upcoming:[DI-20585]- Added code review comments

a4c2fab

agorthi-akamai added 4 commits October 1, 2024 13:40

yarn file reverting

70dba3c

Syncing file processesLanding.tsx with develop

bce5d79

upcoming:[DI-20585]- Added code review comments

892375f

upcoming:[DI-20585]- Added code review comments

87bb3e0

agorthi-akamai requested a review from jdamore-linode October 1, 2024 15:30

upcoming:[DI-20585]- Added code review comments

fe8c04a

jdamore-linode reviewed Oct 2, 2024

View reviewed changes

agorthi-akamai added 3 commits October 2, 2024 23:55

upcoming:[DI-20585]- Added code review comments

a75f392

upcoming:[DI-20585]- Added code review comments

1f5fee9

upcoming:[DI-20585]- Added code review comments

489b9e6

jdamore-linode reviewed Oct 2, 2024

View reviewed changes

agorthi-akamai added 3 commits October 3, 2024 00:45

upcoming:[DI-20585]- Added code review comments

ffea26d

upcoming:[DI-20585]- Added code review comments

1f7a11c

upcoming:[DI-20585]- Added code review comments

4d2051e

jdamore-linode approved these changes Oct 3, 2024

View reviewed changes

jdamore-linode added Add'tl Approval Needed Waiting on another approval! and removed Ready for Review labels Oct 3, 2024

upcoming:[DI-20585]- Added code review comments

92a4643

dwiley-akamai reviewed Oct 4, 2024

View reviewed changes

agorthi-akamai added 2 commits October 5, 2024 13:13

upcoming:[DI-20585]- Added code review comments

13ffe73

upcoming:[DI-20585]- Added code review comments

5e67e04

agorthi-akamai requested a review from dwiley-akamai October 5, 2024 08:11

dwiley-akamai approved these changes Oct 7, 2024

View reviewed changes

jaalah-akamai merged commit f49d104 into linode:develop Oct 7, 2024
20 checks passed

This was referenced Oct 10, 2024

Release v1.130.0 - release → staging #11087

Merged

Release v1.130.0- staging → master #11095

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: [DI-20585] - Add ACLP Cypress Test Coverage for Linode Dashboard Widgets #10891

test: [DI-20585] - Add ACLP Cypress Test Coverage for Linode Dashboard Widgets #10891

agorthi-akamai commented Sep 5, 2024 •

edited

Loading

venkymano-akamai Sep 5, 2024

venkymano-akamai Sep 5, 2024 •

edited

Loading

jdamore-linode Sep 6, 2024

jdamore-linode Sep 25, 2024

venkymano-akamai Sep 5, 2024

jdamore-linode commented Sep 5, 2024

jdamore-linode commented Sep 5, 2024

agorthi-akamai commented Sep 6, 2024

agorthi-akamai commented Oct 1, 2024

jdamore-linode left a comment

jdamore-linode Oct 2, 2024

jdamore-linode left a comment

dwiley-akamai left a comment

dwiley-akamai Oct 4, 2024

agorthi-akamai commented Oct 5, 2024

dwiley-akamai left a comment

cypress bot commented Oct 7, 2024

test: [DI-20585] - Add ACLP Cypress Test Coverage for Linode Dashboard Widgets #10891

test: [DI-20585] - Add ACLP Cypress Test Coverage for Linode Dashboard Widgets #10891

Conversation

agorthi-akamai commented Sep 5, 2024 • edited Loading

Description 📝

Changes 🔄

How to test 🧪

As an Author I have considered 🤔

venkymano-akamai Sep 5, 2024

Choose a reason for hiding this comment

venkymano-akamai Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

jdamore-linode Sep 6, 2024

Choose a reason for hiding this comment

jdamore-linode Sep 25, 2024

Choose a reason for hiding this comment

venkymano-akamai Sep 5, 2024

Choose a reason for hiding this comment

jdamore-linode commented Sep 5, 2024

jdamore-linode commented Sep 5, 2024

agorthi-akamai commented Sep 6, 2024

agorthi-akamai commented Oct 1, 2024

jdamore-linode left a comment

Choose a reason for hiding this comment

jdamore-linode Oct 2, 2024

Choose a reason for hiding this comment

jdamore-linode left a comment

Choose a reason for hiding this comment

dwiley-akamai left a comment

Choose a reason for hiding this comment

dwiley-akamai Oct 4, 2024

Choose a reason for hiding this comment

agorthi-akamai commented Oct 5, 2024

dwiley-akamai left a comment

Choose a reason for hiding this comment

cypress bot commented Oct 7, 2024

Cloud Manager E2E Run #6631

agorthi-akamai commented Sep 5, 2024 •

edited

Loading

venkymano-akamai Sep 5, 2024 •

edited

Loading