ODC-5577:Topology chart view automation #9399

sanketpathak · 2021-07-04T08:58:30Z

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

Problem: Automation of empty state of topology chart view, topology chart view with workloads and filters in topology chart view

Solution: This script will automate the topology chart view page.

Test Setup:

Cluster should be running on local

Check the TAGS in frontend/packages/topology/integration-tests/cypress.json file be: "@Topology and (@smoke or @regression) and not (@Manual or @to-do or @un-verified)",

Command to execute:

yarn run dev
yarn run test-cypress-topology
Execute the topology-chart-area-visual.feature file

Checks for approving Epic scenarios Automation PR:

Execute the @to-do tagged gherkin scripts manually
Convert the @to-do gherkin scripts to cypress automation scripts
Once scripts are automated, remove tag @to-do
Execute the scripts in Remote cluster

Browser
Chrome 89

Test Execution Screenshot:

gajanan-more · 2021-07-07T12:19:39Z

All the tests are running fine but I am getting this error and @sanketpathak is not. Adding /lgtm for running tests successfully.

gajanan-more · 2021-07-07T12:20:27Z

/lgtm

makambalaji · 2021-07-13T07:22:19Z

frontend/packages/dev-console/integration-tests/support/pages/add-flow/git-page.ts

      case 'deployment config':
      case 'Deployment Config':
+      case 'deployment-config':
+      case 'Deployment-Config':
+      case 'DeploymentConfig':


Can we try "||" operator, instead of adding multiple lines

It's not working

frontend/packages/topology/integration-tests/support/page-objects/topology-po.ts

makambalaji · 2021-07-13T07:25:36Z

frontend/packages/topology/integration-tests/support/page-objects/topology-po.ts

+export function typeOfResource(type: string) {
+  return `[data-test="${type}"]`;
+}
+


we can use cy.byTestID("attribute-name"). Please remove this function

you are trying to return data-test attribute using this function and passing it to cy.get() function right, if that is the case. we can directly pass it to cy.byTestID()

makambalaji · 2021-07-13T07:27:47Z

frontend/packages/topology/integration-tests/support/page-objects/topology-po.ts

+export function typeOfWorkload(workload: string) {
+  return `[data-test~="icon"][data-test~="${workload
+    .toLowerCase()
+    .replace(' ', '')
+    .trim()}"]`;
+}


Please use Arrow functions where ever applicable instead this functions

makambalaji · 2021-07-13T07:29:36Z

frontend/packages/topology/integration-tests/support/step-definitions/topology/chart-view.ts

+Then('user sees Topology page with message "No resources found"', () => {
+  cy.get(topologyPO.emptyText).should('be.visible');
+});


Suggested change

Then('user sees Topology page with message "No resources found"', () => {

cy.get(topologyPO.emptyText).should('be.visible');

});

Then('user sees Topology page with message {string}', (text:string) => {

cy.get(topologyPO.emptyText).should('be.visible');

});

or
Modify the step in feature file as Then user sees Topology page with message No resources found

makambalaji · 2021-07-13T09:46:44Z

frontend/packages/topology/integration-tests/support/step-definitions/topology/chart-view.ts

