Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubevirt: split VMI selectors #3191

Merged
merged 1 commit into from Nov 6, 2019
Merged

kubevirt: split VMI selectors #3191

merged 1 commit into from Nov 6, 2019

Conversation

mareklibra
Copy link
Contributor

Hot fix to enable integration tests #3189.

When importing from kubevirt-plugin/src/selectors/vmi/selectors.ts in an integration
test, the tests are failing on unrelated errors (i.e. from CSS loader).

There is suspicion that the issue is related to node.js environment where window
object is missing but @console/internal modules are requesting it without function
wrapper (i.e. k8s.ts and k8sBasePath). I will further investigate.

This change does not break other flows, just enables to import simplier
modules to by-pass immediate execution of code in more complex ones.

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. component/kubevirt Related to kubevirt-plugin labels Nov 1, 2019
@mareklibra
Copy link
Contributor Author

mareklibra commented Nov 1, 2019

@rhrazdil , can you please try to stack #3189 on top of this and use

import { getVMIDisks } from '../../src/selectors/vmi/selectors/basic';

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

I do not have privileges to push changes to your branch.

Hot fix to enable integration tests.

This change does not break other flows, just enables to import simplier
modules to by-pass immediate execution of code in more complex ones.
@rhrazdil
Copy link

rhrazdil commented Nov 4, 2019

@mareklibra with this change integration-tests can be executed with getVMIDisks imported

@vojtechszocs
Copy link
Contributor

There is suspicion that the issue is related to node.js environment where window
object is missing but @console/internal modules are requesting it without function
wrapper (i.e. k8s.ts and k8sBasePath). I will further investigate.

Unit tests (Jest) use jsdom env. (jest-environment-jsdom) which emulates web APIs. We have additional mocks (under frontend/__mocks__) that further tweak the global window object.

Integration tests (Protractor) interact with Chrome browser instance and AFAIK don't use jsdom env. for the code being executed within the Node.js runtime. Protractor uses its own test runner (Jasmine) whose API is similar to one used in Jest.

When importing from kubevirt-plugin/src/selectors/vmi/selectors.ts in an integration
test, the tests are failing on unrelated errors (i.e. from CSS loader).

Jest is configured (via moduleNameMapper) to mock images and stylesheets. AFAIK, Protractor isn't configured as such, so that's probably the root cause of those errors.

@vojtechszocs
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 6, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mareklibra, vojtechszocs

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:

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

@openshift-merge-robot openshift-merge-robot merged commit 4f6ed74 into openshift:master Nov 6, 2019
@spadgett spadgett added this to the v4.3 milestone Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/kubevirt Related to kubevirt-plugin lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants