Skip to content

Commit

Permalink
[Security Solution] Reduce Rules Management e2e flakiness (#164099)
Browse files Browse the repository at this point in the history
**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)
  • Loading branch information
maximpn authored Aug 23, 2023
1 parent 3b6c2c2 commit 40df521
Show file tree
Hide file tree
Showing 28 changed files with 933 additions and 814 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { ROLES } from '@kbn/security-solution-plugin/common/test';
import { tag } from '../../../tags';

import { createRuleAssetSavedObject } from '../../../helpers/rules';
import { waitForRulesTableToBeLoaded } from '../../../tasks/alerts_detection_rules';
import { createAndInstallMockedPrebuiltRules } from '../../../tasks/api_calls/prebuilt_rules';
import { resetRulesTableState, deleteAlertsAndRules } from '../../../tasks/common';
import { login, waitForPageWithoutDateRange } from '../../../tasks/login';
Expand Down Expand Up @@ -66,7 +65,6 @@ describe(
resetRulesTableState();
deleteAlertsAndRules();
cy.task('esArchiverResetKibana');
waitForRulesTableToBeLoaded();
createAndInstallMockedPrebuiltRules({ rules: [OUTDATED_RULE_1, OUTDATED_RULE_2] });
});

Expand All @@ -83,7 +81,6 @@ describe(
// Now login with read-only user in preparation for test
createAndInstallMockedPrebuiltRules({ rules: [RULE_1, RULE_2], installToKibana: false });
loadPageAsReadOnlyUser(SECURITY_DETECTIONS_RULES_URL);
waitForRulesTableToBeLoaded();
});

it('should not be able to install prebuilt rules', () => {
Expand Down Expand Up @@ -111,7 +108,6 @@ describe(
});
// Now login with read-only user in preparation for test
loadPageAsReadOnlyUser(SECURITY_DETECTIONS_RULES_URL);
waitForRulesTableToBeLoaded();
});

