Skip to content
This repository has been archived by the owner on Mar 1, 2024. It is now read-only.

Added tests for vm list/detail vm view actions (start, stop, restart, delete) #163

Merged
merged 1 commit into from
Jan 17, 2019

Conversation

rhrazdil
Copy link

@rhrazdil rhrazdil commented Jan 8, 2019

https://cnv-qe-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/job/test-ocp-3.11-cnv-1.4-integration-web-ui-rhrazdil/30/console

question:
I'm not quite satisfied with defining the VM object in scenario.ts file, especially when is this long. @vojtechszocs What do you think about it? Maybe we could create some resource file wher CRDs could be placed.

@rhrazdil rhrazdil force-pushed the vm_list_view_actions branch 3 times, most recently from 208ce97 to 97fc678 Compare January 9, 2019 12:29
@rhrazdil rhrazdil changed the title [WIP] Added tests for vm list/vies actions (start, stop, restart, delete) Added tests for vm list/vies actions (start, stop, restart, delete) Jan 9, 2019
@rhrazdil rhrazdil changed the title Added tests for vm list/vies actions (start, stop, restart, delete) Added tests for vm list/views actions (start, stop, restart, delete) Jan 9, 2019
@rhrazdil rhrazdil changed the title Added tests for vm list/views actions (start, stop, restart, delete) Added tests for vm list/detail vm view actions (start, stop, restart, delete) Jan 9, 2019
/**
* Performs action for VM via list view kebab menu.
*/
export const listViewAction = (name: string) => (action: string) => rowForName(name).$$('.co-kebab__button').first().click()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be possible to extract common functionality from listViewAction and detailViewAction? And also, extracting the kebap into a variable?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Couldn't find a way to extract the kebab element, for some reason when I had
export const $('.co-kebab__dropdown');
before the action function, the element was undefined.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coulnd't we just pass a function, something like getActionsDropdown()?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, we can't. Such function would have to be implemented by ElementFinder, which is what rowForName(name) returns.
But maybe just defining the string could be OK?
Like:
const listKebabDropdown = '.co-kebab__dropdown';
rowForName(name).$(listKebabDropdown)...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this work or am I missing something?

listViewAction = (name) => {
    getActionsDropdown = () => rowForName(name).$$('.co-kebab__button').first();
    getActionsDropdownMenu = () => rowForName(name).$(listViewKebabDropdown);
    return selectDropdownItem(getActionsDropdown, getActionsDropdownMenu);
}

selectDropdownItem  = (getActionsDropdown, getActionsDropdownMenu) => (action) =>
.
. common functionality (calls getActionsDropdown and getActionsDropdownMenu)
.

and detailViewAction implemented with the same mechanism

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, I'm volunteering on QE Camp. Yes, this approach will work.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@vojtechszocs
Copy link

question:
I'm not quite satisfied with defining the VM object in scenario.ts file, especially when is this long. @vojtechszocs What do you think about it? Maybe we could create some resource file wher CRDs could be placed.

I'd suggest to have simple tests along with reusable utilities.

As for *.scenario.ts files:

  • each describe represents a logical test suite, this is useful for organizing test code
  • each it (or test) represents a specific test case => specific UI workflow
  • try to keep test cases focused => end up having multiple test cases => easier to fix or update
  • after each test suite, the state of the k8s system should be like before that test suite

As for utilities:

  • I'd suggest adding e.g. frontend/integration-tests/tests/kubevirt/vm-utils.ts and use kubectl for programmatic k8s object management
  • templates for test-related k8s objects can be placed in e.g. frontend/integration-tests/tests/kubevirt/vm-mocks.ts
  • this helps to keep our tests focused on actual testing, e.g. for a test that operates on existing VMs, it's not really important how those VMs are created
  • this doesn't impact tests that exercise UI workflows like create VM via wizard or YAML

Following the existing convention, every created k8s object (regardless if created via UI workflow or via kubectl) should have a label automatedTestName: <testName> where <testName> can be taken as-is, or computed, from integration-tests/protractor.conf.ts.

@rhrazdil
Copy link
Author

I have refactored the VM provision tests, now there are 3 specs for each provision source.
Common functionality is being moved to utils.ts and CRDs to mocks.ts (I deviated from vm-* as it's already more general, for example there is network attachment definition in mocks.ts).

Comments regarding vm actions are fixed as well and ready for review.

@rhrazdil rhrazdil force-pushed the vm_list_view_actions branch 2 times, most recently from 5466b69 to 8d7b026 Compare January 14, 2019 15:48
@rhrazdil rhrazdil changed the title Added tests for vm list/detail vm view actions (start, stop, restart, delete) [WIP] Added tests for vm list/detail vm view actions (start, stop, restart, delete) Jan 14, 2019
@rhrazdil
Copy link
Author

Added WIP label again because there seems to be a issue with clicking the dropdown button. It's like the dropdown closes right after it is opened.

@rhrazdil rhrazdil changed the title [WIP] Added tests for vm list/detail vm view actions (start, stop, restart, delete) Added tests for vm list/detail vm view actions (start, stop, restart, delete) Jan 17, 2019
@atiratree atiratree merged commit 4a3c423 into kubevirt:master Jan 17, 2019
@vojtechszocs
Copy link

Thanks a lot @rhrazdil, I'm going to backport those changes into openshift#888.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants