Bug 1878591: Added tests for lso ocs disk inventory #6606

afreen23 · 2020-09-14T04:50:08Z

Signed-off-by: Afreen Rahman afrahman@redhat.com

openshift-ci-robot · 2020-09-14T05:05:20Z

@afreen23: This pull request references Bugzilla bug 1878591, which is invalid:

expected the bug to target the "4.6.0" release, but it targets "---" 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:

Bug 1878591: Added tests for lso ocs disk inventory

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.

afreen23 · 2020-09-14T05:05:46Z

/bugzilla refresh

openshift-ci-robot · 2020-09-14T05:05:48Z

@afreen23: This pull request references Bugzilla bug 1878591, which is invalid:

expected the bug to target the "4.6.0" release, but it targets "---" 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.

afreen23 · 2020-09-14T05:06:13Z

/bugzilla refresh

openshift-ci-robot · 2020-09-14T05:06:19Z

@afreen23: This pull request references Bugzilla bug 1878591, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

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.

gnehapk · 2020-09-14T05:25:50Z

...d/packages/local-storage-operator-plugin/integration-tests/tests/disks-list-page.scenario.ts

+    await browser.get(`${appHost}/`);
+    await clickNavLink(['Compute', 'Nodes']);
+    await isLoaded();
+    $(`a[data-test-id="${nodeName}"]`).click();


All the selectors should be moved to a corresponding view.ts file, similar to other tests.

This takes in a dynamic value therefore I avoided creating anon functions and it wont be reused anywhere else.

...d/packages/local-storage-operator-plugin/integration-tests/tests/disks-list-page.scenario.ts

frontend/packages/local-storage-operator-plugin/src/components/disks-list/disks-list-page.tsx

...d/packages/local-storage-operator-plugin/integration-tests/tests/disks-list-page.scenario.ts

bipuladh · 2020-09-14T05:45:52Z

...d/packages/local-storage-operator-plugin/integration-tests/tests/disks-list-page.scenario.ts

+    expect(firstRow.get(2).getAttribute('innerText')).toBe(firstDisk.type || '-');
+    expect(firstRow.get(3).getAttribute('innerText')).toBe(firstDisk.model || '-');


getText also returns the visible inner text of the element. I am not sure why we would want to do it this way.

getAttribute returns text for hidden elements and on certain screens few of the columns will be hidden

since it will be running headless if getText works on headless mode then let us keep it as getText

getText , does not works therefore we are using getAttribute here.

https://github.com/openshift/console/pull/6606/files#diff-1e194e3ba92229ea049eea86f7bf2e94R47
If all the headers(columns) are visible I am confused why exactly some of the elements are missing here.

for headers the presence of DOM node is checked by the attribute data-label=.
We are not using protractor get() and getText() functions which work on element type

Edit: And these functions work for visible elements and return empty string for hidden elements

I have posted the screenshot of running tests in description as well.
#6606 (comment)

...tend/packages/local-storage-operator-plugin/integration-tests/views/disks-list-page.views.ts

gnehapk · 2020-09-14T07:32:36Z

...d/packages/local-storage-operator-plugin/integration-tests/tests/disks-list-page.scenario.ts

+    await browser.get(`${appHost}/`);
+    await clickNavLink(['Compute', 'Nodes']);
+    await isLoaded();
+    click($(`a[data-test-id="${nodeName}"]`));


You can create a function and pass this dynamic value in view.ts file.
It will be useful, if the selectors get changed in future version.

#6606 (comment)

gnehapk · 2020-09-14T07:35:23Z

...d/packages/local-storage-operator-plugin/integration-tests/tests/disks-list-page.scenario.ts

+    await textbox.sendKeys(discoveredDevices[0].path);
+    let updatedRows = resourceRows.count();
+    expect(updatedRows).toBe(1);
+    // info: clear() is not triggering event to clear the filter and update rows


I don't see clear method being called here. So I guess this comment can be removed.

See the lines 71-72, its for that and its valid here.

...tend/packages/local-storage-operator-plugin/integration-tests/views/disks-list-page.views.ts

bipuladh · 2020-09-14T07:46:57Z

...d/packages/local-storage-operator-plugin/integration-tests/tests/disks-list-page.scenario.ts

+    expect(firstRow.get(2).getAttribute('innerText')).toBe(firstDisk.type || '-');
+    expect(firstRow.get(3).getAttribute('innerText')).toBe(firstDisk.model || '-');


https://github.com/openshift/console/pull/6606/files#diff-1e194e3ba92229ea049eea86f7bf2e94R47
If all the headers(columns) are visible I am confused why exactly some of the elements are missing here.

...d/packages/local-storage-operator-plugin/integration-tests/tests/disks-list-page.scenario.ts

Signed-off-by: Afreen Rahman <afrahman@redhat.com>

bipuladh

/lgtm

openshift-ci-robot · 2020-09-18T08:10:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afreen23, bipuladh

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/packages/local-storage-operator-plugin/OWNERS~~ [afreen23,bipuladh]

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

openshift-ci-robot · 2020-09-18T10:48:56Z

@afreen23: All pull requests linked via external trackers have merged:

openshift/console#6606

Bugzilla bug 1878591 has been moved to the MODIFIED state.

In response to this:

Bug 1878591: Added tests for lso ocs disk inventory

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 requested review from a2batic and cloudbehl September 14, 2020 04:50

openshift-ci-robot added component/lso Related to local-storage-operator-plugin approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 14, 2020

afreen23 changed the title ~~test: Added tests for lso ocs disk inventory~~ Bug 1878591: Added tests for lso ocs disk inventory Sep 14, 2020

openshift-ci-robot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Sep 14, 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 Sep 14, 2020

gnehapk reviewed Sep 14, 2020

View reviewed changes

bipuladh suggested changes Sep 14, 2020

View reviewed changes

afreen23 force-pushed the disk-inventory-tests branch from 7106599 to aded576 Compare September 14, 2020 07:19

gnehapk reviewed Sep 14, 2020

View reviewed changes

bipuladh suggested changes Sep 14, 2020

View reviewed changes

test: Added tests for lso ocs disk inventory

0b89e9a

Signed-off-by: Afreen Rahman <afrahman@redhat.com>

afreen23 force-pushed the disk-inventory-tests branch from aded576 to 0b89e9a Compare September 18, 2020 06:59

bipuladh approved these changes Sep 18, 2020

View reviewed changes

openshift-ci-robot assigned bipuladh Sep 18, 2020

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

openshift-merge-robot merged commit 460c2c3 into openshift:master Sep 18, 2020

spadgett added this to the v4.6 milestone Sep 30, 2020

Bug 1878591: Added tests for lso ocs disk inventory #6606

Bug 1878591: Added tests for lso ocs disk inventory #6606

afreen23 commented Sep 14, 2020

openshift-ci-robot commented Sep 14, 2020

afreen23 commented Sep 14, 2020

openshift-ci-robot commented Sep 14, 2020

afreen23 commented Sep 14, 2020

openshift-ci-robot commented Sep 14, 2020

gnehapk Sep 14, 2020

afreen23 Sep 14, 2020 •

edited

bipuladh Sep 14, 2020

afreen23 Sep 14, 2020

bipuladh Sep 14, 2020

afreen23 Sep 14, 2020 •

edited

bipuladh Sep 14, 2020

afreen23 Sep 18, 2020 •

edited

afreen23 Sep 18, 2020

gnehapk Sep 14, 2020

afreen23 Sep 14, 2020

gnehapk Sep 14, 2020

afreen23 Sep 14, 2020

bipuladh Sep 14, 2020

bipuladh left a comment

openshift-ci-robot commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

		expect(firstRow.get(2).getAttribute('innerText')).toBe(firstDisk.type \|\| '-');
		expect(firstRow.get(3).getAttribute('innerText')).toBe(firstDisk.model \|\| '-');

Bug 1878591: Added tests for lso ocs disk inventory #6606

Bug 1878591: Added tests for lso ocs disk inventory #6606

Conversation

afreen23 commented Sep 14, 2020

openshift-ci-robot commented Sep 14, 2020

afreen23 commented Sep 14, 2020

openshift-ci-robot commented Sep 14, 2020

afreen23 commented Sep 14, 2020

openshift-ci-robot commented Sep 14, 2020

Choose a reason for hiding this comment

afreen23 Sep 14, 2020 • 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

afreen23 Sep 14, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afreen23 Sep 18, 2020 • 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

bipuladh left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

afreen23 Sep 14, 2020 •

edited

afreen23 Sep 14, 2020 •

edited

afreen23 Sep 18, 2020 •

edited