[Kubevirt] Test non-admin UI behaviour #2265

gouyang · 2019-08-05T11:57:40Z

Signed-off-by: Guohua Ouyang [email protected]

openshift-ci-robot · 2019-08-05T11:57:48Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gouyang
To complete the pull request process, please assign alecmerdler
You can assign the PR to them by writing /assign @alecmerdler in a comment when ready.

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

Details

Needs approval from an approver in each of these files:

frontend/OWNERS

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

rhrazdil

Please note that new place for tests will be in our plugin folder, which is in frontend/packages/kubevirt-plugin/integration-tests/.

frontend/integration-tests/protractor.conf.ts

rhrazdil · 2019-08-12T06:47:29Z

frontend/integration-tests/views/login.view.ts

please, avoid changing frontend/integration-tests/*, this would be better to handle in our kubevirt.login.

We should not be creating a separate kubevirt login imo unless it's absolutely necessary.

Some of kubevirt tests are invoking execSync to do some operations from cli to save time, to achieve this it need to do oc login within the login scenario.

@rhrazdil I think maybe we should use yaml dialog or VM wizard to create all resources instead of doing it from command line? So we can share one login scenario.

I revert this change anyway.

rhrazdil · 2019-08-12T06:48:54Z

frontend/package.json

Again, since this changes a file outside of our kubevirt-plugin folder, it would be better to add this change in a separate PR

Disagree... We should have the change with this PR that uses it.

@rhrazdil If we put tests in kubevirt-plugin folder, how do we use the openshift views?

frontend/integration-tests/tests/kubevirt/non-admin.scenario.ts

rhrazdil · 2019-08-12T08:00:08Z

frontend/integration-tests/tests/kubevirt/non-admin.scenario.ts

https://bugzilla.redhat.com/show_bug.cgi?id=1728523 is already VERIFIED

Yes, will address this TODO once the new build available with this fix available.

rhrazdil · 2019-08-12T08:03:02Z

frontend/integration-tests/tests/kubevirt/non-admin.scenario.ts

Maybe we could use more descriptive name of the secret than test ?

How about test-secret?

sounds good

rhrazdil · 2019-08-12T08:07:46Z

frontend/integration-tests/tests/kubevirt/non-admin.scenario.ts

If I understand correctly, specs that test view and noaccess should be skipped by calling xit, since these tests are not completely implemented

It does not need to skip it even the tests are not completely implemented, as long as the test can be run and it's PASS.

spadgett · 2019-08-13T16:47:27Z

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

I'm still unclear on why we need a separate login scenario just for kubevirt. I'd rather not fork the login test code unless there is a very compelling reason.

It just has one more line about oc login as our current tests requires.

You don’t need to use oc login for that. You just need the KUBECONFIG env var set. Admin console tests also run cli commands

Then we don't need another login. Could you comment on #2016?

spadgett · 2019-08-13T16:47:43Z

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

This is no longer needed. Please remove.

spadgett · 2019-08-13T16:49:32Z

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

!= undefined is not a good practice. We should not be disabling this linter rule

spadgett · 2019-08-13T16:50:31Z

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

This seems really hacky. We should get to the bottom of why this is an issue instead of adding arbitrary retries

spadgett · 2019-08-13T16:51:21Z

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

We should not be checking columns by index. Please add a data-test-id if needed

spadgett · 2019-08-13T16:52:01Z

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

Isn't the default timeout used automatically if not specified?

spadgett · 2019-08-13T16:52:51Z

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

Suggested change

export function execCommandFromCli(command: string) {

export function execCommandFromCLI(command: string) {

spadgett · 2019-08-13T16:52:59Z

frontend/integration-tests/views/kubevirt/login.view.ts

Remove this comment

Why is there a special kubevirt login view? We already have a login view

It's there because of a bug in web-ui, the logout does not return to login page which cause logout in view/login.view.ts does not work, this login view removes the last line.

But it should not a problem once we're using the some console, will remove it once I get a test for it asap.

spadgett · 2019-08-13T16:54:14Z

frontend/integration-tests/views/login.view.ts

We should not be creating a separate kubevirt login imo unless it's absolutely necessary.

spadgett · 2019-08-13T16:54:51Z

frontend/package.json

Disagree... We should have the change with this PR that uses it.

Signed-off-by: Guohua Ouyang <[email protected]>

rhrazdil · 2019-08-14T09:02:17Z

@gouyang please, could you base your patch on top of #2053? It adds most of the infra and places our kubevirt integration tests under kubevirt-plugin folder.

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

openshift-ci-robot requested review from rhamilto and spadgett August 5, 2019 11:57

gouyang changed the title ~~Test non-admin UI behaviour~~ [Kubevirt] Test non-admin UI behaviour Aug 6, 2019

gouyang mentioned this pull request Aug 7, 2019

add test for non-admin user kubevirt/web-ui#415

Closed

rhrazdil suggested changes Aug 12, 2019

View reviewed changes

spadgett requested changes Aug 13, 2019

View reviewed changes

Test non-admin UI behaviour

9baa0ed

Signed-off-by: Guohua Ouyang <[email protected]>

gouyang closed this Sep 25, 2019

	export function execCommandFromCli(command: string) {
	export function execCommandFromCLI(command: string) {

[Kubevirt] Test non-admin UI behaviour #2265

[Kubevirt] Test non-admin UI behaviour #2265

Uh oh!

Conversation

gouyang commented Aug 5, 2019

Uh oh!

openshift-ci-robot commented Aug 5, 2019

Uh oh!

rhrazdil left a comment

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!

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!

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!

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!

rhrazdil commented Aug 14, 2019

Uh oh!

Reviewers

Assignees

Labels