it('should not be able to upgrade prebuilt rules', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@
import { tag } from '../../../tags';

import { createRuleAssetSavedObject } from '../../../helpers/rules';
import { waitForRulesTableToBeLoaded } from '../../../tasks/alerts_detection_rules';
import { createAndInstallMockedPrebuiltRules } from '../../../tasks/api_calls/prebuilt_rules';
import { resetRulesTableState, deleteAlertsAndRules, reload } from '../../../tasks/common';
import { login, visitWithoutDateRange } from '../../../tasks/login';
import { SECURITY_DETECTIONS_RULES_URL } from '../../../urls/navigation';
import { login, visitSecurityDetectionRulesPage } from '../../../tasks/login';
import {
addElasticRulesButtonClick,
assertRuleAvailableForInstallAndInstallOne,
Expand All @@ -36,7 +34,7 @@ describe(
deleteAlertsAndRules();
cy.task('esArchiverResetKibana');

visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
visitSecurityDetectionRulesPage();
});

describe('Installation of prebuilt rules - Should fail gracefully with toast error message when', () => {
Expand All @@ -50,7 +48,6 @@ describe(
});
beforeEach(() => {
createAndInstallMockedPrebuiltRules({ rules: [RULE_1, RULE_2], installToKibana: false });
waitForRulesTableToBeLoaded();
});

it('installing prebuilt rules one by one', () => {
Expand Down Expand Up @@ -114,7 +111,6 @@ describe(
rules: [UPDATED_RULE_1, UPDATED_RULE_2],
installToKibana: false,
});
waitForRulesTableToBeLoaded();
reload();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@ import {
SELECT_ALL_RULES_ON_PAGE_CHECKBOX,
TOASTER,
} from '../../../screens/alerts_detection_rules';
import { waitForRulesTableToBeLoaded } from '../../../tasks/alerts_detection_rules';
import {
getRuleAssets,
createAndInstallMockedPrebuiltRules,
} from '../../../tasks/api_calls/prebuilt_rules';
import { resetRulesTableState, deleteAlertsAndRules, reload } from '../../../tasks/common';
import { login, visitWithoutDateRange } from '../../../tasks/login';
import { SECURITY_DETECTIONS_RULES_URL } from '../../../urls/navigation';
import { login, visitSecurityDetectionRulesPage } from '../../../tasks/login';
import {
addElasticRulesButtonClick,
assertRuleAvailableForInstallAndInstallOne,
Expand All @@ -51,7 +49,7 @@ describe(
deleteAlertsAndRules();
cy.task('esArchiverResetKibana');

visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
visitSecurityDetectionRulesPage();
});

describe('Installation of prebuilt rules package via Fleet', () => {
Expand All @@ -60,7 +58,6 @@ describe(
cy.intercept('POST', '/api/fleet/epm/packages/security_detection_engine/*').as(
'installPackage'
);
waitForRulesTableToBeLoaded();
});

it('should install package from Fleet in the background', () => {
Expand Down Expand Up @@ -150,7 +147,6 @@ describe(
});
beforeEach(() => {
createAndInstallMockedPrebuiltRules({ rules: [RULE_1, RULE_2], installToKibana: false });
waitForRulesTableToBeLoaded();
cy.intercept('POST', '/internal/detection_engine/prebuilt_rules/installation/_perform').as(
'installPrebuiltRules'
);
Expand Down Expand Up @@ -232,7 +228,6 @@ describe(
rules: [UPDATED_RULE_1, UPDATED_RULE_2],
installToKibana: false,
});
waitForRulesTableToBeLoaded();
reload();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,15 @@ import {
ADD_ELASTIC_RULES_BTN,
RULES_EMPTY_PROMPT,
RULES_MONITORING_TAB,
RULES_ROW,
RULES_MANAGEMENT_TABLE,
RULE_SWITCH,
SELECT_ALL_RULES_ON_PAGE_CHECKBOX,
INSTALL_ALL_RULES_BUTTON,
} from '../../../screens/alerts_detection_rules';
import {
deleteFirstRule,
getRulesManagementTableRows,
selectAllRules,
selectNumberOfRules,
selectRulesByName,
waitForPrebuiltDetectionRulesToBeLoaded,
waitForRuleToUpdate,
} from '../../../tasks/alerts_detection_rules';
Expand Down Expand Up @@ -69,7 +68,7 @@ describe('Prebuilt rules', { tags: [tag.ESS, tag.SERVERLESS] }, () => {
describe('Alerts rules, prebuilt rules', () => {
it('Loads prebuilt rules', () => {
// Check that the rules table contains rules
cy.get(RULES_MANAGEMENT_TABLE).find(RULES_ROW).should('have.length.gte', 1);
getRulesManagementTableRows().should('have.length.gte', 1);

// Check the correct count of prebuilt rules is displayed
getAvailablePrebuiltRulesCount().then((availablePrebuiltRulesCount) => {
Expand Down Expand Up @@ -111,8 +110,7 @@ describe('Prebuilt rules', { tags: [tag.ESS, tag.SERVERLESS] }, () => {
});

it('Does not allow to delete one rule when more than one is selected', () => {
const numberOfRulesToBeSelected = 2;
selectNumberOfRules(numberOfRulesToBeSelected);
selectAllRules();

cy.get(COLLAPSED_ACTION_BTN).each((collapsedItemActionBtn) => {
cy.wrap(collapsedItemActionBtn).should('have.attr', 'disabled');
Expand Down Expand Up @@ -153,16 +151,16 @@ describe('Prebuilt rules', { tags: [tag.ESS, tag.SERVERLESS] }, () => {

it('Deletes and recovers more than one rule', () => {
getAvailablePrebuiltRulesCount().then((availablePrebuiltRulesCount) => {
const numberOfRulesToBeSelected = 2;
const rulesToDelete = ['Test rule 1', 'Test rule 2'] as const;
const expectedNumberOfRulesAfterDeletion = availablePrebuiltRulesCount - 2;
const expectedNumberOfRulesAfterRecovering = availablePrebuiltRulesCount;

selectNumberOfRules(numberOfRulesToBeSelected);
selectRulesByName(rulesToDelete);
deleteSelectedRules();

cy.get(ADD_ELASTIC_RULES_BTN).should(
'have.text',
`Add Elastic rules${numberOfRulesToBeSelected}`
`Add Elastic rules${rulesToDelete.length}`
);
cy.get(ELASTIC_RULES_BTN).should(
'have.text',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
import { tag } from '../../../tags';

import { createRuleAssetSavedObject } from '../../../helpers/rules';
import { ADD_ELASTIC_RULES_BTN, RULES_UPDATES_TAB } from '../../../screens/alerts_detection_rules';
import {
deleteFirstRule,
waitForRulesTableToBeLoaded,
} from '../../../tasks/alerts_detection_rules';
ADD_ELASTIC_RULES_BTN,
ADD_ELASTIC_RULES_EMPTY_PROMPT_BTN,
RULES_UPDATES_TAB,
} from '../../../screens/alerts_detection_rules';
import { deleteFirstRule } from '../../../tasks/alerts_detection_rules';
import {
installAllPrebuiltRulesRequest,
createAndInstallMockedPrebuiltRules,
Expand All @@ -23,8 +24,7 @@ import {
reload,
deletePrebuiltRulesAssets,
} from '../../../tasks/common';
import { login, visitWithoutDateRange } from '../../../tasks/login';
import { SECURITY_DETECTIONS_RULES_URL } from '../../../urls/navigation';
import { login, visitSecurityDetectionRulesPage } from '../../../tasks/login';

const RULE_1 = createRuleAssetSavedObject({
name: 'Test rule 1',
Expand All @@ -45,8 +45,10 @@ describe(

describe('No notifications', () => {
it('should NOT display install or update notifications when no prebuilt assets and no rules are installed', () => {
visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
waitForRulesTableToBeLoaded();
visitSecurityDetectionRulesPage();

cy.get(ADD_ELASTIC_RULES_EMPTY_PROMPT_BTN).should('be.visible');

// TODO: test plan asserts "should NOT see a CTA to install prebuilt rules"
// but current behavior is to always show the CTA, even with no prebuilt rule assets installed
// Update that behaviour and then update this test.
Expand All @@ -55,8 +57,7 @@ describe(

it('should NOT display install or update notifications when latest rules are installed', () => {
createAndInstallMockedPrebuiltRules({ rules: [RULE_1], installToKibana: true });
visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
waitForRulesTableToBeLoaded();
visitSecurityDetectionRulesPage();

/* Assert that there are no installation or update notifications */
/* Add Elastic Rules button should not contain a number badge */
Expand All @@ -73,7 +74,7 @@ describe(

describe('Rules installation notification when no rules have been installed', () => {
beforeEach(() => {
visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
visitSecurityDetectionRulesPage();
});

it('should notify user about prebuilt rules available for installation', () => {
Expand Down Expand Up @@ -101,8 +102,7 @@ describe(
rules: [RULE_2, RULE_3],
installToKibana: false,
});
visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
waitForRulesTableToBeLoaded();
visitSecurityDetectionRulesPage();
});
});

Expand Down Expand Up @@ -139,8 +139,7 @@ describe(
version: 2,
});
createAndInstallMockedPrebuiltRules({ rules: [UPDATED_RULE], installToKibana: false });
visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
waitForRulesTableToBeLoaded();
visitSecurityDetectionRulesPage();
reload();
});
});
Expand Down Expand Up @@ -174,8 +173,7 @@ describe(
rules: [RULE_2, UPDATED_RULE],
installToKibana: false,
});
visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
waitForRulesTableToBeLoaded();
visitSecurityDetectionRulesPage();
});
});

Expand Down
Loading

0 comments on commit 40df521

Please sign in to comment.