Skip to content
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

[Discover] Unskip _sidebar tests for MKI testing #202133

Merged
merged 9 commits into from
Dec 3, 2024

Conversation

kertal
Copy link
Member

@kertal kertal commented Nov 28, 2024

Summary

Resolves #195100

I tried to reproduce the flakiness on MKI, but it wasn't flaky in my tests. I've refactored the code, so a debugger would stop the execution in case the test would fail, I've tested it with a simple bash script with a for loop executing the test runner, but it didn't fail. So let's unskip this test for MKI and see how it goes.

I do agree with @jughosta

Looks very much like https://github.com/elastic/kibana-team/issues/928 where for some reason ES reports fields as Empty during some period of time but hard to reproduce.

In our 1:1 @jughosta mentioned, that she observered the same behavior in https://github.com/elastic/kibana-team/issues/928, but after a while she couldn't reproduce (the request for available fields returns just meta fields in this case, flagging all fields as unavailable). It look me a quite a while to setup all this, so I haven't had the chance to test it on a fresh instance. I've added code that in and error case the query is resubmitted. However it would be still of interest, what could lead to the failure, which is high likely the field_caps request returning no available fields.

Local testing

That's the commit that I used to open the functional test runner with Chrome Dev Tools automatically open (So I didn't have to do it manually): 586c5a1

And a local shell script flaky test runner, I've used like this

for i in {1..25}; do
  echo "Flaky Bash Runner iteration: $i"
  TEST_CLOUD_HOST_NAME={qaCloudURL} \
  TEST_CLOUD=1 \
  NODE_TLS_REJECT_UNAUTHORIZED=0 \
  TEST_ES_URL="{qaEsInstanceURL}" \
  TEST_KIBANA_URL="{qaKibanaInstanceURL}" \
  node --no-warnings scripts/functional_test_runner \
    --quiet \
    --config x-pack/test_serverless/functional/test_suites/security/common_configs/config.group6.ts
done

Executed in a VSCode JavaScript Debug Terminal, this will stop the test execution on adebugger statement in the code

Checklist

@kertal kertal self-assigned this Nov 28, 2024
@kertal kertal changed the title Discover mki debugging [Discover] unskip _sidebar tests for MKI testing Nov 28, 2024
@kertal
Copy link
Member Author

kertal commented Nov 28, 2024

/ci

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7532

[✅] x-pack/test_serverless/functional/test_suites/security/common_configs/config.group6.ts: 50/50 tests passed.
[✅] test/functional/apps/discover/group6/config.ts: 25/25 tests passed.

see run history

@kertal kertal added Feature:Discover Discover Application Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. backport:skip This commit does not require backporting labels Nov 28, 2024
@kertal kertal marked this pull request as ready for review November 29, 2024 11:04
@kertal kertal requested a review from a team as a code owner November 29, 2024 11:04
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@kertal kertal added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Nov 29, 2024
log.warning(
`Expected Sidebar Aria Description: ${expectedNumber}, got: ${ariaDescription}`
);
await queryBar.submitQuery();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's the code that should help to catch an occacional flakiness, resubmitting the query, so available fields should be returned like expected

@kertal kertal added the release_note:skip Skip the PR/issue when compiling release notes label Nov 29, 2024
@kertal kertal requested a review from wayneseymour November 29, 2024 12:19
@kertal kertal changed the title [Discover] unskip _sidebar tests for MKI testing [Discover] Unskip _sidebar tests for MKI testing Nov 29, 2024
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

cc @kertal

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes look good to me, and this seems like a tricky issue that's difficult to troubleshoot from the consumer side. Do we know if ES teams are aware of the issue? I didn't go through the process of trying to reproduce myself, but I'm not sure what else we could do on our end other than retry the requests.

@kertal
Copy link
Member Author

kertal commented Dec 3, 2024

The code changes look good to me, and this seems like a tricky issue that's difficult to troubleshoot from the consumer side. Do we know if ES teams are aware of the issue? I didn't go through the process of trying to reproduce myself, but I'm not sure what else we could do on our end other than retry the requests.

We raised this back in the days, however they also could not reproduce https://github.com/elastic/kibana-team/issues/928#issuecomment-2172843868 ... let's keep an eye open for this

@kertal kertal merged commit d1c2e04 into elastic:main Dec 3, 2024
9 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12133432347

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 202133

Questions ?

Please refer to the Backport tool documentation

@kertal
Copy link
Member Author

kertal commented Dec 3, 2024

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kertal added a commit to kertal/kibana that referenced this pull request Dec 3, 2024
(cherry picked from commit d1c2e04)

# Conflicts:
#	x-pack/test_serverless/functional/test_suites/common/discover/group6/_sidebar.ts
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 4, 2024
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kertal added a commit that referenced this pull request Dec 4, 2024
…2633)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Discover] Unskip _sidebar tests for MKI testing
(#202133)](#202133)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Matthias
Wilhelm","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-03T05:17:39Z","message":"[Discover]
Unskip _sidebar tests for MKI testing
(#202133)","sha":"d1c2e04912714c30e04f3790ed03ba5b5d154061","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","release_note:skip","v9.0.0","Team:DataDiscovery","backport:prev-minor"],"number":202133,"url":"https://github.com/elastic/kibana/pull/202133","mergeCommit":{"message":"[Discover]
Unskip _sidebar tests for MKI testing
(#202133)","sha":"d1c2e04912714c30e04f3790ed03ba5b5d154061"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202133","number":202133,"mergeCommit":{"message":"[Discover]
Unskip _sidebar tests for MKI testing
(#202133)","sha":"d1c2e04912714c30e04f3790ed03ba5b5d154061"}}]}]
BACKPORT-->
@kibanamachine kibanamachine added v8.18.0 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Dec 4, 2024
hop-dev pushed a commit to hop-dev/kibana that referenced this pull request Dec 5, 2024
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 9, 2024
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FLAKY ON MKI] x-pack/test_serverless/functional/test_suites/common/discover/group6/_sidebar·ts
4 participants