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

Automation: Filter quick starts catalog #9347

Conversation

sanketpathak
Copy link
Contributor

@sanketpathak sanketpathak commented Jun 26, 2021

Fix: https://issues.redhat.com/browse/ODC-6006

Problem: Automation to filter quick starts in the quick starts catalog page.

Solution: This script will filter quick starts in the quick starts catalog page.

Test Setup:

  • Cluster should be running on local

  • Change TAGS in frontend/packages/dev-console/integration-tests/cypress.json file to : "@smoke or @regression and not (@Manual or @to-do or @un-verified)",

  • Command to execute:

    • yarn run dev
    • yarn run test-cypress-devconsole
  • Execute the filter-quick-starts-catalog.feature file

Browser
Chrome 89

Test Execution Screenshot:
Screenshot from 2021-06-26 21-31-56

@openshift-ci openshift-ci bot added component/core Related to console core functionality component/dev-console Related to dev-console labels Jun 26, 2021
@sanketpathak sanketpathak force-pushed the automation-QuickStarts-filter-quick-starts-catalog branch from bd44662 to 025d241 Compare June 28, 2021 12:51
Copy link
Contributor

@gajanan-more gajanan-more left a comment

Choose a reason for hiding this comment

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

Screenshot from 2021-06-28 17-28-23

Working as expected

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 28, 2021
@sanketpathak sanketpathak force-pushed the automation-QuickStarts-filter-quick-starts-catalog branch from 025d241 to 8fa3ed1 Compare June 29, 2021 09:31
@openshift-ci openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm Indicates that a PR is ready to be merged. labels Jun 29, 2021
@sanketpathak sanketpathak force-pushed the automation-QuickStarts-filter-quick-starts-catalog branch from 8fa3ed1 to a92f7ec Compare June 29, 2021 09:47
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2021
@makambalaji
Copy link
Contributor

/assign @makambalaji

Copy link
Contributor

@makambalaji makambalaji left a comment

Choose a reason for hiding this comment

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

looks good to me from QE code point of view. please update https://docs.jboss.org/display/ODC/Automation+Status+Report confluence page
Please get approval from engineering team

@jerolimov
Copy link
Member

/assign

@sanketpathak sanketpathak force-pushed the automation-QuickStarts-filter-quick-starts-catalog branch from a92f7ec to 71f11e3 Compare June 29, 2021 14:56
Copy link
Member

@jerolimov jerolimov left a comment

Choose a reason for hiding this comment

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

Hey @sanketpathak, I take a deep look and found some small things. I tried to explain the suggestion, but please ping me on slack if you have any additional questions.

<Text
component="h1"
className="ocs-page-layout__title"
data-test="QuickStartCatalogPageTitle"
Copy link
Member

Choose a reason for hiding this comment

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

We want avoid global data-test attributes and use unique attributes within a scope. So you can use data-test="page-title" here.

Suggested change
data-test="QuickStartCatalogPageTitle"
data-test="page-title"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

<DrawerPanelBody
hasNoPadding
className="co-quick-start-panel-content__body"
data-test="quickstart-sidebar-content"
Copy link
Member

Choose a reason for hiding this comment

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

Could you select the content also if you define just data-test="content" here,

when you add also data-test="quickstart drawer" (with space) to DrawerPanelContent above?

Your page object could use then quickStartSidebarBody: '[data-test="quickstart drawer"] [data-test="content"]' (with space between both selectors)

The idea of using a space and short non-global attributes is that we can also check them in a more generic way:

  • [data-test="quickstart drawer"] [data-test="content"] (same as above)
  • [data-test~="quickstart"][data-test~="drawer"] [data-test~="content"]' (with a more generic search) or just
  • [data-test~="drawer"] [data-test~="content"]' (more generic)

data-test~= allows you to search one of the space separated data-test attributes while you must be exact when you use data-test=

Suggested change
data-test="quickstart-sidebar-content"
data-test="content"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, updated the code

@@ -72,7 +72,7 @@ const QuickStartCatalog: React.FC<QuickStartCatalogProps> = ({ quickStarts }) =>
)}
</EmptyStateBody>
<EmptyStatePrimary>
<Button variant="link" onClick={clearFilters}>
<Button variant="link" onClick={clearFilters} data-test="clear-filter-button">
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<Button variant="link" onClick={clearFilters} data-test="clear-filter-button">
<Button variant="link" onClick={clearFilters} data-test="clear-filters button">

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@@ -29,14 +29,17 @@ const QuickStartTileHeader: React.FC<QuickStartTileHeaderProps> = ({ status, dur

return (
<div className="co-quick-start-tile-header">
<Title headingLevel="h3">{name}</Title>
<Title headingLevel="h3" data-test={`tile-${name}`}>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<Title headingLevel="h3" data-test={`tile-${name}`}>
<Title headingLevel="h3" data-test="title">

Similar to above, this should be just title.

To differentiate the different tiles you should add a data-test={tile ${name}} to QuickStartTile.tsx.

I would prefer here if we use the QuickStart resource name here instead of the displayName?!

In QuickStartTile.tsx I would suggest:

   const {
+    metadata: { name },
     spec: { icon, displayName, description, durationMinutes, prerequisites },
   } = quickStart;

  ...

   return (
     <CatalogTile
       icon={quickStartIcon}
       className="co-quick-start-tile"
       featured={isActive}
       title={<QuickStartTileHeader name={displayName} status={status} duration={durationMinutes} />}
       onClick={onClick}
+      data-test={`tile ${name}`}
     >
       <QuickStartTileDescription description={description} prerequisites={prerequisites} />
     </CatalogTile>

You need update your page object to

  installOpenshiftPipelineOperatorCard: '[data-test~="tile"][data-test~="explore-pipelines"]',
  getStartedWithASampleApplicationCard: '[data-test~="tile"][data-test~="sample-application"]',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, updated the code

<div className="co-quick-start-tile-header__status">
{status !== QuickStartStatus.NOT_STARTED && (
<Label
className="co-quick-start-tile-header--margin"
variant="outline"
color={statusColorMap[status]}
icon={<StatusIcon status={status} />}
data-test="quick-start-card-status"
Copy link
Member

Choose a reason for hiding this comment

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

Based on the context we know that we are in a quick start card (or tile)

Suggested change
data-test="quick-start-card-status"
data-test="status"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@@ -0,0 +1,20 @@
export const quickStartsPO = {
quickStartTitle: '[data-test="QuickStartCatalogPageTitle"]',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
quickStartTitle: '[data-test="QuickStartCatalogPageTitle"]',
quickStartTitle: '[data-test="page-title"]',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment on lines 16 to 19
quickStartSidebarBody: '[data-test="quickstart-sidebar-content"]',
startButton: '[data-test="Start-quick-starts"]',
nextButton: '[data-test="Next-quick-starts"]',
closeButton: '[data-test="Close-quick-starts"]',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
quickStartSidebarBody: '[data-test="quickstart-sidebar-content"]',
startButton: '[data-test="Start-quick-starts"]',
nextButton: '[data-test="Next-quick-starts"]',
closeButton: '[data-test="Close-quick-starts"]',
quickStartSidebarBody: '[data-test~="drawer"] [data-test~="content"]',
startButton: '[data-test~="drawer"] [data-test="Start button"]',
nextButton: '[data-test~="drawer"] [data-test="Next button"]',
closeButton: '[data-test~="drawer"] [data-test="Close button"]',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of doing [data-test~="drawer"] [data-test="Start button"] for the button I've written it as [data-test="Start button"] in this file and in the code I used the quickStartSidebarBody page object and find the button element(startButton) in it as it is doing similar work and not hindering my test result

Comment on lines 11 to 12
clearFilter: '[data-test="clear-filter-button"]',
cardStatus: '[data-test="quick-start-card-status"]',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
clearFilter: '[data-test="clear-filter-button"]',
cardStatus: '[data-test="quick-start-card-status"]',
clearFilter: '[data-test="clear-filters button"]',
cardStatus: '[data-test~="tile"] [data-test~="status"]',

But notice, the card status will return potential all status fields for the complete page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cardStatus will be used with a particular quick start card to show it's status

Comment on lines 81 to 82
When('user enters asdf', () => {
quickStartsPage.filterByKeyword('asdf');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
When('user enters asdf', () => {
quickStartsPage.filterByKeyword('asdf');
When('user enters {string}', (filterKeyword: string) => {
quickStartsPage.filterByKeyword(filterKeyword);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment on lines 55 to 57
Given('user has completed "Getting started with a sample application" Quick Start', () => {
quickStartsPage.executeQuickStart(quickStartsPO.getStartedWithASampleApplicationCard);
});
Copy link
Member

Choose a reason for hiding this comment

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

If possible, a dynamic parameter would be cool here as well, or?

Suggested change
Given('user has completed "Getting started with a sample application" Quick Start', () => {
quickStartsPage.executeQuickStart(quickStartsPO.getStartedWithASampleApplicationCard);
});
Given('user has completed {string} Quick Start', (quickStartName: string) => {
quickStartsPage.executeQuickStart(quickStartsPO.getStartedWithASampleApplicationCard);
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@sanketpathak sanketpathak force-pushed the automation-QuickStarts-filter-quick-starts-catalog branch 3 times, most recently from 4964528 to 5baa96b Compare July 1, 2021 12:46
Copy link
Member

@jerolimov jerolimov left a comment

Choose a reason for hiding this comment

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

Sorry, one last request and then we are fine here. :)

Comment on lines 33 to 36
export function quickStartCard(quickStartDisplayName: string) {
const tile = quickStartDisplayNameToName(quickStartDisplayName);
const quickStartName = `[data-test~="tile"][data-test~="${tile}"]`;
return quickStartName;
}
Copy link
Member

Choose a reason for hiding this comment

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

Let us use quickStartName instead of tile. The next line is a selector, you can also directly return it or use selector here.

Suggested change
export function quickStartCard(quickStartDisplayName: string) {
const tile = quickStartDisplayNameToName(quickStartDisplayName);
const quickStartName = `[data-test~="tile"][data-test~="${tile}"]`;
return quickStartName;
}
export function quickStartCard(quickStartDisplayName: string) {
const quickStartName = quickStartDisplayNameToName(quickStartDisplayName);
return `[data-test~="tile"][data-test~="${tile}"]`;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment on lines 33 to 35
Then('user can see {string} Quick Start', (quickStartName: string) => {
quickStartsPage.cardPresent(quickStartCard(quickStartName));
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Then('user can see {string} Quick Start', (quickStartName: string) => {
quickStartsPage.cardPresent(quickStartCard(quickStartName));
});
Then('user can see {string} Quick Start', (quickStartDisplayName: string) => {
quickStartsPage.cardPresent(quickStartCard(quickStartDisplayName));
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment on lines 55 to 57
Given('user has completed {string} Quick Start', (quickStartName: string) => {
quickStartsPage.executeQuickStart(quickStartCard(quickStartName));
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Given('user has completed {string} Quick Start', (quickStartName: string) => {
quickStartsPage.executeQuickStart(quickStartCard(quickStartName));
});
Given('user has completed {string} Quick Start', (quickStartDisplayName: string) => {
quickStartsPage.executeQuickStart(quickStartCard(quickStartDisplayName));
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment on lines 69 to 71
Then('user can see {string} Quick Start is present', (quickStartName: string) => {
cy.get(quickStartCard(quickStartName)).should('be.visible');
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Then('user can see {string} Quick Start is present', (quickStartName: string) => {
cy.get(quickStartCard(quickStartName)).should('be.visible');
});
Then('user can see {string} Quick Start is present', (quickStartDisplayName: string) => {
cy.get(quickStartCard(quickStartDisplayName)).should('be.visible');
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@sanketpathak sanketpathak force-pushed the automation-QuickStarts-filter-quick-starts-catalog branch from 5baa96b to 7f63d3b Compare July 1, 2021 14:28
Copy link
Member

@jerolimov jerolimov left a comment

Choose a reason for hiding this comment

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

Thanks for applying all these data-test changes I requested. Looks good now 👍

Run filter-quick-starts-catalog.feature locally and all tests pass:

image

Because @gajanan-more added lgtm already before that changes, I re-add lgtm as well.

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 1, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 1, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gajanan-more, jerolimov, makambalaji, sanketpathak

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 1, 2021
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 1af590b into openshift:master Jul 2, 2021
@spadgett spadgett added this to the v4.9 milestone Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/dev-console Related to dev-console lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants