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

Add error handling/retry logic for search source alert tests #196443

Merged

Conversation

lukasolson
Copy link
Member

@lukasolson lukasolson commented Oct 15, 2024

Summary

Resolves #193842.

Adds error handling & retry logic for search source alerts that are causing failures on MKI.

Checklist

@lukasolson lukasolson added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Oct 15, 2024
@lukasolson lukasolson self-assigned this Oct 15, 2024
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

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

[✅] x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/config.ts: 25/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/observability/common_configs/config.group6.ts: 25/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group6.ts: 25/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/security/common_configs/config.group6.ts: 25/25 tests passed.

see run history

@lukasolson
Copy link
Member Author

@wayneseymour Do you mind taking a quick look at this and see if it's along the lines of what you were thinking?

@wayneseymour wayneseymour self-requested a review October 22, 2024 10:54
Copy link
Member

@wayneseymour wayneseymour left a comment

Choose a reason for hiding this comment

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

@lukasolson I would strongly recommend running the serverless tests through MKI, from your local

One major reason is to ensure the default timeout durations encoded in x-pack/test_serverless/functional/test_suites/common/discover_ml_uptime/discover/search_source_alert.ts, actually work for real serverless as well.
PR CI only runs fake-serverless.
Running against real serverless (mki) will give you useful feedback.

  • network access is typically slower, etc

Ran From my Local

Ran the stateful test, no issues.
Ran the serverless test against a real serverless oblt project (mki), I am seeing errors:

└-: Discover alerting
  └-> "before all" hook: beforeTestSuite.trigger in "Discover alerting"
  └-> "before all" hook in "Discover alerting"
  └-: Search source Alert
    └-> "before all" hook: beforeTestSuite.trigger for "should create an alert when there is no data view"
    └-> "before all" hook for "should create an alert when there is no data view"
            │ERROR timeout of 60000ms exceeded
            │      Waiting 15000 ms before the next attempt
    └-> should create an alert when there is no data view
      └-> "before each" hook: global before each for "should create an alert when there is no data view"
      └- ✓ pass  (23.1s)
    └-> should show time field validation error
      └-> "before each" hook: global before each for "should show time field validation error"
      └- ✓ pass  (14.8s)
    └-> should navigate to alert results via view in app link
      └-> "before each" hook: global before each for "should navigate to alert results via view in app link"
      └- ✓ pass  (40.0s)
    └-> should navigate to alert results via link provided in notification
      └-> "before each" hook: global before each for "should navigate to alert results via link provided in notification"
      └- ✓ pass  (37.8s)
    └-> should display prev rule state after params update on clicking prev generated link
      └-> "before each" hook: global before each for "should display prev rule state after params update on clicking prev generated link"
      └- ✖ fail: Discover alerting Search source Alert should display prev rule state after params update on clicking prev generated link
      │      Error: retry.try reached timeout 120000 ms
      │ TimeoutError: Waiting for element to be located By(css selector, [data-test-subj="saveEditedRuleButton"])
      │ Wait timed out after 10054ms
      │     at /Users/trezworkbox/dev/main/node_modules/selenium-webdriver/lib/webdriver.js:923:22
      │     at processTicksAndRejections (node:internal/process/task_queues:95:5)
      │       at onFailure (retry_for_success.ts:18:9)
      │       at retryForSuccess (retry_for_success.ts:85:7)
      │       at Proxy.try (retry.ts:52:12)
      │       at Proxy.clickByCssSelector (find.ts:420:5)
      │       at Proxy.click (test_subjects.ts:169:5)
      │       at Context.<anonymous> (search_source_alert.ts:509:7)
      │       at Object.apply (wrap_function.js:74:16)
      │
      │
    └-> should display actual state after rule params update on clicking viewInApp link
      └-> "before each" hook: global before each for "should display actual state after rule params update on clicking viewInApp link"
      └- ✖ fail: Discover alerting Search source Alert should display actual state after rule params update on clicking viewInApp link
      │      Error: expected '' to equal 'message:msg-1'
      │       at Assertion.assert (expect.js:100:11)
      │       at Assertion.apply (expect.js:227:8)
      │       at Function.equal (expect.js:531:15)
      │       at checkUpdatedRuleParamsState (search_source_alert.ts:343:31)
      │       at processTicksAndRejections (node:internal/process/task_queues:95:5)
      │       at Context.<anonymous> (search_source_alert.ts:522:7)
      │       at Object.apply (wrap_function.js:74:16)
      │
      │
    └-> should display prev data view state after update on clicking prev generated link
      └-> "before each" hook: global before each for "should display prev data view state after update on clicking prev generated link"
     
     ....
     

