-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Convert vega tests to jest #71073
Convert vega tests to jest #71073
Conversation
@elasticmachine merge upstream |
user doesn't have permission to update head repository |
@elasticmachine merge upstream |
user doesn't have permission to update head repository |
@elasticmachine merge upstream |
user doesn't have permission to update head repository |
@elasticmachine merge upstream |
user doesn't have permission to update head repository |
Pinging @elastic/kibana-app (Team:KibanaApp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't use the __test__
folder name for jest test utils, as this is reserved folder name for karma/mocha env. You could use test
or test_utils
instead
vegaVisualizationDependencies = { | ||
core: { | ||
uiSettings: coreStart.uiSettings, | ||
}, | ||
plugins: { | ||
data: { | ||
query: { | ||
timefilter: { | ||
timefilter: {}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to TS VegaVisualizationDependencies
seems that right implementation of this mock is:
vegaVisualizationDependencies = { | |
core: { | |
uiSettings: coreStart.uiSettings, | |
}, | |
plugins: { | |
data: { | |
query: { | |
timefilter: { | |
timefilter: {}, | |
}, | |
}, | |
}, | |
}, | |
}; | |
vegaVisualizationDependencies = { | |
core: coreMock.createSetup(), | |
plugins: { | |
data: dataPluginMock.createSetupContract() | |
}, | |
}; |
This will help to avoid TS errors when moving to typescript (if it will)
I noticed there is the |
💚 Build SucceededBuild metrics
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🍰
* Convert vega tests to jest Part of elastic#57813 * Remove unused config * Move assets to __test__ folder and remove unnecessary code * clenup * cleanup * Revert default.spec.hjson file and mock default_spec * Refactor some code Co-authored-by: Alexey Antonov <[email protected]>
* Convert vega tests to jest Part of #57813 * Remove unused config * Move assets to __test__ folder and remove unnecessary code * clenup * cleanup * Revert default.spec.hjson file and mock default_spec * Refactor some code Co-authored-by: Alexey Antonov <[email protected]> Co-authored-by: Alexey Antonov <[email protected]>
Part of #57813
Summary
Converted vega tests to jest and removed old ones from legacy.
Checklist
Delete any items that are not applicable to this PR.
For maintainers