-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Update jest to v24 #31825
Update jest to v24 #31825
Conversation
💔 Build Failed |
There's also jest inside |
Mock interface doesn't match CallCluster. Requires additional work
], | ||
"results": Array [ |
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.
it doesn't make sense to test jest
internals, so I removed part of snapshot test (yes, it had a conflict with new jest version)
@@ -22,7 +22,7 @@ import { LOCK_WINDOW, ReindexActions, reindexActionsFactory } from './reindex_ac | |||
|
|||
describe('ReindexActions', () => { | |||
let client: jest.Mocked<any>; | |||
let callCluster: jest.Mock<CallCluster>; | |||
let callCluster: jest.Mock; |
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.
had to delete declaration as mock doesn't implement full CallCluster
contract
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.
Sounds good, I'll fix this up to be complete after this is merged (I wrote this).
x-pack/package.json
Outdated
@@ -46,7 +46,7 @@ | |||
"@types/expect.js": "^0.3.29", | |||
"@types/graphql": "^0.13.1", | |||
"@types/history": "^4.6.2", | |||
"@types/jest": "^23.3.1", | |||
"@types/jest": "^24.0.6", |
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.
@epixa why we need to have another package.json
for x-pack?
x-pack/package.json
Outdated
@@ -202,6 +203,8 @@ | |||
"inline-style": "^2.0.0", | |||
"intl": "^1.2.5", | |||
"io-ts": "^1.4.2", | |||
"jest": "^24.1.0", |
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.
why packages are duplicated in "devDependencies" and "dependencies"?
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.
Probably a mistake. I would be surprised if we wanted to ship jest
in the production build to users.
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.
Sometimes it happens. Contentful did it to run their functional tests 😅
💔 Build Failed |
💚 Build Succeeded |
Pinging @elastic/kibana-platform |
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 from APM. Thanks for updating Jest!
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.
A few notes, mostly about the tests themselves rather than your changes. Figured now is a good time to fix some of these issues since their aren't too many, but I'll leave some of the trickier cases at your discretion on whether or not they're worth fixing right now.
@@ -12,7 +12,7 @@ exports[`#start constructs UiSettingsClient and UiSettingsApi: UiSettingsApi arg | |||
], | |||
"results": Array [ | |||
Object { | |||
"isThrow": false, | |||
"type": "return", |
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.
These snapshots look like they're testing more jest internals rather than just the call arguments themselves. Can we fix these too?
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.
Mocked functions are inside JS objects that serialised for snapshot testing. I didn't find a cheap way to fix it and decided to keep it as is.
As an alternative solution, we can define custom serialiser for mocked functions https://jestjs.io/docs/en/configuration#snapshotserializers-array-string
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.
Perfectly good reason.
@@ -37,7 +37,7 @@ Array [ | |||
], | |||
"results": Array [ | |||
Object { | |||
"isThrow": false, | |||
"type": "return", |
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.
Jest internals here as well, though this may be a bit hard to fix since it's an entire object. I'm not sure why we're testing such large snapshots like this though.
@@ -27,7 +27,7 @@ let appenderMocks: Appender[]; | |||
let logger: BaseLogger; | |||
|
|||
const timestamp = new Date(2012, 1, 1); | |||
jest.spyOn(global, 'Date').mockImplementation(() => timestamp); | |||
jest.spyOn<any, any>(global, 'Date').mockImplementation(() => timestamp); |
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.
This spy doesn't ever seem to be restored with mockRestore
. We should make sure that happens explicitly after these tests run since we don't have the restoreMocks
config option set to true.
As an aside: it'd be awesome if there was linter rule we could add for this.
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.
👍 will fix. don't know why but I was sure we use restoreMocks: true
@@ -27,10 +27,10 @@ const timestamp = new Date(Date.UTC(2012, 1, 1)); | |||
const mockConsoleLog = jest.spyOn(global.console, 'log').mockImplementation(() => { | |||
// noop | |||
}); | |||
jest.spyOn(global, 'Date').mockImplementation(() => timestamp); | |||
jest.spyOn<any, any>(global, 'Date').mockImplementation(() => timestamp); |
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.
Same as above about mockRestore
@@ -22,7 +22,7 @@ import { LOCK_WINDOW, ReindexActions, reindexActionsFactory } from './reindex_ac | |||
|
|||
describe('ReindexActions', () => { | |||
let client: jest.Mocked<any>; | |||
let callCluster: jest.Mock<CallCluster>; | |||
let callCluster: jest.Mock; |
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.
Sounds good, I'll fix this up to be complete after this is merged (I wrote this).
src/core/server/root/index.test.ts
Outdated
}); | ||
const mockProcessExit = jest | ||
.spyOn(global.process, 'exit') | ||
.mockImplementation(() => undefined as never); |
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.
Also missing a restore.
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
@@ -281,7 +281,7 @@ | |||
"@types/has-ansi": "^3.0.0", |
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.
question: how about updating babel-jest
to 24.1.0
here and in x-pack
?
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.
babel-jest@v24
uses babel@v7
which require @kbn/babel-preset/node_preset
update. seems like quite a bit task
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.
will be done in #32326
x-pack/package.json
Outdated
@@ -202,6 +202,7 @@ | |||
"inline-style": "^2.0.0", | |||
"intl": "^1.2.5", | |||
"io-ts": "^1.4.2", | |||
"jest-cli": "^24.1.0", |
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.
question/issue: why do we need jest-cli
in non-dev dependencies?
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.
seems like a mistake when updated version via yarn cli. will fix now
💚 Build Succeeded |
* udpate jest, jest-cli, @types/jest to v24 * fix type error in kibana-i18n package * return serivce explicitly to fix typings * add explicit never * suppress typings errors * update jest versions in x-pack * make tests in x-pack more robust and fix incompatibility * suppress CallCluster mock typings Mock interface doesn't match CallCluster. Requires additional work * x-pack. resolve other typing conflicts * remove unused types/jest * fix snapshots * restore mocks after jest.spyOn * remove outdated definitions for jest * cleanup x-pack package.json and update @types/jest * fix tests merged from master * updated yarn.lock and log errors for scripts/type_check * This commit fixes error in TS, which failed on parsing the file. * suppress type errors from master * jest-cli is devDep
* udpate jest, jest-cli, @types/jest to v24 * fix type error in kibana-i18n package * return serivce explicitly to fix typings * add explicit never * suppress typings errors * update jest versions in x-pack * make tests in x-pack more robust and fix incompatibility * suppress CallCluster mock typings Mock interface doesn't match CallCluster. Requires additional work * x-pack. resolve other typing conflicts * remove unused types/jest * fix snapshots * restore mocks after jest.spyOn * remove outdated definitions for jest * cleanup x-pack package.json and update @types/jest * fix tests merged from master * updated yarn.lock and log errors for scripts/type_check * This commit fixes error in TS, which failed on parsing the file. * suppress type errors from master * jest-cli is devDep Removes sinon from saved objects unit tests. (elastic#32045) (elastic#32151) * Removes sinon from saved objects unit tests. * Uses mockResolvedValue for return values as promises. temp
* udpate jest, jest-cli, @types/jest to v24 * fix type error in kibana-i18n package * return serivce explicitly to fix typings * add explicit never * suppress typings errors * update jest versions in x-pack * make tests in x-pack more robust and fix incompatibility * suppress CallCluster mock typings Mock interface doesn't match CallCluster. Requires additional work * x-pack. resolve other typing conflicts * remove unused types/jest * fix snapshots * restore mocks after jest.spyOn * remove outdated definitions for jest * cleanup x-pack package.json and update @types/jest * fix tests merged from master * updated yarn.lock and log errors for scripts/type_check * This commit fixes error in TS, which failed on parsing the file. * suppress type errors from master * jest-cli is devDep Removes sinon from saved objects unit tests. (#32045) (#32151) * Removes sinon from saved objects unit tests. * Uses mockResolvedValue for return values as promises. temp
* Update jest to v24 (#31825) * udpate jest, jest-cli, @types/jest to v24 * fix type error in kibana-i18n package * return serivce explicitly to fix typings * add explicit never * suppress typings errors * update jest versions in x-pack * make tests in x-pack more robust and fix incompatibility * suppress CallCluster mock typings Mock interface doesn't match CallCluster. Requires additional work * x-pack. resolve other typing conflicts * remove unused types/jest * fix snapshots * restore mocks after jest.spyOn * remove outdated definitions for jest * cleanup x-pack package.json and update @types/jest * fix tests merged from master * updated yarn.lock and log errors for scripts/type_check * This commit fixes error in TS, which failed on parsing the file. * suppress type errors from master * jest-cli is devDep * updated yarn.lock
Fixes elastic#32619 With elastic#31825 the reporter contract changed slightly and this updates our custom reporter to fix our junit reports.
Fixes elastic#32619 With elastic#31825 the reporter contract changed slightly and this updates our custom reporter to fix our junit reports.
Fixes elastic#32619 With elastic#31825 the reporter contract changed slightly and this updates our custom reporter to fix our junit reports.
Summary
Update to v24
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers