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

Make sort integration test resilient to network delays. #196516

Merged

Conversation

awahab07
Copy link
Contributor

@awahab07 awahab07 commented Oct 16, 2024

Fixes #182017

Summary

The PR tries to address the flakiness from the test.

@awahab07 awahab07 requested a review from a team as a code owner October 16, 2024 10:10
@awahab07 awahab07 added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:obs-ux-logs Observability Logs User Experience Team labels Oct 16, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

@awahab07 awahab07 added the bug Fixes for quality problems that affect the customer experience label Oct 16, 2024
await retry.try(async () => {
await PageObjects.observabilityLogsExplorer.clickSortButtonBy('desc');
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't be better here just to await until the proper button is clicked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The await on button click works and reflects the correct UI state. The problem is we can't determine if the network call against the button click succeeds or fails. Doing this should re-issue the network call in retry if previous one fails or takes longer.

Copy link
Contributor

Choose a reason for hiding this comment

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

From what I saw directly in the UI, the call made doesn't include the sort attribute, so calling it for the proper asc value would be enough. I don't get why we are calling asc and then desc, and viceversa.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't get why we are calling asc and then desc, and viceversa

Without toggling the asc/desc, it doesn't re-issue the request:

does-not-send-request-only-one-button.mov

With the toggle it re-issues the request on each retry:

retry-on-test-failure.mov

the call made doesn't include the sort attribute

If you mean the sort attribute in API call, the sort asc/desc request does include the sort param.

sort-order-fleet-installed-network-request

Copy link
Contributor

Choose a reason for hiding this comment

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

If you mean the sort attribute in API call, the sort asc/desc request does include the sort param.

I missed it somehow 😓

With the toggle it re-issues the request on each retry

I think that a weird behaviour, thanks for digging into it

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

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

[✅] x-pack/test_serverless/functional/test_suites/observability/config.ts: 100/100 tests passed.

see run history

@awahab07
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

Copy link
Contributor

@yngrdyn yngrdyn left a comment

Choose a reason for hiding this comment

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

LGTM

@awahab07 awahab07 merged commit cbf7982 into elastic:main Oct 22, 2024
21 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 22, 2024
Fixes elastic#182017

## Summary

The PR tries to address the flakiness from the test.

Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit cbf7982)
@kibanamachine
Copy link
Contributor

💚 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

kibanamachine added a commit that referenced this pull request Oct 22, 2024
… (#197237)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Make sort integration test resilient to network delays.
(#196516)](#196516)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Abdul Wahab
Zahid","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-22T13:11:08Z","message":"Make
sort integration test resilient to network delays. (#196516)\n\nFixes
https://github.com/elastic/kibana/issues/182017\r\n\r\n##
Summary\r\n\r\nThe PR tries to address the flakiness from the
test.\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"cbf7982887decc527455dd8f64477813e8ea2672","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v9.0.0","backport:prev-minor","Team:obs-ux-logs"],"title":"Make
sort integration test resilient to network
delays.","number":196516,"url":"https://github.com/elastic/kibana/pull/196516","mergeCommit":{"message":"Make
sort integration test resilient to network delays. (#196516)\n\nFixes
https://github.com/elastic/kibana/issues/182017\r\n\r\n##
Summary\r\n\r\nThe PR tries to address the flakiness from the
test.\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"cbf7982887decc527455dd8f64477813e8ea2672"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196516","number":196516,"mergeCommit":{"message":"Make
sort integration test resilient to network delays. (#196516)\n\nFixes
https://github.com/elastic/kibana/issues/182017\r\n\r\n##
Summary\r\n\r\nThe PR tries to address the flakiness from the
test.\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"cbf7982887decc527455dd8f64477813e8ea2672"}}]}]
BACKPORT-->

Co-authored-by: Abdul Wahab Zahid <[email protected]>
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) bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-logs Observability Logs User Experience Team v8.17.0 v9.0.0
Projects
None yet
4 participants