Skip to content

[Detection Engine] Unskip some flaky tests, add better failure messages#230318

Merged
rylnd merged 12 commits intoelastic:mainfrom
rylnd:rylnd/unskip_with_better_failures
Aug 11, 2025
Merged

[Detection Engine] Unskip some flaky tests, add better failure messages#230318
rylnd merged 12 commits intoelastic:mainfrom
rylnd:rylnd/unskip_with_better_failures

Conversation

@rylnd
Copy link
Copy Markdown
Contributor

@rylnd rylnd commented Aug 1, 2025

Summary

While the tests affected by this PR are varied, the changes contained here fall under one of two categories:

  1. Unskipping flaky tests
  2. Adding better test assertions (in order to produce more actionable failures later)

Related Issues

Checklist

rylnd added 5 commits August 1, 2025 16:57
This generally has better matchers, and more verbose failure messages.
In this case, that is embodied in the use of `toHaveLength()` instead of
`expect(length).to.eql()`, which will display the array's contents upon
failure.
I've been unable to reproduce these failures locally, and they fit the
pattern caused by our usage of es_archiver, and fixed in
elastic#229457.

Closes elastic#224780, closes elastic#221659.
A single test in this suite had failed (elastic#224699) due to alerts not being
found. While this passes locally, this should give us a better error in
case it fails again.

Closes elastic#224699.
Regardless of why these are failing, this will result in better error
messages when they do.
@rylnd rylnd self-assigned this Aug 1, 2025
@rylnd rylnd added release_note:skip Skip the PR/issue when compiling release notes backport:version Backport to applied version labels v9.1.1 v8.19.1 labels Aug 1, 2025
Similar to the other commits on this branch, we're unskipping tests
while improving the failure messages.

Closes elastic#220943
@rylnd rylnd added the Team:Detection Engine Security Solution Detection Engine Area label Aug 1, 2025
@rylnd rylnd marked this pull request as ready for review August 1, 2025 22:31
@rylnd rylnd requested a review from a team as a code owner August 1, 2025 22:31
@rylnd rylnd requested a review from vitaliidm August 1, 2025 22:31
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/security-detection-engine (Team:Detection Engine)

rylnd added 2 commits August 1, 2025 17:36
* Unskips and closes elastic#202940, closes elastic#202945
* Replaces expect(length).toEqual() with expect().toHaveLength()

// Failing: See https://github.com/elastic/kibana/issues/224699
describe.skip('shard failures', () => {
describe('shard failures', () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we run these tests in flaky test runner before enabling them?

they were failing quite consistently before #224699

rylnd added 2 commits August 5, 2025 13:52
Particularly in the ML tests, which have been flaky recently.
These just started failing in
elastic#230660, and we're again left
with a dead-end investigation due to the unhelpful error messages there.
@kibanamachine
Copy link
Copy Markdown
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#9036

[✅] x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/esql/trial_license_complete_tier/configs/ess.config.ts: 100/100 tests passed.
[❌] x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/machine_learning/trial_license_complete_tier/configs/ess.config.ts: 75/100 tests passed.

see run history

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #42 / Interactive setup APIs - Manual configuration flow should be able to configure with valid authentication code

Metrics [docs]

✅ unchanged

History

cc @rylnd

@kibanamachine
Copy link
Copy Markdown
Contributor

Flaky Test Runner Stats

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

[✅] x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/esql/trial_license_complete_tier/configs/ess.config.ts: 50/50 tests passed.
[✅] x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/machine_learning/trial_license_complete_tier/configs/ess.config.ts: 50/50 tests passed.
[✅] x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/exceptions/workflows/basic_license_essentials_tier/configs/ess.config.ts: 50/50 tests passed.

see run history

@rylnd
Copy link
Copy Markdown
Contributor Author

rylnd commented Aug 7, 2025

Now that #229457 is merged and contained in this PR, the tests seem much more stable, and I obtained a successful (and mostly representative) build yesterday.

Keep in mind that the point of this PR is not to "fix" any tests, but instead to identify, improve, and unskip a group of them that I believe will be made less flakey (or fixed entirely) by #229457.

@rylnd rylnd requested a review from vitaliidm August 7, 2025 20:38
@vitaliidm
Copy link
Copy Markdown
Contributor

Keep in mind that the point of this PR is not to "fix" any tests, but instead to identify, improve, and unskip a group of them that I believe will be made less flakey (or fixed entirely) by #229457.

Yes, I only was concerned about shards tests #224699, since they were failing constantly, unlike the rest of tests, which are flaky.

@kibanamachine
Copy link
Copy Markdown
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @rylnd

rylnd added a commit that referenced this pull request Aug 13, 2025
…messages (#230318) (#231523)

# Backport

This will backport the following commits from `main` to `9.1`:
- [[Detection Engine] Unskip some flaky tests, add better failure
messages (#230318)](#230318)

<!--- Backport version: 10.0.1 -->

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

<!--BACKPORT [{"author":{"name":"Ryland
Herrick","email":"ryalnd@gmail.com"},"sourceCommit":{"committedDate":"2025-08-11T16:40:18Z","message":"[Detection
Engine] Unskip some flaky tests, add better failure messages
(#230318)\n\n## Summary\n\nWhile the tests affected by this PR are
varied, the changes contained\nhere fall under one of two
categories:\n\n1. Unskipping flaky tests\n2. Adding better test
assertions (in order to produce more actionable\nfailures later)\n\n###
Related Issues\n* Closes #224699\n* Closes #224780\n* Closes #220943\n*
Closes #202940\n* Closes #202945\n\n\n### Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"0a3e7bb41a92b25ca6ae25bef76e53ef634d244b","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport
missing","Team:Detection
Engine","backport:version","v9.2.0","v9.1.1","v8.19.1"],"title":"[Detection
Engine] Unskip some flaky tests, add better failure
messages","number":230318,"url":"https://github.com/elastic/kibana/pull/230318","mergeCommit":{"message":"[Detection
Engine] Unskip some flaky tests, add better failure messages
(#230318)\n\n## Summary\n\nWhile the tests affected by this PR are
varied, the changes contained\nhere fall under one of two
categories:\n\n1. Unskipping flaky tests\n2. Adding better test
assertions (in order to produce more actionable\nfailures later)\n\n###
Related Issues\n* Closes #224699\n* Closes #224780\n* Closes #220943\n*
Closes #202940\n* Closes #202945\n\n\n### Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"0a3e7bb41a92b25ca6ae25bef76e53ef634d244b"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/230318","number":230318,"mergeCommit":{"message":"[Detection
Engine] Unskip some flaky tests, add better failure messages
(#230318)\n\n## Summary\n\nWhile the tests affected by this PR are
varied, the changes contained\nhere fall under one of two
categories:\n\n1. Unskipping flaky tests\n2. Adding better test
assertions (in order to produce more actionable\nfailures later)\n\n###
Related Issues\n* Closes #224699\n* Closes #224780\n* Closes #220943\n*
Closes #202940\n* Closes #202945\n\n\n### Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"0a3e7bb41a92b25ca6ae25bef76e53ef634d244b"}},{"branch":"9.1","label":"v9.1.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
rylnd added a commit that referenced this pull request Aug 13, 2025
… messages (#230318) (#231526)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[Detection Engine] Unskip some flaky tests, add better failure
messages (#230318)](#230318)

<!--- Backport version: 10.0.1 -->

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

<!--BACKPORT [{"author":{"name":"Ryland
Herrick","email":"ryalnd@gmail.com"},"sourceCommit":{"committedDate":"2025-08-11T16:40:18Z","message":"[Detection
Engine] Unskip some flaky tests, add better failure messages
(#230318)\n\n## Summary\n\nWhile the tests affected by this PR are
varied, the changes contained\nhere fall under one of two
categories:\n\n1. Unskipping flaky tests\n2. Adding better test
assertions (in order to produce more actionable\nfailures later)\n\n###
Related Issues\n* Closes #224699\n* Closes #224780\n* Closes #220943\n*
Closes #202940\n* Closes #202945\n\n\n### Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"0a3e7bb41a92b25ca6ae25bef76e53ef634d244b","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport
missing","Team:Detection
Engine","backport:version","v9.2.0","v9.1.1","v8.19.1"],"title":"[Detection
Engine] Unskip some flaky tests, add better failure
messages","number":230318,"url":"https://github.com/elastic/kibana/pull/230318","mergeCommit":{"message":"[Detection
Engine] Unskip some flaky tests, add better failure messages
(#230318)\n\n## Summary\n\nWhile the tests affected by this PR are
varied, the changes contained\nhere fall under one of two
categories:\n\n1. Unskipping flaky tests\n2. Adding better test
assertions (in order to produce more actionable\nfailures later)\n\n###
Related Issues\n* Closes #224699\n* Closes #224780\n* Closes #220943\n*
Closes #202940\n* Closes #202945\n\n\n### Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"0a3e7bb41a92b25ca6ae25bef76e53ef634d244b"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/230318","number":230318,"mergeCommit":{"message":"[Detection
Engine] Unskip some flaky tests, add better failure messages
(#230318)\n\n## Summary\n\nWhile the tests affected by this PR are
varied, the changes contained\nhere fall under one of two
categories:\n\n1. Unskipping flaky tests\n2. Adding better test
assertions (in order to produce more actionable\nfailures later)\n\n###
Related Issues\n* Closes #224699\n* Closes #224780\n* Closes #220943\n*
Closes #202940\n* Closes #202945\n\n\n### Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"0a3e7bb41a92b25ca6ae25bef76e53ef634d244b"}},{"branch":"9.1","label":"v9.1.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
@kibanamachine kibanamachine added v8.19.3 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Aug 13, 2025
This was referenced Aug 15, 2025
This was referenced Aug 18, 2025
NicholasPeretti pushed a commit to NicholasPeretti/kibana that referenced this pull request Aug 18, 2025
…es (elastic#230318)

## Summary

While the tests affected by this PR are varied, the changes contained
here fall under one of two categories:

1. Unskipping flaky tests
2. Adding better test assertions (in order to produce more actionable
failures later)

### Related Issues
* Closes elastic#224699
* Closes elastic#224780
* Closes elastic#220943
* Closes elastic#202940
* Closes elastic#202945


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
qn895 pushed a commit to qn895/kibana that referenced this pull request Aug 26, 2025
…es (elastic#230318)

## Summary

While the tests affected by this PR are varied, the changes contained
here fall under one of two categories:

1. Unskipping flaky tests
2. Adding better test assertions (in order to produce more actionable
failures later)

### Related Issues
* Closes elastic#224699
* Closes elastic#224780
* Closes elastic#220943
* Closes elastic#202940
* Closes elastic#202945


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Detection Engine Security Solution Detection Engine Area v8.19.1 v8.19.3 v9.1.1 v9.1.3 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failing test: Detection Engine - EQL Rule Execution Logic Integration Tests - ESS Env - Trial License.x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/eql/trial_license_complete_tier/eql_alert_suppression·ts - EQL execution logic API @ess @serverless Alert Suppression for EQL rules @skipInServerless sequence queries with suppression duration does not suppress alerts outside of duration when query with 3 sequences Failing test: Detection Engine - General Execution Logic Integration Tests - ESS Env - Basic License.x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/general_logic/basic_license_essentials_tier/timestamps·ts - Rule execution logic API - Basic License/Essentials Tier @ess @serverless @serverlessQA timestamp tests alerts generated from events with timestamp override field KQL should generate 2 alerts when timestamp override does not exist Failing test: Detection Engine - ESQL Rule Execution Logic Integration Tests - ESS Env - Trial License.x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/esql/trial_license_complete_tier/esql·ts - ESQL execution logic API @ess @serverless ES|QL rule type shard failures should handle shard failures and include warning in logs for query that is not aggregating Failing test: Detection Engine - EQL Rule Execution Logic Integration Tests - ESS Env - Trial License.x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/eql/trial_license_complete_tier/eql·ts - EQL execution logic API @ess @serverless @serverlessQA EQL type rules uses the provided timestamp_field Failing test: Detection Engine - EQL Rule Execution Logic Integration Tests - ESS Env - Trial License.x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/eql/trial_license_complete_tier/eql_alert_suppression·ts - EQL execution logic API @ess @serverless Alert Suppression for EQL rules @skipInServerless sequence queries with suppression duration does not suppress alerts outside of duration

5 participants