-
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
[Kibana Overview] Fix failing test #139112
Conversation
}); | ||
|
||
it('click on Observability card leads to Observability', async () => { | ||
// skipped as on Cloud it's a different setup than locally | ||
xit('click on Observability card leads to Observability', async () => { |
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 do we keep those skipped tests if we can't run them on CI?
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.
So we know we need to tackle them at one point in time, which may not be right away.
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 see. Should we create an issue for it then and add the link in this comment?
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.
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.
Cool thanks. Sorry I meant in the comment of the code base 😊 So we know it is being tracked
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.
Hope it's ok now
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.
Yeah perfect thanks. It is just to be consistent with how the operation team flags skipped tests when there are too many failure in CI.
@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.
LGTM nice comments for tracking failing test issues
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 the tests
Thanks @majagrubic for putting in this fix, can we update the PR and merge as soon as possible to main and 8.4. |
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
@majagrubic this looks ready, should we merge? |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
* [Kibana Overview] Fix failing test * Remove comment * Fix regexp * Add a comment Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 1929395)
* [Kibana Overview] Fix failing test * Remove comment * Fix regexp * Add a comment Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 1929395)
Summary
Fixes: #137741
On Cloud, there is "Enterprise Search" solution as well, which messes up the order of the solutions, so checking for the exact order is not an option.
Checklist
Delete any items that are not applicable to this PR.
For maintainers