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

[Security Solution] Fix flaky tests: Rule Management area #161507

Closed
18 tasks done
Tracked by #153633 ...
banderror opened this issue Jul 8, 2023 · 3 comments
Closed
18 tasks done
Tracked by #153633 ...

[Security Solution] Fix flaky tests: Rule Management area #161507

banderror opened this issue Jul 8, 2023 · 3 comments
Assignees
Labels
8.10 candidate Meta Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. technical debt Improvement of the software architecture and operational architecture test_ui_functional test test-api-integration v8.10.0

Comments

@banderror
Copy link
Contributor

banderror commented Jul 8, 2023

Epic: #153633

Summary

Fix flaky tests and unskip tests that belong to the Rule Management team.

https://github.com/elastic/kibana/issues?q=is%3Aopen+is%3Aissue+label%3Afailed-test%2Cskipped-test+label%3A%22Team%3ADetection+Rule+Management%22+

Sub-tasks

Prebuilt Rules

  1. 8.10 candidate Feature:Prebuilt Detection Rules Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    jpdjere
  2. 8.10 candidate Feature:Prebuilt Detection Rules Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    jpdjere
  3. 8.10 candidate Feature:Prebuilt Detection Rules Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    jpdjere
  4. 8.10 candidate Feature:Prebuilt Detection Rules Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    jpdjere
  5. 8.10 candidate Feature:Rule Management Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    jpdjere maximpn
  6. 8.10 candidate Feature:Prebuilt Detection Rules Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    jpdjere maximpn
  7. 8.10 candidate Feature:Prebuilt Detection Rules Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    jpdjere maximpn
  8. 8.10 candidate Feature:Prebuilt Detection Rules Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    jpdjere maximpn

Rule Management

  1. 8.10 candidate Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    maximpn
  2. 8.10 candidate Feature:Rule Management Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp impact:high skipped-test test test_ui_functional
    maximpn
  3. 8.10 candidate Feature:Rule Management Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp impact:high skipped-test test test_ui_functional
    jpdjere
  4. 8.10 candidate Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test test test-failure-flaky
    maximpn
  5. 8.10 candidate Feature:Rule Management Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    maximpn
  6. 8.12 candidate Feature:Rule Management Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test
    jpdjere

Related Integrations

  1. 8.10 candidate Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test test test-failure-flaky
    maximpn
  2. 8.10 candidate Feature:Related Integrations Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp impact:high skipped-test test test_ui_functional
    maximpn

Rule Details

  1. 8.10 candidate Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test test
    jpdjere
  2. 8.10 candidate Team: SecuritySolution Team:Detection Rule Management Team:Detections and Resp failed-test test
    e40pud
@botelastic botelastic bot added the needs-team Issues missing a team label label Jul 8, 2023
@banderror banderror added test Meta test_ui_functional technical debt Improvement of the software architecture and operational architecture test-api-integration Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team 8.10 candidate and removed needs-team Issues missing a team label labels Jul 8, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

maximpn added a commit that referenced this issue Aug 23, 2023
**Relates to: #161507
**Fixes: #163704
**Fixes: #163182
**Fixes: #163558
**Fixes: #163974
**Fixes: #153914
**Fixes: #164079
**Fixes: #164279

## Summary

While working on fixing Rules Management flaky tests I've noticed similar fails in different tests. This PR addresses common pitfalls to reduce a number of reasons causing e2e tests flakiness and as a result reduce a number of flaky tests.

## Details

The common reasons causing e2e tests flakiness for the rules tables are

- Auto-refresh

Auto-refresh functionality is enabled by default and the table gets auto-refreshed every 60 seconds. If a test takes more than 60 seconds the table fetches updated rules. Having rules enabled by default and sorted by `Enabled` column makes the sorting order undetermined and as rules get updated due to execution ES return them in undetermined order. This update can happen between commands working on the same element and indexed access like `eq()` would access different elements. 

- Missing selectors

Some tests or helper functions have expectations for an element absence like `should('not.exist')` without checking an element is visible before like `should('be.visible')`. This way a referenced element may disappear from the codebase after refactoring and the expectation still fulfils.

- Checking for `should('not.visible')` while an element is removed from the DOM

It most applicable to popovers as it first animates to be hidden and then removed from the DOM. Cypress first need to find an element to check its visibility. Replacing `should('not.visible')` with `should('not.exist')` and taking into concern from the account previous bullet fixes the problem.

- Modifying ES data without refreshing (`_delete_by_query` in particular)

Due to high performance ES architecture data isn't updated instantly. Having such behavior in tests leads to undetermined state depending on a number of environmental factors. As UI doesn't always auto-refreshes to fetch the recent updates in short period of time test fail. `_delete_by_query` may take some time to update the data but it doesn't support `refresh=wait_for` as it stated in [docs](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete-by-query.html#_refreshing_shards). Adding `?refresh=true` or just `?refresh` to `_delete_by_query` ES request urls fixes the problem.

### What was done to address mentioned reasons above?

- Auto-refresh functionality disabled in tests where it's not necessary.
- `enabled: false` field was added to rule creators to have disabled rules as the majority of tests don't need enabled rules.
- `waitForRulesTableToBeLoaded` was removed and replaced with `expectManagementTableRules` at some tests.
- `should('not.visible')` replaced with `should('not.exist')` in `deleteRuleFromDetailsPage()`
- `?refresh` added to `_delete_by_query` ES data update requests

The other changes get rid of global constants and improve readability.

## Flaky test runs

[All Cypress tests under `detection_response` folder (100 runs)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2920) (value lists export is flaky but it's out of scope of this PR)
maximpn added a commit that referenced this issue Aug 25, 2023
**Relates to:** #161507

## Summary

This PR fixes **bulk_edit_rules.cy.ts** flakiness found while running Security Solution Cypress tests in Flaky test runner ([run 1](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2969) and [run 2](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2970)).

## Flaky test runner

[bulk_edit_rules.cy.ts 300 runs](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2975) 🟢
maximpn added a commit to maximpn/kibana that referenced this issue Aug 25, 2023
…4099)

**Relates to: elastic#161507
**Fixes: elastic#163704
**Fixes: elastic#163182
**Fixes: elastic#163558
**Fixes: elastic#163974
**Fixes: elastic#153914
**Fixes: elastic#164079
**Fixes: elastic#164279

## Summary

While working on fixing Rules Management flaky tests I've noticed similar fails in different tests. This PR addresses common pitfalls to reduce a number of reasons causing e2e tests flakiness and as a result reduce a number of flaky tests.

## Details

The common reasons causing e2e tests flakiness for the rules tables are

- Auto-refresh

Auto-refresh functionality is enabled by default and the table gets auto-refreshed every 60 seconds. If a test takes more than 60 seconds the table fetches updated rules. Having rules enabled by default and sorted by `Enabled` column makes the sorting order undetermined and as rules get updated due to execution ES return them in undetermined order. This update can happen between commands working on the same element and indexed access like `eq()` would access different elements.

- Missing selectors

Some tests or helper functions have expectations for an element absence like `should('not.exist')` without checking an element is visible before like `should('be.visible')`. This way a referenced element may disappear from the codebase after refactoring and the expectation still fulfils.

- Checking for `should('not.visible')` while an element is removed from the DOM

It most applicable to popovers as it first animates to be hidden and then removed from the DOM. Cypress first need to find an element to check its visibility. Replacing `should('not.visible')` with `should('not.exist')` and taking into concern from the account previous bullet fixes the problem.

- Modifying ES data without refreshing (`_delete_by_query` in particular)

Due to high performance ES architecture data isn't updated instantly. Having such behavior in tests leads to undetermined state depending on a number of environmental factors. As UI doesn't always auto-refreshes to fetch the recent updates in short period of time test fail. `_delete_by_query` may take some time to update the data but it doesn't support `refresh=wait_for` as it stated in [docs](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete-by-query.html#_refreshing_shards). Adding `?refresh=true` or just `?refresh` to `_delete_by_query` ES request urls fixes the problem.

### What was done to address mentioned reasons above?