I recommend running it against search and security projects as well.

@lukasolson lukasolson marked this pull request as ready for review November 20, 2024 22:17
@lukasolson lukasolson requested a review from a team as a code owner November 20, 2024 22:17
@elasticmachine
Copy link
Contributor

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

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

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

[✅] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group6.ts: 25/25 tests passed.

see run history

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.

Code changes LGTM, but it looks like a different test is failing now in this PR. Possibly related to #198987.

@lukasolson
Copy link
Member Author

Code changes LGTM, but it looks like a different test is failing now in this PR. Possibly related to #198987.

I've skipped this test for now, looks to be an actual bug. I've filed an issue here: #203045

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.

LGTM, thanks for fixing them and adding the issue!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing the conditionals for buttons and inputs are for O11y projects? We should probably consider splitting these out since they could diverge more, but let's get them unskipped for now.

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 right, and yes I totally agree. There was already conditionals like these in these tests but I agree, it's probably time now to split these.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

cc @lukasolson

@lukasolson lukasolson merged commit 5acba96 into elastic:main Dec 10, 2024
9 checks passed
@davismcphee
Copy link
Contributor

@lukasolson Did we only want this merged for 9.0 or should it also be backported to an 8.x branch?

@lukasolson
Copy link
Member Author

💚 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

lukasolson added a commit to lukasolson/kibana that referenced this pull request Dec 11, 2024
…#196443)

## Summary

Resolves elastic#193842.

Adds error handling & retry logic for search source alerts that are
causing failures on MKI.

### Checklist

- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

(cherry picked from commit 5acba96)
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
…#196443)

## Summary

Resolves elastic#193842.

Adds error handling & retry logic for search source alerts that are
causing failures on MKI.

### Checklist

- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
stephmilovic pushed a commit that referenced this pull request Dec 19, 2024
…196443) (#203878)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Add error handling/retry logic for search source alert tests
(#196443)](#196443)

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

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

<!--BACKPORT [{"author":{"name":"Lukas
Olson","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-10T20:27:36Z","message":"Add
error handling/retry logic for search source alert tests (#196443)\n\n##
Summary\r\n\r\nResolves
https://github.com/elastic/kibana/issues/193842.\r\n\r\nAdds error
handling & retry logic for search source alerts that are\r\ncausing
failures on MKI.\r\n\r\n### Checklist\r\n\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"5acba9678aef16b460bb7b578dcdf4ce10a012a4","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:skip","v9.0.0","Team:DataDiscovery"],"number":196443,"url":"https://github.com/elastic/kibana/pull/196443","mergeCommit":{"message":"Add
error handling/retry logic for search source alert tests (#196443)\n\n##
Summary\r\n\r\nResolves
https://github.com/elastic/kibana/issues/193842.\r\n\r\nAdds error
handling & retry logic for search source alerts that are\r\ncausing
failures on MKI.\r\n\r\n### Checklist\r\n\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"5acba9678aef16b460bb7b578dcdf4ce10a012a4"}},"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/196443","number":196443,"mergeCommit":{"message":"Add
error handling/retry logic for search source alert tests (#196443)\n\n##
Summary\r\n\r\nResolves
https://github.com/elastic/kibana/issues/193842.\r\n\r\nAdds error
handling & retry logic for search source alerts that are\r\ncausing
failures on MKI.\r\n\r\n### Checklist\r\n\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"5acba9678aef16b460bb7b578dcdf4ce10a012a4"}}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting 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.

[FTR][Serverless] Err handling needed in search_source_alert.ts
5 participants