Bug 1883766: Adjust tests due to UI changes #6850

irosenzw · 2020-10-06T12:31:37Z

Signed-off-by: Ido Rosenzwig irosenzw@redhat.com

openshift-ci-robot · 2020-10-06T12:31:44Z

@irosenzw: This pull request references Bugzilla bug 1883766, 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.7.0) matches configured target release for branch (4.7.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Details

In response to this:

Bug 1883766: Adjust tests due to UI changes

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.

atiratree

@irosenzw can we also include wizard tests in kubevirt-gating suite once we fix them here?

atiratree · 2020-10-06T15:13:09Z

...nd/packages/kubevirt-plugin/integration-tests/tests/utils/constants/enums/provisionSource.ts

can you please create a PR for moving object-enum to console/shared? So we don't have to depend on the kubevirt code base here?

Or I can do the PR - up to you.

I'll do it.

atiratree · 2020-10-06T15:13:41Z

frontend/packages/kubevirt-plugin/integration-tests/tests/utils/constants/wizard.ts

please remove alltogether

atiratree · 2020-10-06T15:22:11Z

frontend/packages/kubevirt-plugin/integration-tests/tests/mocks/mocks.ts

we can compare just the provisionSource here

atiratree · 2020-10-06T15:25:40Z

frontend/packages/kubevirt-plugin/integration-tests/tests/mocks/mocks.ts

source can be included in the ProvisionSource object

please also change provisionSourceInputs

what is wrong with provisionSourceInputs ?

the keys are a string and should rather refer to the enum. Anyway it fails currently since the method is a description and not the right constant.

atiratree · 2020-10-06T15:29:28Z

frontend/packages/kubevirt-plugin/integration-tests/tests/vmtemplate.resources.scenario.ts

this will not create a VM from a template. Please consider template use cases properly.

irosenzw · 2020-10-08T07:02:02Z

/retest

irosenzw · 2020-10-11T09:11:06Z

/retest

atiratree · 2020-10-12T12:59:02Z

frontend/packages/kubevirt-plugin/integration-tests/tests/mocks/mocks.ts

atiratree · 2020-10-12T13:18:35Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

please use the enum also in provisionSourceInputs

how would you suggest to use it? I used the values as keys. how else would you do it ?

[ ProvisionSource.Container.getValue()] like this? Or you can move the inputs to the provisionSource enum but please beware circular deps.

atiratree · 2020-10-12T13:19:38Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

why do you compare the value?

atiratree · 2020-10-12T13:19:52Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

same here, can compare just the objects

atiratree · 2020-10-12T13:21:41Z

...nd/packages/kubevirt-plugin/integration-tests/tests/utils/constants/enums/provisionSource.ts

using ObjectEnum.getAllClassEnumProperties makes this easier. Please see other object enums

Actually I don't use it. I'll just remove it

atiratree · 2020-10-12T13:24:37Z

frontend/packages/kubevirt-plugin/integration-tests/tests/vm.wizard.scenario.ts

can we compare just the provision sources? Also the value will never equal to description right?

atiratree · 2020-10-12T13:29:28Z

frontend/packages/kubevirt-plugin/integration-tests/tests/vmtemplate.resources.scenario.ts

do we need the whole template every time we want to create a VM from it. Can we utilize the create method to open the wizard (either from template or plain wizard) and process all the steps?

The create processes additional attributes like startOnCreate

atiratree · 2020-10-12T13:30:59Z

frontend/packages/kubevirt-plugin/integration-tests/tests/vmtemplate.resources.scenario.ts

there would be also a less need to create an instance of the wizard every time

gouyang · 2020-10-13T09:00:50Z

Test in vmtemplate.wizard.scenario.ts cannot be run due to vm.create() cannot load template data into wizard form.
All tests has to use

      await vmTemplate.createVMFromRowLink();
      await wizard.processWizard(vm.getData());

irosenzw · 2020-10-13T09:18:40Z

Test in vmtemplate.wizard.scenario.ts cannot be run due to vm.create() cannot load template data into wizard form.
All tests has to use
      await vmTemplate.createVMFromRowLink();
      await wizard.processWizard(vm.getData());

@gouyang I've changed vm.create() logic.
see link1 and link2

gouyang · 2020-10-13T09:29:18Z

Test in vmtemplate.wizard.scenario.ts cannot be run due to vm.create() cannot load template data into wizard form.
All tests has to use
      await vmTemplate.createVMFromRowLink();
      await wizard.processWizard(vm.getData());
@gouyang I've changed vm.create() logic.
see link1 and link2

It should work in this way though I believe there is no need template at all, I will give it a try.

irosenzw · 2020-10-13T09:35:55Z

Test in vmtemplate.wizard.scenario.ts cannot be run due to vm.create() cannot load template data into wizard form.
All tests has to use
      await vmTemplate.createVMFromRowLink();
      await wizard.processWizard(vm.getData());
@gouyang I've changed vm.create() logic.
see link1 and link2
Ah, I pulled the head and didn't see it. It should work in this way though I believe there is no need template at all.

That's OK it is a small change among many I made in this PR.
btw, you need the template when you create it in the first place.

atiratree · 2020-10-13T09:59:32Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

let's also use the same logic for navigating to Virtualization section as openWizard does

atiratree · 2020-10-13T10:19:25Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

selectTemplate and templateSelect should be removed

atiratree · 2020-10-13T10:20:12Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

is isEqual necessary?

atiratree · 2020-10-13T10:25:56Z

frontend/packages/kubevirt-plugin/integration-tests/tests/vmtemplate.wizard.scenario.ts

no need to navigateToListView if you change the openWizard logic

gouyang · 2020-10-13T11:24:59Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

There are two if (provisionSource) { now, better to remove the first one.

gouyang · 2020-10-13T11:36:35Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

provisionSource === ProvisionSource.DISK this never return true.

Can you please debug what is the value of provisionSource then? Maybe we are passing and setting just a simple "DISK" string or something wrong elsewhere. Or maybe the bootable prop is set to false in your case?

The value of them looks exactly the same, but it should not be the same object, so it never be true.
I tried to use provisionSource.value and ProvisionSource.DISK.value but it will have other issues.

Both of them returns

ProvisionSource { getValue: [Function], value: 'PXE', getDescription: [Function], getSource: [Function], description: 'PXE (network boot - adds network interface)', source: null } ProvisionSource { getValue: [Function], value: 'Disk', getDescription: [Function], getSource: [Function], description: 'Existing PVC (adds disk)', source: null }

It fails at create vm from disk.

1) Kubevirt create VM using wizard : ID(CNV-2446) Create VM using Disk. Error: Please select the boot source. at Wizard.<anonymous> /home/gouyang/console/frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts:83:13 at Generator.next <anonymous> at fulfilled /home/gouyang/console/frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts:5:58 at runMicrotasks <anonymous> at processTicksAndRejections internal/process/task_queues.js:97:5 From: Task: Run it("IDCNV-2446 Create VM using Disk.") in control flow at UserContext.<anonymous> /home/gouyang/console/frontend/node_modules/jasminewd2/index.js:94:19 From asynchronous test: Error at Suite.<anonymous> /home/gouyang/console/frontend/packages/kubevirt-plugin/integration-tests/tests/vm.wizard.scenario.ts:69:5 at Env.<anonymous> /home/gouyang/console/frontend/node_modules/jasmine-fail-fast/dist/jasmine-fail-fast.js:57:26 at Env.wrapper [as describe] /home/gouyang/console/frontend/node_modules/jasmine-fail-fast/node_modules/lodash/index.js:3592:19 at Object.<anonymous> /home/gouyang/console/frontend/packages/kubevirt-plugin/integration-tests/tests/vm.wizard.scenario.ts:40:1 at Module._compile internal/modules/cjs/loader.js:1137:30 at Module.m._compile /home/gouyang/console/frontend/node_modules/ts-node/src/index.ts:400:23

If use provisionSource.value and ProvisionSource.DISK.value, error see in vmtemplate tests.

1) Create VM from Template using wizard : ID(CNV-871) Create VM Template using Container. TypeError: Cannot read property 'value' of undefined at Wizard.<anonymous> /home/gouyang/console/frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts:294:27 at Generator.next <anonymous> at fulfilled /home/gouyang/console/frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts:5:58 at runMicrotasks <anonymous> at processTicksAndRejections internal/process/task_queues.js:97:5 From: Task: Run it("IDCNV-871 Create VM Template using Container.") in control flow at UserContext.<anonymous> /home/gouyang/console/frontend/node_modules/jasminewd2/index.js:94:19 From asynchronous test: Error at Suite.<anonymous> /home/gouyang/console/frontend/packages/kubevirt-plugin/integration-tests/tests/vmtemplate.wizard.scenario.ts:50:5 at Env.<anonymous> /home/gouyang/console/frontend/node_modules/jasmine-fail-fast/dist/jasmine-fail-fast.js:57:26 at Env.wrapper [as describe] /home/gouyang/console/frontend/node_modules/jasmine-fail-fast/node_modules/lodash/index.js:3592:19 at Object.<anonymous> /home/gouyang/console/frontend/packages/kubevirt-plugin/integration-tests/tests/vmtemplate.wizard.scenario.ts:25:1 at Module._compile internal/modules/cjs/loader.js:1137:30 at Module.m._compile /home/gouyang/console/frontend/node_modules/ts-node/src/index.ts:400:23

