-
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
Fix typo in ReasonFound test #102690
Fix typo in ReasonFound test #102690
Conversation
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. Does this also need a backport?
Pinging @elastic/stack-monitoring (Team:Monitoring) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
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.
@matschaffer the change looks good, but I was curious to know why the test was green if there was a prop misspelled.
The enabler
prop is not used in the component, did you have the chance to look in more detail at what is, because I think it would make sense to clean it and remove it from the component.
Fair point, @estermv - I'll have to take a deeper look to say why the test seemed to work regardless. Not sure if the prop isn't used at all or just isn't involved in this particular test case. |
From looking at Line 40 in 4584a8b
enabler would only get invoked when props.property is xpack.monitoring.collection.enabled but we don't have a test case for that setting in this file.
The enabler gets covered by Line 24 in 4584a8b
@estermv given that I'd like to propose we merge this as-is. If there's a different change you'd recommend, please let me know! |
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.
@matschaffer makes sense to me! Thanks a lot for taking a look at it!
LGTM!!
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/uptime/overview·ts.Uptime app with real-world data overview page clears pagination parameters when size changesStandard Out
Stack Trace
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @matschaffer |
Friendly reminder: Looks like this PR hasn’t been backported yet. |
Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Summary
x-pack/plugins/monitoring/public/components/no_data/reasons/reason_found.test.js
was loadingReasonFound
withenabled
instead ofenabler
property. This didn't impact test functionality, but did cause the following test error:Checklist
For maintainers