- Auto-refresh functionality disabled in tests where it's not necessary.
- `enabled: false` field was added to rule creators to have disabled rules as the majority of tests don't need enabled rules.
- `waitForRulesTableToBeLoaded` was removed and replaced with `expectManagementTableRules` at some tests.
- `should('not.visible')` replaced with `should('not.exist')` in `deleteRuleFromDetailsPage()`
- `?refresh` added to `_delete_by_query` ES data update requests

The other changes get rid of global constants and improve readability.

## Flaky test runs

[All Cypress tests under `detection_response` folder (100 runs)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2920) (value lists export is flaky but it's out of scope of this PR)

(cherry picked from commit 40df521)

# Conflicts:
#	x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_creation/custom_query_rule.cy.ts
maximpn referenced this issue Aug 26, 2023
…4099) (#164903)

# Backport

This will backport the following commits from `main` to `8.10`:
- [[Security Solution] Reduce Rules Management e2e flakiness
(#164099)](#164099)

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

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

<!--BACKPORT [{"author":{"name":"Maxim
Palenov","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-08-23T14:58:04Z","message":"[Security
Solution] Reduce Rules Management e2e flakiness (#164099)\n\n**Relates
to: https://github.com/elastic/kibana/issues/161507**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163704**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163182**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163558**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163974**\r\n**Fixes:
https://github.com/elastic/kibana/issues/153914**\r\n**Fixes:
https://github.com/elastic/kibana/issues/164079**\r\n**Fixes:
https://github.com/elastic/kibana/issues/164279**\r\n\r\n##
Summary\r\n\r\nWhile working on fixing Rules Management flaky tests I've
noticed similar fails in different tests. This PR addresses common
pitfalls to reduce a number of reasons causing e2e tests flakiness and
as a result reduce a number of flaky tests.\r\n\r\n## Details\r\n\r\nThe
common reasons causing e2e tests flakiness for the rules tables
are\r\n\r\n- Auto-refresh\r\n\r\nAuto-refresh functionality is enabled
by default and the table gets auto-refreshed every 60 seconds. If a test
takes more than 60 seconds the table fetches updated rules. Having rules
enabled by default and sorted by `Enabled` column makes the sorting
order undetermined and as rules get updated due to execution ES return
them in undetermined order. This update can happen between commands
working on the same element and indexed access like `eq()` would access
different elements. \r\n\r\n- Missing selectors\r\n\r\nSome tests or
helper functions have expectations for an element absence like
`should('not.exist')` without checking an element is visible before like
`should('be.visible')`. This way a referenced element may disappear from
the codebase after refactoring and the expectation still
fulfils.\r\n\r\n- Checking for `should('not.visible')` while an element
is removed from the DOM\r\n\r\nIt most applicable to popovers as it
first animates to be hidden and then removed from the DOM. Cypress first
need to find an element to check its visibility. Replacing
`should('not.visible')` with `should('not.exist')` and taking into
concern from the account previous bullet fixes the problem.\r\n\r\n-
Modifying ES data without refreshing (`_delete_by_query` in
particular)\r\n\r\nDue to high performance ES architecture data isn't
updated instantly. Having such behavior in tests leads to undetermined
state depending on a number of environmental factors. As UI doesn't
always auto-refreshes to fetch the recent updates in short period of
time test fail. `_delete_by_query` may take some time to update the data
but it doesn't support `refresh=wait_for` as it stated in
[docs](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete-by-query.html#_refreshing_shards).
Adding `?refresh=true` or just `?refresh` to `_delete_by_query` ES
request urls fixes the problem.\r\n\r\n### What was done to address
mentioned reasons above?\r\n\r\n- Auto-refresh functionality disabled in
tests where it's not necessary.\r\n- `enabled: false` field was added to
rule creators to have disabled rules as the majority of tests don't need
enabled rules.\r\n- `waitForRulesTableToBeLoaded` was removed and
replaced with `expectManagementTableRules` at some tests.\r\n-
`should('not.visible')` replaced with `should('not.exist')` in
`deleteRuleFromDetailsPage()`\r\n- `?refresh` added to
`_delete_by_query` ES data update requests\r\n\r\nThe other changes get
rid of global constants and improve readability.\r\n\r\n## Flaky test
runs\r\n\r\n[All Cypress tests under `detection_response` folder (100
runs)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2920)
(value lists export is flaky but it's out of scope of this
PR)","sha":"40df5219ea7165e89623afcc22bb0dbc3cc19152","branchLabelMapping":{"^v8.11.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["test","release_note:skip","Team:Detections
and Resp","Team: SecuritySolution","Feature:Rule
Management","Team:Detection Rule
Management","v8.10.0","v8.11.0"],"number":164099,"url":"https://github.com/elastic/kibana/pull/164099","mergeCommit":{"message":"[Security
Solution] Reduce Rules Management e2e flakiness (#164099)\n\n**Relates
to: https://github.com/elastic/kibana/issues/161507**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163704**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163182**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163558**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163974**\r\n**Fixes:
https://github.com/elastic/kibana/issues/153914**\r\n**Fixes:
https://github.com/elastic/kibana/issues/164079**\r\n**Fixes:
https://github.com/elastic/kibana/issues/164279**\r\n\r\n##
Summary\r\n\r\nWhile working on fixing Rules Management flaky tests I've
noticed similar fails in different tests. This PR addresses common
pitfalls to reduce a number of reasons causing e2e tests flakiness and
as a result reduce a number of flaky tests.\r\n\r\n## Details\r\n\r\nThe
common reasons causing e2e tests flakiness for the rules tables
are\r\n\r\n- Auto-refresh\r\n\r\nAuto-refresh functionality is enabled
by default and the table gets auto-refreshed every 60 seconds. If a test
takes more than 60 seconds the table fetches updated rules. Having rules
enabled by default and sorted by `Enabled` column makes the sorting
order undetermined and as rules get updated due to execution ES return
them in undetermined order. This update can happen between commands
working on the same element and indexed access like `eq()` would access
different elements. \r\n\r\n- Missing selectors\r\n\r\nSome tests or
helper functions have expectations for an element absence like
`should('not.exist')` without checking an element is visible before like
`should('be.visible')`. This way a referenced element may disappear from
the codebase after refactoring and the expectation still
fulfils.\r\n\r\n- Checking for `should('not.visible')` while an element
is removed from the DOM\r\n\r\nIt most applicable to popovers as it
first animates to be hidden and then removed from the DOM. Cypress first
need to find an element to check its visibility. Replacing
`should('not.visible')` with `should('not.exist')` and taking into
concern from the account previous bullet fixes the problem.\r\n\r\n-
Modifying ES data without refreshing (`_delete_by_query` in
particular)\r\n\r\nDue to high performance ES architecture data isn't
updated instantly. Having such behavior in tests leads to undetermined
state depending on a number of environmental factors. As UI doesn't
always auto-refreshes to fetch the recent updates in short period of
time test fail. `_delete_by_query` may take some time to update the data
but it doesn't support `refresh=wait_for` as it stated in
[docs](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete-by-query.html#_refreshing_shards).
Adding `?refresh=true` or just `?refresh` to `_delete_by_query` ES
request urls fixes the problem.\r\n\r\n### What was done to address
mentioned reasons above?\r\n\r\n- Auto-refresh functionality disabled in
tests where it's not necessary.\r\n- `enabled: false` field was added to
rule creators to have disabled rules as the majority of tests don't need
enabled rules.\r\n- `waitForRulesTableToBeLoaded` was removed and
replaced with `expectManagementTableRules` at some tests.\r\n-
`should('not.visible')` replaced with `should('not.exist')` in
`deleteRuleFromDetailsPage()`\r\n- `?refresh` added to
`_delete_by_query` ES data update requests\r\n\r\nThe other changes get
rid of global constants and improve readability.\r\n\r\n## Flaky test
runs\r\n\r\n[All Cypress tests under `detection_response` folder (100
runs)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2920)
(value lists export is flaky but it's out of scope of this
PR)","sha":"40df5219ea7165e89623afcc22bb0dbc3cc19152"}},"sourceBranch":"main","suggestedTargetBranches":["8.10"],"targetPullRequestStates":[{"branch":"8.10","label":"v8.10.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.11.0","labelRegex":"^v8.11.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/164099","number":164099,"mergeCommit":{"message":"[Security
Solution] Reduce Rules Management e2e flakiness (#164099)\n\n**Relates
to: https://github.com/elastic/kibana/issues/161507**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163704**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163182**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163558**\r\n**Fixes:
https://github.com/elastic/kibana/issues/163974**\r\n**Fixes:
https://github.com/elastic/kibana/issues/153914**\r\n**Fixes:
https://github.com/elastic/kibana/issues/164079**\r\n**Fixes:
https://github.com/elastic/kibana/issues/164279**\r\n\r\n##
Summary\r\n\r\nWhile working on fixing Rules Management flaky tests I've
noticed similar fails in different tests. This PR addresses common
pitfalls to reduce a number of reasons causing e2e tests flakiness and
as a result reduce a number of flaky tests.\r\n\r\n## Details\r\n\r\nThe
common reasons causing e2e tests flakiness for the rules tables
are\r\n\r\n- Auto-refresh\r\n\r\nAuto-refresh functionality is enabled
by default and the table gets auto-refreshed every 60 seconds. If a test
takes more than 60 seconds the table fetches updated rules. Having rules
enabled by default and sorted by `Enabled` column makes the sorting
order undetermined and as rules get updated due to execution ES return
them in undetermined order. This update can happen between commands
working on the same element and indexed access like `eq()` would access
different elements. \r\n\r\n- Missing selectors\r\n\r\nSome tests or
helper functions have expectations for an element absence like
`should('not.exist')` without checking an element is visible before like
`should('be.visible')`. This way a referenced element may disappear from
the codebase after refactoring and the expectation still
fulfils.\r\n\r\n- Checking for `should('not.visible')` while an element
is removed from the DOM\r\n\r\nIt most applicable to popovers as it
first animates to be hidden and then removed from the DOM. Cypress first
need to find an element to check its visibility. Replacing
`should('not.visible')` with `should('not.exist')` and taking into
concern from the account previous bullet fixes the problem.\r\n\r\n-
Modifying ES data without refreshing (`_delete_by_query` in
particular)\r\n\r\nDue to high performance ES architecture data isn't
updated instantly. Having such behavior in tests leads to undetermined
state depending on a number of environmental factors. As UI doesn't
always auto-refreshes to fetch the recent updates in short period of
time test fail. `_delete_by_query` may take some time to update the data
but it doesn't support `refresh=wait_for` as it stated in
[docs](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete-by-query.html#_refreshing_shards).
Adding `?refresh=true` or just `?refresh` to `_delete_by_query` ES
request urls fixes the problem.\r\n\r\n### What was done to address
mentioned reasons above?\r\n\r\n- Auto-refresh functionality disabled in
tests where it's not necessary.\r\n- `enabled: false` field was added to
rule creators to have disabled rules as the majority of tests don't need
enabled rules.\r\n- `waitForRulesTableToBeLoaded` was removed and
replaced with `expectManagementTableRules` at some tests.\r\n-
`should('not.visible')` replaced with `should('not.exist')` in
`deleteRuleFromDetailsPage()`\r\n- `?refresh` added to
`_delete_by_query` ES data update requests\r\n\r\nThe other changes get
rid of global constants and improve readability.\r\n\r\n## Flaky test
runs\r\n\r\n[All Cypress tests under `detection_response` folder (100
runs)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2920)
(value lists export is flaky but it's out of scope of this
PR)","sha":"40df5219ea7165e89623afcc22bb0dbc3cc19152"}}]}] BACKPORT-->
maximpn added a commit to maximpn/kibana that referenced this issue Aug 26, 2023
**Relates to:** elastic#161507

## Summary

This PR fixes **bulk_edit_rules.cy.ts** flakiness found while running Security Solution Cypress tests in Flaky test runner ([run 1](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2969) and [run 2](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2970)).

## Flaky test runner

[bulk_edit_rules.cy.ts 300 runs](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2975) 🟢

(cherry picked from commit 6ab3aa3)
maximpn referenced this issue Aug 26, 2023
# Backport

This will backport the following commits from `main` to `8.10`:
- [[Security Solution] Fix bulk edit rules tests
(#164692)](#164692)

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

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

<!--BACKPORT [{"author":{"name":"Maxim
Palenov","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-08-25T13:26:47Z","message":"[Security
Solution] Fix bulk edit rules tests (#164692)\n\n**Relates to:**
https://github.com/elastic/kibana/issues/161507\r\n\r\n##
Summary\r\n\r\nThis PR fixes **bulk_edit_rules.cy.ts** flakiness found
while running Security Solution Cypress tests in Flaky test runner ([run
1](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2969)
and [run
2](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2970)).\r\n\r\n##
Flaky test runner\r\n\r\n[bulk_edit_rules.cy.ts 300
runs](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2975)
🟢","sha":"6ab3aa3da1a3d1ed37b711efb9891cae43c0da39","branchLabelMapping":{"^v8.11.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["test","release_note:skip","Team:Detections
and Resp","Team: SecuritySolution","Feature:Rule
Management","Team:Detection Rule
Management","v8.10.0","v8.11.0"],"number":164692,"url":"https://github.com/elastic/kibana/pull/164692","mergeCommit":{"message":"[Security
Solution] Fix bulk edit rules tests (#164692)\n\n**Relates to:**
https://github.com/elastic/kibana/issues/161507\r\n\r\n##
Summary\r\n\r\nThis PR fixes **bulk_edit_rules.cy.ts** flakiness found
while running Security Solution Cypress tests in Flaky test runner ([run
1](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2969)
and [run
2](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2970)).\r\n\r\n##
Flaky test runner\r\n\r\n[bulk_edit_rules.cy.ts 300
runs](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2975)
🟢","sha":"6ab3aa3da1a3d1ed37b711efb9891cae43c0da39"}},"sourceBranch":"main","suggestedTargetBranches":["8.10"],"targetPullRequestStates":[{"branch":"8.10","label":"v8.10.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.11.0","labelRegex":"^v8.11.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/164692","number":164692,"mergeCommit":{"message":"[Security
Solution] Fix bulk edit rules tests (#164692)\n\n**Relates to:**
https://github.com/elastic/kibana/issues/161507\r\n\r\n##
Summary\r\n\r\nThis PR fixes **bulk_edit_rules.cy.ts** flakiness found
while running Security Solution Cypress tests in Flaky test runner ([run
1](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2969)
and [run
2](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2970)).\r\n\r\n##
Flaky test runner\r\n\r\n[bulk_edit_rules.cy.ts 300
runs](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2975)
🟢","sha":"6ab3aa3da1a3d1ed37b711efb9891cae43c0da39"}}]}] BACKPORT-->

Co-authored-by: Kibana Machine <[email protected]>
jpdjere added a commit that referenced this issue Aug 28, 2023
…s` - Deletes and recovers more than one rule (#164694)

Relates to: #161507

## Summary

- Solves flakiness in following test:
- Filename:
`x-pack/test/security_solution_cypress/cypress/e2e/detection_response/prebuilt_rules/prebuilt_rules_management.cy.ts`
- Test name: **Prebuilt rules Actions with prebuilt rules Rules table
Deletes and recovers more than one rule**

- Test was failing because of already observed issue of `autoRefresh`
taking place while the rule selection is happening, causing Cypress to
lose focus and preventing the checkbox from being checked. This PR
disables autorefresh from the table to prevent that from happening.

## Flaky test runner

350 iters:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2976
🟢
jpdjere added a commit that referenced this issue Aug 28, 2023
….cy.ts - Deletes and recovers more than one rule (#164824)

**NOTE: This is a manual backport of
#164694


Relates to: #161507

## Summary

- Solves flakiness in following test:
- Filename:
`x-pack/plugins/security_solution/cypress/e2e/detection_rules/prebuilt_rules_management.cy.ts`
- Test name: **Prebuilt rules Actions with prebuilt rules Rules table
Deletes and recovers more than one rule**

- Test was failing because of already observed issue of `autoRefresh`
taking place while the rule selection is happening, causing Cypress to
lose focus and preventing the checkbox from being checked. This PR
disables autorefresh from the table to prevent that from happening.

## Flaky test runner

350 iters:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2976
🟢
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Aug 28, 2023
…s` - Deletes and recovers more than one rule (elastic#164694)

Relates to: elastic#161507

## Summary

- Solves flakiness in following test:
- Filename:
`x-pack/test/security_solution_cypress/cypress/e2e/detection_response/prebuilt_rules/prebuilt_rules_management.cy.ts`
- Test name: **Prebuilt rules Actions with prebuilt rules Rules table
Deletes and recovers more than one rule**

- Test was failing because of already observed issue of `autoRefresh`
taking place while the rule selection is happening, causing Cypress to
lose focus and preventing the checkbox from being checked. This PR
disables autorefresh from the table to prevent that from happening.

## Flaky test runner

350 iters:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2976
🟢

(cherry picked from commit e22f9b8)
kibanamachine referenced this issue Aug 28, 2023
…nt.cy.ts` - Deletes and recovers more than one rule (#164694) (#164961)

# Backport

This will backport the following commits from `main` to `8.10`:
- [[Security Solution] Fix flakiness in:
`prebuilt_rules_management.cy.ts` - Deletes and recovers more than one
rule (#164694)](#164694)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Juan Pablo
Djeredjian","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-08-28T11:02:22Z","message":"[Security
Solution] Fix flakiness in: `prebuilt_rules_management.cy.ts` - Deletes
and recovers more than one rule (#164694)\n\nRelates to:
https://github.com/elastic/kibana/issues/161507\r\n\r\n##
Summary\r\n\r\n- Solves flakiness in following test:\r\n-
Filename:\r\n`x-pack/test/security_solution_cypress/cypress/e2e/detection_response/prebuilt_rules/prebuilt_rules_management.cy.ts`\r\n-
Test name: **Prebuilt rules Actions with prebuilt rules Rules
table\r\nDeletes and recovers more than one rule**\r\n\r\n- Test was
failing because of already observed issue of `autoRefresh`\r\ntaking
place while the rule selection is happening, causing Cypress to\r\nlose
focus and preventing the checkbox from being checked. This
PR\r\ndisables autorefresh from the table to prevent that from
happening.\r\n\r\n## Flaky test runner\r\n\r\n350
iters:\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2976\r\n🟢","sha":"e22f9b860e6d62ddcaacae6a1b2f57178afa8e54","branchLabelMapping":{"^v8.11.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["test","release_note:skip","Team:Detections
and Resp","Team: SecuritySolution","Feature:Rule
Management","Team:Detection Rule
Management","v8.10.0","v8.11.0"],"number":164694,"url":"https://github.com/elastic/kibana/pull/164694","mergeCommit":{"message":"[Security
Solution] Fix flakiness in: `prebuilt_rules_management.cy.ts` - Deletes
and recovers more than one rule (#164694)\n\nRelates to:
https://github.com/elastic/kibana/issues/161507\r\n\r\n##
Summary\r\n\r\n- Solves flakiness in following test:\r\n-
Filename:\r\n`x-pack/test/security_solution_cypress/cypress/e2e/detection_response/prebuilt_rules/prebuilt_rules_management.cy.ts`\r\n-
Test name: **Prebuilt rules Actions with prebuilt rules Rules
table\r\nDeletes and recovers more than one rule**\r\n\r\n- Test was
failing because of already observed issue of `autoRefresh`\r\ntaking
place while the rule selection is happening, causing Cypress to\r\nlose
focus and preventing the checkbox from being checked. This
PR\r\ndisables autorefresh from the table to prevent that from
happening.\r\n\r\n## Flaky test runner\r\n\r\n350
iters:\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2976\r\n🟢","sha":"e22f9b860e6d62ddcaacae6a1b2f57178afa8e54"}},"sourceBranch":"main","suggestedTargetBranches":["8.10"],"targetPullRequestStates":[{"branch":"8.10","label":"v8.10.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.11.0","labelRegex":"^v8.11.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/164694","number":164694,"mergeCommit":{"message":"[Security
Solution] Fix flakiness in: `prebuilt_rules_management.cy.ts` - Deletes
and recovers more than one rule (#164694)\n\nRelates to:
https://github.com/elastic/kibana/issues/161507\r\n\r\n##
Summary\r\n\r\n- Solves flakiness in following test:\r\n-
Filename:\r\n`x-pack/test/security_solution_cypress/cypress/e2e/detection_response/prebuilt_rules/prebuilt_rules_management.cy.ts`\r\n-
Test name: **Prebuilt rules Actions with prebuilt rules Rules
table\r\nDeletes and recovers more than one rule**\r\n\r\n- Test was
failing because of already observed issue of `autoRefresh`\r\ntaking
place while the rule selection is happening, causing Cypress to\r\nlose
focus and preventing the checkbox from being checked. This
PR\r\ndisables autorefresh from the table to prevent that from
happening.\r\n\r\n## Flaky test runner\r\n\r\n350
iters:\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2976\r\n🟢","sha":"e22f9b860e6d62ddcaacae6a1b2f57178afa8e54"}}]}]
BACKPORT-->

Co-authored-by: Juan Pablo Djeredjian <[email protected]>
@banderror
Copy link
Contributor Author

@maximpn @jpdjere Thank you for all your work to fix so many flaky tests. I'm closing this one in favor of a new ticket for 8.11: #165359

jpdjere added a commit that referenced this issue Sep 8, 2023
)

**NOTE: This is a manual backport of #164099**

**Original PR description:**
---

**Relates to: #161507
**Fixes: #163704
**Fixes: #163182
**Fixes: #163558
**Fixes: #163974
**Fixes: #153914
**Fixes: #164079
**Fixes: #164279

## Summary

While working on fixing Rules Management flaky tests I've noticed
similar fails in different tests. This PR addresses common pitfalls to
reduce a number of reasons causing e2e tests flakiness and as a result
reduce a number of flaky tests.

## Details

The common reasons causing e2e tests flakiness for the rules tables are

- Auto-refresh

Auto-refresh functionality is enabled by default and the table gets
auto-refreshed every 60 seconds. If a test takes more than 60 seconds
the table fetches updated rules. Having rules enabled by default and
sorted by `Enabled` column makes the sorting order undetermined and as
rules get updated due to execution ES return them in undetermined order.
This update can happen between commands working on the same element and
indexed access like `eq()` would access different elements.

- Missing selectors

Some tests or helper functions have expectations for an element absence
like `should('not.exist')` without checking an element is visible before
like `should('be.visible')`. This way a referenced element may disappear
from the codebase after refactoring and the expectation still fulfils.

- Checking for `should('not.visible')` while an element is removed from
the DOM

It most applicable to popovers as it first animates to be hidden and
then removed from the DOM. Cypress first need to find an element to
check its visibility. Replacing `should('not.visible')` with
`should('not.exist')` and taking into concern from the account previous
bullet fixes the problem.

- Modifying ES data without refreshing (`_delete_by_query` in
particular)

Due to high performance ES architecture data isn't updated instantly.
Having such behavior in tests leads to undetermined state depending on a
number of environmental factors. As UI doesn't always auto-refreshes to
fetch the recent updates in short period of time test fail.
`_delete_by_query` may take some time to update the data but it doesn't
support `refresh=wait_for` as it stated in
[docs](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete-by-query.html#_refreshing_shards).
Adding `?refresh=true` or just `?refresh` to `_delete_by_query` ES
request urls fixes the problem.

### What was done to address mentioned reasons above?

- Auto-refresh functionality disabled in tests where it's not necessary.
- `enabled: false` field was added to rule creators to have disabled
rules as the majority of tests don't need enabled rules.
- `waitForRulesTableToBeLoaded` was removed and replaced with
`expectManagementTableRules` at some tests.
- `should('not.visible')` replaced with `should('not.exist')` in
`deleteRuleFromDetailsPage()`
- `?refresh` added to `_delete_by_query` ES data update requests

The other changes get rid of global constants and improve readability.

## Flaky test runs

[All Cypress tests under `detection_response` folder (100
runs)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2920)
(value lists export is flaky but it's out of scope of this PR)

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.10 candidate Meta Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. technical debt Improvement of the software architecture and operational architecture test_ui_functional test test-api-integration v8.10.0
Projects
None yet
Development

No branches or pull requests

4 participants