Add VM Wizard test cases for Kubevirt plugin #2053

rhrazdil · 2019-07-16T12:02:14Z

Adding test cases for working with VM Wizard in Kubevirt plugin to openshift console.

Added Tests:
https://polarion.engineering.redhat.com/polarion/redirect/project/CNV/workitem?id=CNV-771
https://polarion.engineering.redhat.com/polarion/redirect/project/CNV/workitem?id=CNV-869
https://polarion.engineering.redhat.com/polarion/redirect/project/CNV/workitem?id=CNV-870
https://polarion.engineering.redhat.com/polarion/redirect/project/CNV/workitem?id=CNV-2446
https://polarion.engineering.redhat.com/polarion/redirect/project/CNV/workitem?id=CNV-2447

Tests are organized under {tests,views}/kubevirt/* directory, executed with kubevirt test suite.

rhrazdil · 2019-07-16T13:43:31Z

/assign @bparees

bparees · 2019-07-16T13:48:32Z

/unassign
/assign @spadgett

rhrazdil · 2019-07-17T10:17:23Z

In reaction to #2051, moving this to WIP, and will move the code to the kubevirt-plugin directory

rhrazdil · 2019-07-17T11:52:41Z

@yaacov Moved the tests to our kubevirt-plugin package. The last change that is made to openshift core code is in protractor.conf.ts, this can be removed when #2051 it merged.
@vojtechszocs I believe you can review this PR now.

yaacov · 2019-07-22T10:22:23Z

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

It looks like lint doesn't like the order here, KubevirtDetailView should be below rowForName:
2478b2f

frontend/packages/kubevirt-plugin/integration-tests/tests/utils/consts.ts

yaacov · 2019-07-23T09:51:42Z

frontend/packages/kubevirt-plugin/integration-tests/views/kubevirtDetailView.view.ts

newResourceRowInput is also for nic , should it be less specific ?

export const newResourceRowInput = $('[data-id^=create-]')

frontend/packages/kubevirt-plugin/integration-tests/views/kubevirtDetailView.view.ts

rhrazdil · 2019-08-12T13:40:38Z

/test e2e-aws-console

vojtechszocs · 2019-08-12T14:40:25Z

frontend/integration-tests/protractor.conf.ts

      'tests/monitoring.scenario.ts',
      'tests/crd-extensions.scenario.ts',
    ]),
+    kubevirt: [


@rhrazdil @yaacov @spadgett

Does it make sense to have a convention where the name of test suite corresponds with the name of Console plugin package?

For example, use kubevirt-plugin for package @console/kubevirt-plugin:

'kubevirt-plugin': [ /* test scenarios */ ],

This would go well with active plugin resolution mechanism (after we add support for this):

# Active plugin list will be [app, kubevirt-plugin, dev-console] # List of test suites will be enhanced to [foo, bar, kubevirt-plugin, dev-console] CONSOLE_PLUGINS=kubevirt-plugin,dev-console yarn run test-suite --suite foo,bar # Same as above, without specifying non-plugin test suites CONSOLE_PLUGINS=kubevirt-plugin,dev-console yarn run test-suite

Also, why not use the suite() function here?

Is it because e.g. kubevirt.login.scenario.ts effectively replaces standard login.scenario.ts?

Yep, makes sense.
suite is not used because of custom login.
@spadgett already wondered what's the reason behind the custom login scenario.
I added it because at certain point login page of openshift/console and kubevirt/web-ui were different and login.scenario.ts didn't work. That is not an issue now, however, login.scenario.ts adds numerous test cases, that are not relevant to our plugin.

That is not an issue now, however, login.scenario.ts adds numerous test cases, that are not relevant to our plugin.

OK, understood, thanks for clarification.

suite is not used because of custom login.

You could also modify the suite function to support a custom (non-standard) login scenario(s) but this can be done later on.

vojtechszocs · 2019-08-12T14:50:58Z

frontend/integration-tests/protractor.conf.ts

      'tests/crd-extensions.scenario.ts',
    ]),
+    kubevirt: [
+      '../packages/kubevirt-plugin/integration-tests/tests/kubevirt.login.scenario.ts',


Nit: I'd extract the path to packages directory:

import * as path from 'path'; const consolePkgRoot = path.resolve(__dirname, '../packages'); `${consolePkgRoot}/kubevirt-plugin/integration-tests/tests/kubevirt.login.scenario.ts`

p.s. we want to move all this tests to the plugin dir using #2051 , so the links will change ...

@vojtechszocs Yeah, the path looks bad, I can do this. But with #2051, we will remove kubevirt suite from protractor.conf anyway

p.s. we want to move all this tests to the plugin dir using #2051 , so the links will change ...

Yes, they will change in future 😃 once #2051 is merged, we should start moving all integration tests (including the non-plugin ones) into appropriate monorepo packages.

@vojtechszocs Yeah, the path looks bad, I can do this. But with #2051, we will remove kubevirt suite from protractor.conf anyway

Agreed, let's keep the change minimal for now. #2051 will change this anyway.

vojtechszocs · 2019-08-12T14:58:46Z

frontend/packages/kubevirt-plugin/integration-tests/tests/kubevirt.login.scenario.ts

+describe('Authentication', () => {
+  it('Web console logs in.', async () => {
+    await browser.get(appHost);
+    if (process.env.BRIDGE_BASE_ADDRESS !== undefined) {


We could replace if condition with:

expect(process.env.BRIDGE_BASE_ADDRESS).toBeDefined();

to enforce our expectation.

Actually, we use this variable to distinguish between execution against cluster UI and local bridge. If BRIDGE_BASE_ADDRESS is undefined, we assume running against localhost, which doesn't require logging in

Understood, thanks for clarification.

vojtechszocs · 2019-08-12T15:08:23Z

frontend/packages/kubevirt-plugin/integration-tests/tests/kubevirt.login.scenario.ts

@@ -0,0 +1,18 @@
+import { execSync } from 'child_process';
+import { browser } from 'protractor';
+import { appHost } from '../../../../integration-tests/protractor.conf';


Please avoid relative paths that point outside the monorepo. Let's do two things here:

1, add integration-tests/package.json

{ "name": "@console/integration-tests", "version": "0.0.0-fixed", "description": "Console integration tests to be moved into appropriate packages", "private": true }

2, update root package.json & run yarn to update symlinks in node_modules

"workspaces": [ "packages/*", "public", "integration-tests" ],

3, update all imports in plugin packages

-import { appHost } from '../../../../integration-tests/protractor.conf'; +import { appHost } from '@console/integration-tests/protractor.conf';

we do it in #2051 , see comment about links.

Can we use @console/internal-integration-tests ?

Ref: https://github.com/openshift/console/pull/2051/files#r313030053

p.s.
if changing here, this should be changed in all relevant places.

Done. Kobi told me about this comment: #2051 (comment)
Is it relevant for this PR? Should I try to incorporated it?

@vojtechszocs So I added this change and then removed it to not create conflict with Kobi's PR #2051

OK, we can do it in Kobi's PR #2051.

vojtechszocs · 2019-08-12T15:12:29Z

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

@@ -0,0 +1,195 @@
+/* eslint-disable no-await-in-loop, no-console, no-underscore-dangle */


This (and other utils if appropriate) could be moved under packages/console-shared/src/test-utils.

yaacov · 2019-08-14T06:14:07Z

@rhrazdil @vojtechszocs if #2051 is planed for merge in the same phase as this one, we can revert the parts about making integration-tests a separate monorepo workspace [1] because it will create a conflict with #2051, WDYT ?

[1] #2053

rhrazdil · 2019-08-14T06:35:28Z

@yaacov I can revert those changes, so that you don't need to change #2051, since your PR is practically ready, and there may be additional changes in this one.

atiratree · 2019-08-22T10:55:31Z

This will need quite some changes once we move to the new wizard in the future

vojtechszocs · 2019-08-22T17:57:21Z

frontend/packages/console-shared/src/test-utils/utils.ts

If the runtime for this code is Node.js (i.e. Protractor or Jest) then you can simply add

/* eslint-env node */

at the top of the file.

By doing that, the above rule disable list might not be necessary.

didn't help

vojtechszocs · 2019-08-22T18:15:13Z

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

cc @christianvogt

Please add frontend/packages/kubevirt-plugin/integration-tests/.eslintrc.js

module.exports = { root: true, extends: [ 'plugin:console/base', 'plugin:console/typescript', 'plugin:console/node', 'plugin:console/prettier', ], };

The root: true option is used to override the parent cross-package packages/.eslintrc.js.

Ideally, we shouldn't repeat those /* eslint-disable ... */ comments in multiple files.

Instead, simply edit the closest .eslintrc.js file in hierarchy and update it like

rules: { 'no-unused-vars': 'off', 'no-console': ['error', { allow: ['warn', 'error', 'log', 'info'] }], },

vojtechszocs · 2019-08-22T18:20:55Z

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

You can disable or tweak no-await-in-loop (and other rules too) in the appropriate .eslintrc.js file.

vojtechszocs · 2019-08-22T18:22:33Z

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

Just a reminder that Console acronym naming convention is to use uppercase, e.g. getAttachedNICs.

In context of KubeVirt plugin package, we should pick one convention and stick with it consistently.

vojtechszocs · 2019-08-22T18:25:22Z

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

I really like the way how we express the logical operations here. It's highly readable and clean!

vojtechszocs · 2019-08-22T18:28:52Z

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

This will probably change in future due to VM wizard changes made by @suomiy.

Yes, I expect I'll have to refactor.

vojtechszocs · 2019-08-22T18:31:31Z

frontend/packages/kubevirt-plugin/integration-tests/tests/utils/types.ts

Please use CamelCase when declaring types, i.e. ProvisionOption.

(Also in other files, if relevant.)

vojtechszocs · 2019-08-22T18:36:48Z

frontend/packages/kubevirt-plugin/integration-tests/views/wizard.view.ts

Nit: please add newline.

vojtechszocs · 2019-08-22T18:39:09Z

frontend/packages/kubevirt-plugin/integration-tests/views/wizard.view.ts

Using ESDoc @param tag properly, great to see this! 😃

(In existing code, most @param tags are auto-generated stubs with no added value.)

rhrazdil · 2019-08-23T13:36:31Z

/test e2e-aws-console

vojtechszocs · 2019-08-23T17:40:15Z

@rhrazdil I'll do the proposed ESLint related improvements in a separate PR.

/lgtm
/approve

openshift-ci-robot · 2019-08-23T17:40:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhrazdil, suomiy, vojtechszocs

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/OWNERS~~ [vojtechszocs]

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

vojtechszocs · 2019-08-23T18:09:23Z

@rhrazdil I'll do the proposed ESLint related improvements in a separate PR.

ESLint changes addressed in #2472.

openshift-bot · 2019-08-23T21:14:45Z

/retest

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

openshift-bot · 2019-08-23T21:28:22Z

/retest

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

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 16, 2019

openshift-ci-robot requested review from jcaianirh and spadgett July 16, 2019 12:02

openshift-ci-robot assigned bparees Jul 16, 2019

openshift-ci-robot assigned spadgett and unassigned bparees Jul 16, 2019

rhrazdil changed the title ~~Add VM Wizard test cases for Kubevirt plugin~~ [WIP] Add VM Wizard test cases for Kubevirt plugin Jul 17, 2019

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2019

rhrazdil force-pushed the console-vmwizard-kubevirt-tests branch from 77cfd6c to 5359f06 Compare July 17, 2019 11:47

rhrazdil changed the title ~~[WIP] Add VM Wizard test cases for Kubevirt plugin~~ Add VM Wizard test cases for Kubevirt plugin Jul 17, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2019

rhrazdil force-pushed the console-vmwizard-kubevirt-tests branch from 5359f06 to 918bb3e Compare July 17, 2019 12:14

yaacov mentioned this pull request Jul 18, 2019

Add VM Template test cases for Kubevirt plugin #2066

Merged

rhrazdil force-pushed the console-vmwizard-kubevirt-tests branch 5 times, most recently from 8140c57 to c7dd5d7 Compare July 22, 2019 09:50

yaacov reviewed Jul 22, 2019

View reviewed changes

rhrazdil force-pushed the console-vmwizard-kubevirt-tests branch 2 times, most recently from 328e164 to f32c3e2 Compare July 22, 2019 11:01

yaacov reviewed Jul 23, 2019

View reviewed changes

frontend/packages/kubevirt-plugin/integration-tests/tests/utils/consts.ts Outdated Show resolved Hide resolved

yaacov reviewed Jul 23, 2019

View reviewed changes

frontend/packages/kubevirt-plugin/integration-tests/views/kubevirtDetailView.view.ts Outdated Show resolved Hide resolved

rhrazdil mentioned this pull request Jul 23, 2019

Adding test cases for VM actions #2125

Merged

rhrazdil force-pushed the console-vmwizard-kubevirt-tests branch from f32c3e2 to 600fe45 Compare July 23, 2019 12:54

vojtechszocs suggested changes Aug 12, 2019

View reviewed changes

vojtechszocs mentioned this pull request Aug 12, 2019

Add plugin integration tests to protractor #2051

Merged

5 tasks

rhrazdil force-pushed the console-vmwizard-kubevirt-tests branch from 8a4902b to c14a00c Compare August 13, 2019 13:31

openshift-ci-robot added the component/shared Related to console-shared label Aug 13, 2019

rhrazdil force-pushed the console-vmwizard-kubevirt-tests branch from c14a00c to 52f273b Compare August 14, 2019 08:11

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

rhrazdil force-pushed the console-vmwizard-kubevirt-tests branch from 52f273b to 536347b Compare August 14, 2019 08:18

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

rhrazdil mentioned this pull request Aug 14, 2019

[Kubevirt] Test non-admin UI behaviour #2265

Closed

rhrazdil force-pushed the console-vmwizard-kubevirt-tests branch from 536347b to 0f0f30e Compare August 19, 2019 12:49

atiratree approved these changes Aug 22, 2019

View reviewed changes

vojtechszocs suggested changes Aug 22, 2019

View reviewed changes

Add VM Wizard test cases for Kubevirt plugin

7da161e

rhrazdil force-pushed the console-vmwizard-kubevirt-tests branch from 0f0f30e to 7da161e Compare August 23, 2019 12:17

openshift-ci-robot assigned vojtechszocs Aug 23, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 23, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 23, 2019

vojtechszocs mentioned this pull request Aug 23, 2019

Improve the use of ESLint configs #2472

Merged

openshift-merge-robot merged commit 800b645 into openshift:master Aug 23, 2019

spadgett added this to the v4.2 milestone Sep 6, 2019

		@@ -0,0 +1,195 @@
		/* eslint-disable no-await-in-loop, no-console, no-underscore-dangle */

Add VM Wizard test cases for Kubevirt plugin #2053

Add VM Wizard test cases for Kubevirt plugin #2053

Uh oh!

Conversation

rhrazdil commented Jul 16, 2019

Uh oh!

rhrazdil commented Jul 16, 2019

Uh oh!

bparees commented Jul 16, 2019

Uh oh!

rhrazdil commented Jul 17, 2019

Uh oh!

rhrazdil commented Jul 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rhrazdil commented Aug 12, 2019

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!

vojtechszocs Aug 22, 2019 • 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!

rhrazdil Aug 13, 2019 • 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!

rhrazdil Aug 13, 2019 • 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!

yaacov commented Aug 14, 2019

Uh oh!

rhrazdil commented Aug 14, 2019

Uh oh!

atiratree commented Aug 22, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vojtechszocs Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

vojtechszocs Aug 22, 2019 •

edited

Loading

rhrazdil Aug 13, 2019 •

edited

Loading

rhrazdil Aug 13, 2019 •

edited

Loading

vojtechszocs Aug 22, 2019 •

edited

Loading