-
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
[Watcher] Update skipped jests tests #88225
[Watcher] Update skipped jests tests #88225
Conversation
💚 Build SucceededMetrics [docs]
To update your PR or re-run it, just comment with: |
@@ -119,7 +119,7 @@ export class WatchStatus { | |||
} | |||
|
|||
get lastFired() { | |||
const actionStatus = max(this.actionStatuses, 'lastExecution'); | |||
const actionStatus = maxBy(this.actionStatuses, 'lastExecution'); |
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 appears to be an actual bug that the test caught. I don't see how this was working before based on the lodash documentation.
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.
Nice it got caught 👍
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
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! Thanks for fixing those tests @alisonelizabeth !
@@ -119,7 +119,7 @@ export class WatchStatus { | |||
} | |||
|
|||
get lastFired() { | |||
const actionStatus = max(this.actionStatuses, 'lastExecution'); | |||
const actionStatus = maxBy(this.actionStatuses, 'lastExecution'); |
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.
Nice it got caught 👍
Fixes #87691
Some of the watcher tests were recently converted to jest via #87599. As part of that work, it was revealed that not all tests had been running in CI previously. Some of the tests were failing as-is and skipped. This PR addresses any issues and all watcher jest tests should be running now.