The failure related to the Disk provision source is not as a result of this PR but changes in the golden images feature. we should see a fix in the next HCO release

The main issue is that we are using cloneDeep on the data that we use, which clones the object and effectively changes the reference.

The solution is to echange of cloneDeep method in the integration tests with following custom implementation.

import { ObjectEnum } from '@console/shared/src/constants/object-enum'; import * as _ from 'lodash'; export const cloneDeepCustom = (object) => { return _.cloneDeepWith(object, (value) => { if (value instanceof ObjectEnum) { return value; } return undefined; }); };

PS please keep it in a separate file to prevent circular dependencies

@irosenzw can you please replace all _.isEqual to plain comparisons?

atiratree · 2020-10-14T15:09:34Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

+      (await view.goldenImageCloneCheckbox.isPresent()) &&
+      (await view.goldenImageCloneCheckbox.isSelected())
+    ) {
+      await click(view.goldenImageCloneCheckbox);


I noticed on my env that some clicks (like next) dont always go through. Can you please add this to the click implementation?

await browser.executeScript('arguments[0].scrollIntoView();', elem);

atiratree · 2020-10-14T15:12:32Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

        throw Error('VM OS not defined');
      }
+
+      if (provisionSource && !_.isEqual(provisionSource, ProvisionSource.DISK)) {


can you use a simpler logic please?

if (provisionSource) { if (provisionSource === ProvisionSource.DISK) { await this.disableGoldenImageCloneCheckbox(); } await this.selectProvisionSource(provisionSource); } else { throw Error('VM Provision source not defined'); }

but we said provisionSource === ProvisionSource.DISK doesn't work

it works if you clone it properly - please see #6850 (comment)

atiratree · 2020-10-15T17:07:22Z

frontend/packages/kubevirt-plugin/integration-tests/tests/models/wizard.ts

this is called right after the os selection which might make this appear. IMO we should wait at least 1sec if it does not appear

atiratree · 2020-10-15T17:10:16Z

frontend/packages/kubevirt-plugin/integration-tests/tests/vmtemplate.wizard.scenario.ts

create should be here. Please see https://github.com/openshift/console/pull/6850/files#r503824368

irosenzw · 2020-10-16T10:22:15Z

/retest

gouyang · 2020-10-19T01:16:41Z

/lgtm

openshift-ci-robot · 2020-10-19T01:16:55Z

@gouyang: changing LGTM is restricted to collaborators

Details

In response to this:

/lgtm

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.

jelkosz · 2020-10-19T06:56:27Z

/test kubevirt-plugin

atiratree · 2020-10-19T09:05:29Z

/test kubevirt-plugin

atiratree · 2020-10-19T10:01:29Z

looks in a pretty good shape (except one nitpick). Let's hope the tests pass

atiratree · 2020-10-19T11:26:20Z

/approve

Signed-off-by: Ido Rosenzwig <irosenzw@redhat.com>

rawagner · 2020-10-19T13:17:22Z

/lgtm

openshift-ci-robot · 2020-10-19T13:17:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gouyang, irosenzw, rawagner, suomiy

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~frontend/packages/console-shared/OWNERS~~ [rawagner]
~~frontend/packages/kubevirt-plugin/OWNERS~~ [rawagner,suomiy]

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

irosenzw · 2020-10-19T15:43:10Z

/retest

openshift-bot · 2020-10-19T15:55:09Z

/retest

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

openshift-bot · 2020-10-19T21:19:27Z

/retest

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

openshift-bot · 2020-10-19T22:24:31Z

/retest

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

openshift-ci-robot · 2020-10-19T23:49:57Z

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

openshift/console#6850

Bugzilla bug 1883766 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1883766: Adjust tests due to UI changes

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 the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Oct 6, 2020

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. component/kubevirt Related to kubevirt-plugin labels Oct 6, 2020

openshift-ci-robot requested review from rawagner and vojtechszocs October 6, 2020 12:31

irosenzw force-pushed the fix-nic-disk-integration-tests branch 2 times, most recently from 555ec87 to e18a806 Compare October 6, 2020 14:33

atiratree suggested changes Oct 6, 2020

View reviewed changes

irosenzw force-pushed the fix-nic-disk-integration-tests branch from e18a806 to 4cbe929 Compare October 7, 2020 15:28

openshift-ci-robot added the component/shared Related to console-shared label Oct 7, 2020

irosenzw force-pushed the fix-nic-disk-integration-tests branch from 4cbe929 to 84e6423 Compare October 7, 2020 16:33

This was referenced Oct 8, 2020

Bug 1896101: Added negative tests for migration from VMWare and RHV #6249

Merged

Bug 1883766: Adjust tests for UI changes #6788

Closed

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2020

irosenzw force-pushed the fix-nic-disk-integration-tests branch from 84e6423 to 461585a Compare October 11, 2020 07:17

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2020

irosenzw force-pushed the fix-nic-disk-integration-tests branch from 461585a to 1fdb93e Compare October 11, 2020 07:24

atiratree suggested changes Oct 12, 2020

View reviewed changes

irosenzw force-pushed the fix-nic-disk-integration-tests branch 2 times, most recently from 82eb10a to 823237c Compare October 12, 2020 16:46

atiratree suggested changes Oct 13, 2020

View reviewed changes

gouyang reviewed Oct 13, 2020

View reviewed changes

irosenzw force-pushed the fix-nic-disk-integration-tests branch from 8f4f9d2 to e6a1fc9 Compare October 14, 2020 11:31

atiratree suggested changes Oct 14, 2020

View reviewed changes

irosenzw force-pushed the fix-nic-disk-integration-tests branch from e6a1fc9 to 2d49325 Compare October 15, 2020 17:00

atiratree suggested changes Oct 15, 2020

View reviewed changes

irosenzw force-pushed the fix-nic-disk-integration-tests branch from 2d49325 to 1b4bf6c Compare October 19, 2020 09:38

irosenzw force-pushed the fix-nic-disk-integration-tests branch 2 times, most recently from 035eaa0 to fdcdfc7 Compare October 19, 2020 11:13

irosenzw force-pushed the fix-nic-disk-integration-tests branch from fdcdfc7 to bb3f7c0 Compare October 19, 2020 12:57

atiratree approved these changes Oct 19, 2020

View reviewed changes

Bug 1883766: Adjust tests due to UI changes

c47ac36

Signed-off-by: Ido Rosenzwig <irosenzw@redhat.com>

irosenzw force-pushed the fix-nic-disk-integration-tests branch from bb3f7c0 to c47ac36 Compare October 19, 2020 13:16

openshift-ci-robot assigned rawagner Oct 19, 2020

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

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

openshift-merge-robot merged commit 5406db6 into openshift:master Oct 19, 2020

spadgett added this to the v4.7 milestone Oct 21, 2020

Bug 1883766: Adjust tests due to UI changes #6850

Bug 1883766: Adjust tests due to UI changes #6850

Uh oh!

Conversation

irosenzw commented Oct 6, 2020

Uh oh!

openshift-ci-robot commented Oct 6, 2020

Uh oh!

atiratree left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irosenzw Oct 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irosenzw commented Oct 8, 2020

Uh oh!

irosenzw commented Oct 11, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gouyang commented Oct 13, 2020

Uh oh!

irosenzw commented Oct 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gouyang commented Oct 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

irosenzw commented Oct 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irosenzw Oct 7, 2020 •

edited

Loading

irosenzw commented Oct 13, 2020 •

edited

Loading

gouyang commented Oct 13, 2020 •

edited

Loading

irosenzw commented Oct 13, 2020 •

edited

Loading

atiratree Oct 14, 2020 •

edited

Loading