Skip to content

Commit

Permalink
[8.9] [Security Solution] Reduce Rules Management e2e flakiness (#165468
Browse files Browse the repository at this point in the history
)

**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]>
  • Loading branch information
jpdjere and kibanamachine authored Sep 8, 2023
1 parent cb8b5a9 commit 2c0c69a
Show file tree
Hide file tree
Showing 31 changed files with 1,170 additions and 1,037 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
RULE_NAME,
} from '../../screens/alerts_detection_rules';
import { VALUE_LISTS_MODAL_ACTIVATOR } from '../../screens/lists';
import { waitForRulesTableToBeLoaded } from '../../tasks/alerts_detection_rules';
import { createRule } from '../../tasks/api_calls/rules';
import { cleanKibana } from '../../tasks/common';
import {
Expand All @@ -22,19 +21,17 @@ import {
waitForCallOutToBeShown,
MISSING_PRIVILEGES_CALLOUT,
} from '../../tasks/common/callouts';
import { login, visitWithoutDateRange } from '../../tasks/login';
import { SECURITY_DETECTIONS_RULES_URL } from '../../urls/navigation';
import { login, visitSecurityDetectionRulesPage } from '../../tasks/login';

describe('All rules - read only', () => {
before(() => {
cleanKibana();
createRule(getNewRule({ rule_id: '1' }));
createRule(getNewRule({ rule_id: '1', enabled: false }));
});

beforeEach(() => {
login(ROLES.reader);
visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL, ROLES.reader);
waitForRulesTableToBeLoaded();
visitSecurityDetectionRulesPage(ROLES.reader);
cy.get(RULE_NAME).should('have.text', getNewRule().name);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,29 @@
* 2.0.
*/

import { getNewRule } from '../../objects/rule';
import { EXCEPTION_CARD_ITEM_NAME } from '../../screens/exceptions';
import {
waitForRulesTableToBeLoaded,
goToTheRuleDetailsOf,
selectNumberOfRules,
duplicateSelectedRulesWithoutExceptions,
disableAutoRefresh,
selectAllRules,
expectManagementTableRules,
duplicateSelectedRulesWithExceptions,
duplicateSelectedRulesWithNonExpiredExceptions,
goToTheRuleDetailsOf,
} from '../../tasks/alerts_detection_rules';

import { goToExceptionsTab, viewExpiredExceptionItems } from '../../tasks/rule_details';
import { login, visitWithoutDateRange } from '../../tasks/login';

import { SECURITY_DETECTIONS_RULES_URL } from '../../urls/navigation';
import { createRuleExceptionItem } from '../../tasks/api_calls/exceptions';
import { createRule } from '../../tasks/api_calls/rules';
import { cleanKibana, resetRulesTableState, deleteAlertsAndRules } from '../../tasks/common';

import { getNewRule } from '../../objects/rule';

import { esArchiverResetKibana } from '../../tasks/es_archiver';

import { createRuleExceptionItem } from '../../tasks/api_calls/exceptions';
import { EXCEPTION_CARD_ITEM_NAME } from '../../screens/exceptions';
import {
assertExceptionItemsExists,
assertNumberOfExceptionItemsExists,
} from '../../tasks/exceptions';
import { login, visitSecurityDetectionRulesPage } from '../../tasks/login';
import { goToExceptionsTab, viewExpiredExceptionItems } from '../../tasks/rule_details';
import {
duplicateSelectedRulesWithExceptions,
duplicateSelectedRulesWithNonExpiredExceptions,
duplicateSelectedRulesWithoutExceptions,
} from '../../tasks/rules_bulk_edit';
import { esArchiverResetKibana } from '../../tasks/es_archiver';

const RULE_NAME = 'Custom rule for bulk actions';

Expand Down Expand Up @@ -61,55 +57,54 @@ describe('Detection rules, bulk duplicate', () => {
resetRulesTableState();
deleteAlertsAndRules();
esArchiverResetKibana();
createRule(getNewRule({ name: RULE_NAME, ...defaultRuleData, rule_id: '1' })).then(
(response) => {
createRuleExceptionItem(response.body.id, [
{
description: 'Exception item for rule default exception list',
entries: [
{
field: 'user.name',
operator: 'included',
type: 'match',
value: 'some value',
},
],
name: EXPIRED_EXCEPTION_ITEM_NAME,
type: 'simple',
expire_time: expiredDate,
},
{
description: 'Exception item for rule default exception list',
entries: [
{
field: 'user.name',
operator: 'included',
type: 'match',
value: 'some value',
},
],
name: NON_EXPIRED_EXCEPTION_ITEM_NAME,
type: 'simple',
expire_time: futureDate,
},
]);
}
);

visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
createRule(
getNewRule({ name: RULE_NAME, ...defaultRuleData, rule_id: '1', enabled: false })
).then((response) => {
createRuleExceptionItem(response.body.id, [
{
description: 'Exception item for rule default exception list',
entries: [
{
field: 'user.name',
operator: 'included',
type: 'match',
value: 'some value',
},
],
name: EXPIRED_EXCEPTION_ITEM_NAME,
type: 'simple',
expire_time: expiredDate,
},
{
description: 'Exception item for rule default exception list',
entries: [
{
field: 'user.name',
operator: 'included',
type: 'match',
value: 'some value',
},
],
name: NON_EXPIRED_EXCEPTION_ITEM_NAME,
type: 'simple',
expire_time: futureDate,
},
]);
});

waitForRulesTableToBeLoaded();
visitSecurityDetectionRulesPage();
disableAutoRefresh();
});

it('Duplicates rules', () => {
selectNumberOfRules(1);
selectAllRules();
duplicateSelectedRulesWithoutExceptions();
expectManagementTableRules([`${RULE_NAME} [Duplicate]`]);
});

describe('With exceptions', () => {
it('Duplicates rules with expired exceptions', () => {
selectNumberOfRules(1);
selectAllRules();
duplicateSelectedRulesWithExceptions();
expectManagementTableRules([`${RULE_NAME} [Duplicate]`]);
goToTheRuleDetailsOf(`${RULE_NAME} [Duplicate]`);
Expand All @@ -120,7 +115,7 @@ describe('Detection rules, bulk duplicate', () => {
});

it('Duplicates rules with exceptions, excluding expired exceptions', () => {
selectNumberOfRules(1);
selectAllRules();
duplicateSelectedRulesWithNonExpiredExceptions();
expectManagementTableRules([`${RULE_NAME} [Duplicate]`]);
goToTheRuleDetailsOf(`${RULE_NAME} [Duplicate]`);
Expand Down
Loading

0 comments on commit 2c0c69a

Please sign in to comment.