Bug 1803257: Test pipeline v2 #4147

sanketpathak · 2020-02-02T23:52:02Z

No description provided.

ldimaggi · 2020-02-04T01:06:48Z

npx eslint ./packages/dev-console/integration-tests/tests/*

/home/ldimaggi/local/redhat_git/go_path/src/github.com/openshift/console/frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts
   30:3   error  'pipelineRunName' is defined but never used                                                                                                                                                                                                   @typescript-eslint/no-unused-vars
  136:5   error  Unexpected var, use let or const instead                                                                                                                                                                                                      no-var
  136:18  error  Don't use 'String' as a type. Use string instead                                                                                                                                                                                              @typescript-eslint/ban-types
  145:12  error  Replace `pipelineTableBody.element(by.css('[data-test-id="sample-app-pipeline"]')).isPresent()).toBe(⏎······true,⏎····` with `⏎······pipelineTableBody.element(by.css('[data-test-id="sample-app-pipeline"]')).isPresent(),⏎····).toBe(true`  prettier/prettier
  151:24  error  Replace `until.visibilityOf(pipelineTableBody.element(by.css('[data-test-id="sample-app-pipeline"]')))` with `⏎······until.visibilityOf(pipelineTableBody.element(by.css('[data-test-id="sample-app-pipeline"]'))),⏎····`                     prettier/prettier
  153:67  error  Insert `⏎····`                                                                                                                                                                                                                                prettier/prettier
  155:24  error  Replace `until.visibilityOf(element(by.cssContainingText('h2',·'Pipeline·Run·Overview')))` with `⏎······until.visibilityOf(element(by.cssContainingText('h2',·'Pipeline·Run·Overview'))),⏎····`                                               prettier/prettier
  156:64  error  Insert `·`                                                                                                                                                                                                                                    prettier/prettier
  164:38  error  Unexpected string concatenation                                                                                                                                                                                                               prefer-template
  164:55  error  Replace `+runName+` with `·+·runName·+·`                                                                                                                                                                                                      prettier/prettier
  166:17  error  Delete `·`                                                                                                                                                                                                                                    prettier/prettier
  174:65  error  Replace `5000);` with `·5000);⏎····`                                                                                                                                                                                                          prettier/prettier
  177:83  error  Insert `·`                                                                                                                                                                                                                                    prettier/prettier
  185:1   error  Delete `··`                                                                                                                                                                                                                                   prettier/prettier
  188:67  error  Replace `.getText().then(function·` with `⏎······.getText()⏎······.then(function`                                                                                                                                                             prettier/prettier
  189:7   error  Insert `··`                                                                                                                                                                                                                                   prettier/prettier
  190:7   error  Insert `··`                                                                                                                                                                                                                                   prettier/prettier
  190:7   error  Unexpected var, use let or const instead                                                                                                                                                                                                      no-var
  191:1   error  Insert `··`                                                                                                                                                                                                                                   prettier/prettier
  192:7   error  Insert `··`                                                                                                                                                                                                                                   prettier/prettier
  193:5   error  Replace `})` with `··});`                                                                                                                                                                                                                     prettier/prettier
  194:5   error  Insert `;`                                                                                                                                                                                                                                    prettier/prettier

✖ 22 problems (22 errors, 0 warnings)
  21 errors and 0 warnings potentially fixable with the `--fix` option.

ldimaggi · 2020-02-04T01:07:44Z

npx eslint ./packages/dev-console/integration-tests/views/*

/home/ldimaggi/local/redhat_git/go_path/src/github.com/openshift/console/frontend/packages/dev-console/integration-tests/views/pipeline.view.ts
  19:61  error  Replace `by.css('[data-test-id="kebab-button"]')` with `⏎··by.css('[data-test-id="kebab-button"]'),⏎`                                                                        prettier/prettier
  21:38  error  Replace `by.cssContainingText('button.pf-c-dropdown__menu-item',·'Start')` with `⏎··by.cssContainingText('button.pf-c-dropdown__menu-item',·'Start'),⏎`                      prettier/prettier
  22:39  error  Replace `by.cssContainingText('button.pf-c-dropdown__menu-item',·'Delete·Pipeline')` with `⏎··by.cssContainingText('button.pf-c-dropdown__menu-item',·'Delete·Pipeline'),⏎`  prettier/prettier
  23:36  error  Replace `by.cssContainingText('li.co-m-horizontal-nav__menu-item',·'Pipeline·Runs')` with `⏎··by.cssContainingText('li.co-m-horizontal-nav__menu-item',·'Pipeline·Runs'),⏎`  prettier/prettier
  25:93  error  Replace `by.cssContainingText('dd',·'sample-app-pipeline')` with `⏎··by.cssContainingText('dd',·'sample-app-pipeline'),⏎`                                                    prettier/prettier
  26:39  error  Delete `·`                                                                                                                                                                   prettier/prettier
  27:44  error  Unexpected string concatenation                                                                                                                                              prefer-template
  27:61  error  Replace `+name+` with `·+·name·+·`                                                                                                                                           prettier/prettier
  29:2   error  Insert `;`                                                                                                                                                                   prettier/prettier
  33:40  error  Replace `by.cssContainingText('li.co-m-horizontal-nav__menu-item',·'Logs')` with `⏎··by.cssContainingText('li.co-m-horizontal-nav__menu-item',·'Logs'),⏎`                    prettier/prettier
  35:51  error  Delete `·`                                                                                                                                                                   prettier/prettier
  43:50  error  Delete `·`                                                                                                                                                                   prettier/prettier

/home/ldimaggi/local/redhat_git/go_path/src/github.com/openshift/console/frontend/packages/dev-console/integration-tests/views/sample-app-pipeline.yaml
  7:11  error  Parsing error: Unexpected token :

/home/ldimaggi/local/redhat_git/go_path/src/github.com/openshift/console/frontend/packages/dev-console/integration-tests/views/simple-pipeline-demo.yaml
  7:11  error  Parsing error: Unexpected token :

✖ 14 problems (14 errors, 0 warnings)
  12 errors and 0 warnings potentially fixable with the `--fix` option.

sanketpathak · 2020-02-05T23:18:13Z

/test analyze

ldimaggi · 2020-02-06T21:28:56Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

@@ -120,4 +133,70 @@ describe('Pipeline', async () => {
      true,
    );
  });
+  it('start pipeline scenario', async () => {


Please add a description of the test that this function performs. Also - please add comments to the code as it is not always possible to understand the action that the "user" is performing in the UI .

ldimaggi · 2020-02-06T21:29:56Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    );
+    await browser.wait(until.visibilityOf(pipelineTable));
+    expect(
+      pipelineTableBody.element(by.css('[data-test-id="sample-app-pipeline"]')).isPresent(),


The definition of the UI element being accessed should be in the test's corresponding view file and not here in the test code.

ldimaggi · 2020-02-06T21:30:22Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    await pipelineFilter.click();
+    await pipelineFilter.sendKeys('sample-app-pipeline');
+    await browser.wait(
+      until.visibilityOf(pipelineTableBody.element(by.css('[data-test-id="sample-app-pipeline"]'))),


The definition of the UI element being accessed should be in the test's corresponding view file and not here in the test code.

ldimaggi · 2020-02-06T21:30:37Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    browser.sleep(5000);
+    await pipelineStart.click();
+    await browser.wait(
+      until.visibilityOf(element(by.cssContainingText('h2', 'Pipeline Run Overview'))),


The definition of the UI element being accessed should be in the test's corresponding view file and not here in the test code.

ldimaggi · 2020-02-06T21:30:52Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    });
+    await pipelinecheckStatus();
+    await browser.wait(until.visibilityOf(pipelineTableBody));
+    await element(by.css('[data-test-id="sample-app-pipeline"]')).click();


The definition of the UI element being accessed should be in the test's corresponding view file and not here in the test code.

ldimaggi · 2020-02-06T21:31:04Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    await element(by.css('[data-test-id="sample-app-pipeline"]')).click();
+    await browser.wait(until.visibilityOf(pipelineRun));
+    await pipelineRun.click();
+    const pipelines = element(by.css(`[data-test-id="${runName}"]`));


The definition of the UI element being accessed should be in the test's corresponding view file and not here in the test code.

ldimaggi · 2020-02-06T21:31:48Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    await browser.wait(until.visibilityOf(pipelineActionList));
+    await pipelineActionList.click();
+    await pipelineStartLastRun.click();
+    await browser.wait(until.elementToBeClickable(pipelineRuns), 5000);


The timeout value of 5000 msec should be defined in a constant.

ldimaggi · 2020-02-06T21:32:22Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    });
+    await pipelineSelect(runName).click();
+    await pipelineRunLogs.click();
+    // browser.wait(function() {


We should not leave commented out code in the repo.

ldimaggi · 2020-02-06T21:32:31Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    //   return element(by.className('odc-pipeline-run-logs__tasklist')).isLoaded()
+    // });
+
+    await browser.wait(until.presenceOf(element(by.className('odc-pipeline-run-logs__tasklist'))));


The definition of the UI element being accessed should be in the test's corresponding view file and not here in the test code.

ldimaggi · 2020-02-06T21:32:42Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    // });
+
+    await browser.wait(until.presenceOf(element(by.className('odc-pipeline-run-logs__tasklist'))));
+    await element(by.className('odc-pipeline-run-logs__tasklist'))


The definition of the UI element being accessed should be in the test's corresponding view file and not here in the test code.

ldimaggi · 2020-02-06T21:33:27Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+      .getText()
+      .then(function(text) {
+        const sample = text.replace(/\s+/g, ' ');
+        // expect(text.replace(/\s+/g, ' ')).toContain('install-deps compile code-sanity e2e-tests');


More commented out code - we should remove this.

ldimaggi · 2020-02-06T22:29:22Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    await pipelineActionList.click();
+    await pipelineStartLastRun.click();
+    await browser.wait(until.elementToBeClickable(pipelineRuns), 5000);
+    await browser.sleep(4000);


I had success if I replaced the sleep statement with:

await browser.wait(until.elementToBeClickable(pipelineRuns), 5000); //// await browser.sleep(4000); await pipelineRuns.click(); await browser.wait(until.visibilityOf(pipelineTableBody));

It's not working on my machine

ldimaggi · 2020-02-06T22:30:33Z

frontend/packages/dev-console/integration-tests/tests/pipeline.scenario.ts

+    //   return element(by.className('odc-pipeline-run-logs__tasklist')).isLoaded()
+    // });
+
+    await browser.wait(until.presenceOf(element(by.className('odc-pipeline-run-logs__tasklist'))));


I had success if I replaced the classname with:

await browser.wait(until.presenceOf(element(by.className('odc-pipeline-topology-graph')))); await element(by.className('odc-pipeline-topology-graph'))

It's not working for me

andrewballantyne · 2020-02-06T22:31:05Z

frontend/packages/operator-lifecycle-manager/integration-tests/views/operator-hub.view.ts

@@ -1,6 +1,6 @@
 import { browser, $, ExpectedConditions as until, by, element } from 'protractor';

-export const operatorModal = $('.pf-c-modal-box');
+export const operatorModal = $('.co-catalog-page__overlay');


@sanketpathak FYI this change will prevent me from providing an official approval. You'll need to seek out Christian or another dev from the frontend OWNERS file... so they can get both dev-console & OLM.

Also, will this impact any integration tests in OLM?

This change is necessary as the above modal element is using pattern fly class which is quite unreliable and tests are failing because of it.

I wasn't directly inquiring if this was a needed change - I was inquiring if it will impact other tests. I was also mentioning because of this change, I am unable to give an approved label... so the best I can give is lgtm - the same as Len.

andrewballantyne

/assign

Spoke with Sanket, he'll update this PR with Len's feedback + the comments before I do my review.

Still getting used to reading protractor code, so these comments will hopefully help. Once that's done I'll try to make this review a high priority.

spadgett · 2020-02-20T15:16:56Z

/bugzilla refresh

openshift-ci-robot · 2020-02-20T15:16:59Z

@spadgett: This pull request references Bugzilla bug 1803257, which is invalid:

expected the bug to target the "4.5.0" release, but it targets "4.4.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sanketpathak · 2020-02-20T15:38:43Z

/bugzilla refresh

openshift-ci-robot · 2020-02-20T15:40:50Z

@sanketpathak: This pull request references Bugzilla bug 1803257, which is valid.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sanketpathak · 2020-02-20T15:46:26Z

/retest

christianvogt · 2020-02-20T14:59:04Z

frontend/public/components/utils/horizontal-nav.tsx

@@ -146,7 +146,7 @@ export const NavBar = withRouter<NavBarProps>(({ pages, basePath }) => {
          'co-m-horizontal-nav-item--active': matchUrl && matchUrl.isExact,
        });
        return (
-          <li className={klass} key={name}>
+          <li className={klass} key={name} data-test-id={`horizontal-nav-${name}`}>


Why isn't this on the actionable link?

christianvogt · 2020-02-20T15:00:13Z

frontend/packages/dev-console/integration-tests/views/pipeline.view.ts

+export const pipelineSelect = function(name) {
+  const newpipelineSelect = element(by.css(`[data-test-id="${name}"]`));
+  return newpipelineSelect;
+};


export const pipelineSelect = (name) => element(by.css(`[data-test-id="${name}"]`));

christianvogt · 2020-02-27T20:13:12Z

/lgtm
/approve

openshift-ci-robot · 2020-02-27T20:15:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, 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~~ [christianvogt]

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

openshift-bot · 2020-02-27T20:23:48Z

/retest

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

openshift-bot · 2020-02-27T22:33:23Z

/retest

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

openshift-ci-robot · 2020-02-28T00:11:32Z

@sanketpathak: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/verify	`948095b`	link	`/test verify`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-bot · 2020-02-28T00:18:11Z

/retest

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

openshift-ci-robot · 2020-02-28T02:20:40Z

@sanketpathak: All pull requests linked via external trackers have merged. Bugzilla bug 1803257 has been moved to the MODIFIED state.

In response to this:

Bug 1803257: Test pipeline v2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sanketpathak · 2020-02-28T13:34:09Z

/cherrypick release-4.4

openshift-cherrypick-robot · 2020-02-28T13:34:18Z

@sanketpathak: new pull request created: #4555

In response to this:

/cherrypick release-4.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. component/dev-console Related to dev-console labels Feb 2, 2020

openshift-ci-robot requested review from christianvogt and divyanshiGupta February 2, 2020 23:52

sanketpathak force-pushed the test-pipeline-v2 branch 2 times, most recently from 7f2c2f0 to d62796c Compare February 3, 2020 22:09

ldimaggi mentioned this pull request Feb 4, 2020

Bug 1797382: git test updation-1 #4027

Merged

sanketpathak force-pushed the test-pipeline-v2 branch 2 times, most recently from 582125d to 1ef54ce Compare February 5, 2020 22:59

openshift-ci-robot added the component/olm Related to OLM label Feb 5, 2020

sanketpathak mentioned this pull request Feb 5, 2020

[WIP]Defining path in e2e suite for dev console tests #4181

Closed

ldimaggi reviewed Feb 6, 2020

View reviewed changes

andrewballantyne reviewed Feb 6, 2020

View reviewed changes

andrewballantyne reviewed Feb 7, 2020

View reviewed changes

sanketpathak force-pushed the test-pipeline-v2 branch from d9d2e5a to 2fc94f0 Compare February 20, 2020 13:40

openshift-ci-robot removed the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Feb 20, 2020

openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Feb 20, 2020

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Feb 20, 2020

sanketpathak force-pushed the test-pipeline-v2 branch 2 times, most recently from 2c4fb37 to 948095b Compare February 20, 2020 23:48

christianvogt reviewed Feb 21, 2020

View reviewed changes

pipeline test iteration v2

a075823

sanketpathak force-pushed the test-pipeline-v2 branch from 948095b to a075823 Compare February 26, 2020 15:09

openshift-ci-robot assigned christianvogt Feb 27, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 27, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 27, 2020

openshift-merge-robot merged commit 9424e9b into openshift:master Feb 28, 2020

openshift-cherrypick-robot mentioned this pull request Feb 28, 2020

[release-4.4] Bug 1803257: Test pipeline v2 #4555

Closed

spadgett added this to the v4.5 milestone Feb 28, 2020

Bug 1803257: Test pipeline v2 #4147

Bug 1803257: Test pipeline v2 #4147

Conversation

sanketpathak commented Feb 2, 2020

ldimaggi commented Feb 4, 2020

ldimaggi commented Feb 4, 2020

sanketpathak commented Feb 5, 2020

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

andrewballantyne Feb 6, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewballantyne left a comment

Choose a reason for hiding this comment

spadgett commented Feb 20, 2020

openshift-ci-robot commented Feb 20, 2020

sanketpathak commented Feb 20, 2020

openshift-ci-robot commented Feb 20, 2020

sanketpathak commented Feb 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christianvogt commented Feb 27, 2020

openshift-ci-robot commented Feb 27, 2020

openshift-bot commented Feb 27, 2020

openshift-bot commented Feb 27, 2020

openshift-ci-robot commented Feb 28, 2020 • edited

openshift-bot commented Feb 28, 2020

openshift-ci-robot commented Feb 28, 2020

sanketpathak commented Feb 28, 2020

openshift-cherrypick-robot commented Feb 28, 2020

andrewballantyne Feb 6, 2020 •

edited

openshift-ci-robot commented Feb 28, 2020 •

edited