+  navigateTo(devNavigationMenu.Topology);
+});
+
+When('user clicks on "Add page" link in the topology page', () => {


same comment as mentioned above

sanketpathak · 2021-07-15T12:58:45Z

/assign @makambalaji

makambalaji · 2021-07-19T06:58:04Z

Tested on my localhost, worked fine

makambalaji · 2021-07-19T07:01:17Z

/approve
Please get approval from Developers

makambalaji · 2021-07-19T12:12:51Z

/lgtm

gajanan-more · 2021-07-19T12:13:18Z

/lgtm

gajanan-more · 2021-07-19T12:14:27Z

@jerolimov Can you please review this PR? QE Review is already done. If there is nothing, please add approve
Thank you.

jerolimov · 2021-07-21T07:00:53Z

/retest

jerolimov

Added some findings. Please take a look and let me know what you think.

jerolimov · 2021-07-21T07:49:11Z

frontend/packages/topology/src/components/page/TopologyEmptyState.tsx

+          or visit the{' '}
+          <Link to="/add" data-test="add-page">
+            Add page
+          </Link>{' '}
+          for more details.


The {' '} ensures that the output is the same, but changes the i18n string in the translation files. I recommend that we concat these space strings into one string, like this:

<Trans t={t} ns="topology"> <Button isInline variant="link" data-test="start-building-your-application" onClick={(e) => { e.stopPropagation(); setIsQuickSearchOpen(true); }} > Start building your application </Button> {' or visit the '} <Link to="/add" data-test="add-page"> Add page </Link> {' for more details.'} </Trans>

Your code (and my example as well) changed the translation file! Please run yarn i18n after changing this.

You can also change the <3> to <2> manually in the other translation files, so that they still match? Thanks.

Alternative:

<Trans t={t} ns="topology"> <Button isInline variant="link" data-test="start-building-your-application" onClick={(e) => { e.stopPropagation(); setIsQuickSearchOpen(true); }} > Start building your application </Button>{' '} {'or visit the '} <Link to="/add" data-test="add-page"> Add page </Link>{' '} for more details. </Trans>

Then you don't need to change the translation files.

Updated with the first change

jerolimov · 2021-07-21T07:55:16Z

frontend/packages/console-shared/src/components/quick-search/QuickSearchBar.tsx

@@ -36,6 +36,7 @@ const QuickSearchBar: React.FC<QuickSearchBarProps> = ({
        onChange={onSearch}
        autoFocus={autoFocus}
        value={searchTerm}
+        data-test="quick-search-input"


We don't want repeat the prefixes. In this case please just use input or drop it as it is not used yet.

Changed to input, It can be used for automation in future

jerolimov · 2021-07-21T08:00:57Z

frontend/packages/topology/integration-tests/support/step-definitions/topology/chart-view.ts

+  );
+});
+
+Given('user has created deployment config workload {string}', (componentName: string) => {


Suggested change

Given('user has created deployment config workload {string}', (componentName: string) => {

Given('user has created a deployment config workload {string}', (componentName: string) => {

jerolimov · 2021-07-21T08:01:43Z

frontend/packages/topology/integration-tests/support/step-definitions/topology/chart-view.ts

+  cy.get(topologyPO.graph.startBuildingNewApp)
+    .should('be.visible')
+    .click();


Little bit confusing that you don't use build: string here.

jerolimov · 2021-07-21T08:02:28Z

frontend/packages/topology/integration-tests/support/step-definitions/topology/chart-view.ts

+When('user navigates to Topology page', () => {
+  navigateTo(devNavigationMenu.Topology);
+});
+


This is already defined above.

Suggested change

When('user navigates to Topology page', () => {

navigateTo(devNavigationMenu.Topology);

});

Sorry I missed that, removed

jerolimov · 2021-07-21T08:05:36Z

frontend/packages/topology/integration-tests/support/step-definitions/topology/chart-view.ts

+When('user is at Topology page chart view', () => {
+  navigateTo(devNavigationMenu.Topology);
+});
+


Same as When('user navigates to Topology page', ...

And we call the both topology variants "graph" and "list". If this should ensure that this should show the graph (chart), I would expect some additional code to do that.

So can you reuse When('user navigates to Topology page', here as well or add an check for the topology switch button on the top right corner?

Updated with checking the switch button

jerolimov · 2021-07-21T08:10:20Z

frontend/packages/topology/integration-tests/support/step-definitions/topology/chart-view.ts

+    cy.byTestID('DeploymentConfig').click();
+    cy.get(typeOfWorkload(workload)).should('be.visible');
+    cy.byTestID('DeploymentConfig').click();


Is this back and forward clicks really required?

Suggested change

cy.byTestID('DeploymentConfig').click();

cy.get(typeOfWorkload(workload)).should('be.visible');

cy.byTestID('DeploymentConfig').click();

cy.get(typeOfWorkload(workload)).should('be.visible');

It's not required but I did to make it consistent with the previous then statement and if in future further test scenarios are added in the file, then we do not have to worry about the checks being disabled

Sorry don't get the edge case where it is needed to click items twice.

Also, this is an Then(... condition. I would suggest not to click an items here anymore. If a click before that is needed this should be part of a When statement, or?

I would suggest to remove all clicks from

Then( 'user can see only the {string} workload when Deployment option is selected',

and

Then( 'user can see only the {string} workload when Deployment-Config option is selected',

jerolimov · 2021-07-21T08:13:50Z

frontend/packages/topology/src/components/page/TopologyEmptyState.tsx

@@ -21,22 +21,27 @@ const TopologyEmptyState: React.FC<TopologyEmptyStateProps> = ({ setIsQuickSearc
  return (
    <EmptyState className="odc-topology__empty-state" variant={EmptyStateVariant.full}>
      <EmptyStateIcon variant="container" component={TopologyIcon} />
-      <Title headingLevel="h3" size="lg">
+      <Title headingLevel="h3" size="lg" data-test="no-resources-found">


If possible, please move this 2 lines up to the EmptyState as the complete component define this.

<EmptyState className="odc-topology__empty-state" variant={EmptyStateVariant.full} data-test="empty-state" > <EmptyStateIcon variant="container" component={TopologyIcon} /> <Title headingLevel="h3" size="lg">

jerolimov · 2021-07-21T08:16:06Z

frontend/packages/topology/src/filters/KindFilterDropdown.tsx

+          key={filter.id}
+          value={filter.id}
+          isChecked={filter.value}
+          data-test={filter.labelKey ? t(filter.labelKey) : filter.label}


I think we don't want and need to translate data-test attributes.

Suggested change

data-test={filter.labelKey ? t(filter.labelKey) : filter.label}

data-test={filter.label}

jerolimov · 2021-07-21T08:22:21Z

frontend/packages/topology/src/components/graph-view/components/nodes/BaseNode.tsx

@@ -154,6 +154,7 @@ const BaseNode: React.FC<BaseNodeProps> = ({
            paddingX={8}
            paddingY={4}
            kind={kind}
+            data-test={`name-${element.getLabel()}`}


I don't find any code which is looking for this name- data-test attribute?

I was trying out something so this attribute was required but it didn't work out. I've not removed it because it can be used for future automation

But then I would recommend another data-test. Please remove it from this PR when it is not used and then you can add it with a test scenario where it is used.

Sure Christoph, will update it

jerolimov

Looks good so far. But please address also this two comments:

makambalaji · 2021-07-22T13:48:12Z

frontend/packages/dev-console/integration-tests/support/pages/add-flow/git-page.ts

-    cy.get(gitPO.gitSection.validatedMessage).should('not.have.text', 'Validating...');
+    cy.get(gitPO.gitSection.validatedMessage)
+      .should('not.include.text', 'Validating...')
+      .and('not.include.text', 'Repository URL to build and deploy your code from source');


Can you please add this text to static-text folder

jerolimov · 2021-07-22T15:00:11Z

frontend/packages/topology/src/filters/TopologyFilterBar.tsx

@@ -92,7 +92,7 @@ const TopologyFilterBar: React.FC<TopologyFilterBarProps> = ({
          </ToolbarItem>
        </ToolbarGroup>
        <ToolbarGroup variant={ToolbarGroupVariant['filter-group']}>
-          <ToolbarItem>
+          <ToolbarItem data-test={'filter-by-resource'}>


Mini nit:

Suggested change

<ToolbarItem data-test={'filter-by-resource'}>

<ToolbarItem data-test="filter-by-resource">

jerolimov

@sanketpathak @makambalaji I run all topology tests with yarn run test-cypress-topology, feature which is touched in this PR is green, but all others are red. Sanket says that you Balaji will check these later / in another PR.

The frontend changes are fine for me! Approving them. Balaji, please add lgtm if the rest is fine for you.

/approve

makambalaji · 2021-07-22T15:49:48Z

Executed the test scenarios as per the pr description, working as expected

makambalaji · 2021-07-22T15:50:02Z

/lgtm

jerolimov · 2021-07-22T16:01:01Z

Readd Balajis lgtm as the latest changes are requested by myself and looks fine. 👍

/lgtm

openshift-ci · 2021-07-22T16:01:18Z

[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:

~~frontend/OWNERS~~ [jerolimov]

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

jerolimov · 2021-07-22T20:12:37Z

/retest

openshift-bot · 2021-07-22T21:07:22Z

/retest-required

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

openshift-bot · 2021-07-23T05:41:46Z

/retest-required

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

openshift-bot · 2021-07-23T05:53:46Z

/retest-required

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

openshift-ci bot requested review from debsmita1 and gajanan-more July 4, 2021 08:58

openshift-ci bot added component/dev-console Related to dev-console component/topology Related to topology labels Jul 4, 2021

sanketpathak force-pushed the Topology-chart-view-automation-odc-5577 branch 2 times, most recently from 4bf8175 to 4312fa3 Compare July 7, 2021 07:06

openshift-ci bot assigned gajanan-more Jul 7, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 7, 2021

makambalaji suggested changes Jul 13, 2021

View reviewed changes

sanketpathak force-pushed the Topology-chart-view-automation-odc-5577 branch from 4312fa3 to 79e9b76 Compare July 14, 2021 22:32

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2021

openshift-ci bot assigned makambalaji Jul 15, 2021

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 15, 2021

sanketpathak force-pushed the Topology-chart-view-automation-odc-5577 branch from 79e9b76 to 50dffdc Compare July 16, 2021 13:04

openshift-ci bot added component/shared Related to console-shared and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 16, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2021

jerolimov suggested changes Jul 21, 2021

View reviewed changes

sanketpathak force-pushed the Topology-chart-view-automation-odc-5577 branch from 50dffdc to 7f90ff3 Compare July 21, 2021 14:03

openshift-ci bot added kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated and removed lgtm Indicates that a PR is ready to be merged. labels Jul 21, 2021

jerolimov suggested changes Jul 21, 2021

View reviewed changes

sanketpathak force-pushed the Topology-chart-view-automation-odc-5577 branch from 7f90ff3 to 056c82b Compare July 22, 2021 08:59

makambalaji suggested changes Jul 22, 2021

View reviewed changes

sanketpathak force-pushed the Topology-chart-view-automation-odc-5577 branch from 056c82b to f73a751 Compare July 22, 2021 14:14

jerolimov reviewed Jul 22, 2021

View reviewed changes

jerolimov approved these changes Jul 22, 2021

View reviewed changes

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

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2021

ODC-5577:Topology chart view automation

d79b408

sanketpathak force-pushed the Topology-chart-view-automation-odc-5577 branch from f73a751 to d79b408 Compare July 22, 2021 15:53

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2021

openshift-ci bot assigned jerolimov Jul 22, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2021

openshift-merge-robot merged commit cdea2e7 into openshift:master Jul 23, 2021

spadgett added this to the v4.9 milestone Aug 30, 2021

	Given('user has created deployment config workload {string}', (componentName: string) => {
	Given('user has created a deployment config workload {string}', (componentName: string) => {

	When('user navigates to Topology page', () => {
	navigateTo(devNavigationMenu.Topology);
	});

	data-test={filter.labelKey ? t(filter.labelKey) : filter.label}
	data-test={filter.label}

	<ToolbarItem data-test={'filter-by-resource'}>
	<ToolbarItem data-test="filter-by-resource">

ODC-5577:Topology chart view automation #9399

ODC-5577:Topology chart view automation #9399

Conversation

sanketpathak commented Jul 4, 2021 • edited

gajanan-more commented Jul 7, 2021

gajanan-more commented Jul 7, 2021

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanketpathak commented Jul 15, 2021

makambalaji commented Jul 19, 2021

makambalaji commented Jul 19, 2021

makambalaji commented Jul 19, 2021

gajanan-more commented Jul 19, 2021

gajanan-more commented Jul 19, 2021

jerolimov commented Jul 21, 2021

jerolimov left a comment

Choose a reason for hiding this comment

jerolimov Jul 21, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanketpathak Jul 21, 2021 • edited

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

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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerolimov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerolimov Jul 22, 2021 • edited

Choose a reason for hiding this comment

jerolimov left a comment • edited

Choose a reason for hiding this comment

makambalaji commented Jul 22, 2021

makambalaji commented Jul 22, 2021

jerolimov commented Jul 22, 2021

openshift-ci bot commented Jul 22, 2021

jerolimov commented Jul 22, 2021

openshift-bot commented Jul 22, 2021

openshift-bot commented Jul 23, 2021

openshift-bot commented Jul 23, 2021

sanketpathak commented Jul 4, 2021 •

edited

jerolimov Jul 21, 2021 •

edited

sanketpathak Jul 21, 2021 •

edited

jerolimov Jul 22, 2021 •

edited

jerolimov left a comment •

edited