Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[8.9] [Security Solution] [Detection Engine] Fix
rule_exception cyp…
…ress test flakiness (#161751) (#163688) # Backport This will backport the following commits from `main` to `8.9`: - [[Security Solution] [Detection Engine] Fix `rule_exception` cypress test flakiness (#161751)](#161751) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Wafaa Nasr","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-08-01T11:28:46Z","message":"[Security Solution] [Detection Engine] Fix `rule_exception` cypress test flakiness (#161751)\n\n## Summary\r\n\r\n- Fixes #159341 \r\n\r\n**Steps to insure the flakiness** \r\n\r\n- Run the test through Flaky test runner by modifying the\r\n`x-pack/plugins/security_solution/package.json` to only run the\r\n`Rule_exceptions` as the following =>\r\n`./cypress/e2e/exceptions/**/rule_exception.cy.ts`\r\n- Run the test locally multiple times by wrapping the test suite by \r\n `Cypress._.times(50, () => {});`\r\n\r\n**Failing steps**\r\n\r\n- The test only failed in the step mentioned when executed through the\r\nFlaky test runner.\r\n\r\n```\r\n 1) Rule Exceptions workflows from Alert\r\n --\r\n | Should create a Rule exception item from the alert actions overflow menu and close all matching alerts:\r\n | AssertionError: Timed out retrying after 150000ms: Expected to find element: `[data-test-subj=\"alertsStateTableEmptyState\"]`, but never found it.\r\n```\r\n\r\n- By executing it locally multiple times, the test failed at different\r\nsteps on each occasion.\r\n \r\n- Immediately after creating an Exception with the Closing all matching\r\nalerts option, the Alert table was not found empty, indicating a\r\nfailure.\r\n\r\n\r\n\r\n- After removing the exception, the expected number of alerts was not\r\nfound in the Alerts table, resulting in a failure.\r\n\r\n\r\n\r\n- Unrelated to this test, but concerning exceptions, the failure\r\noccurred when attempting to click on \"Submit exception\" due to an error\r\ntoast obstructing the button, similar to the 2nd screenshot provided\r\nbelow.\r\n\r\n\r\n\r\n\r\n\r\n**Steps to mitigate test flakiness:**\r\n\r\n- **Splitting Test Cases:** The rule_exceptions test currently covers\r\nmultiple areas using different Rule types. To improve stability, we\r\nshould divide it into separate test files. For example,\r\n`closing_all_matching_alerts.cy.ts` and\r\n`auto_populate_with_alert_data.cy.ts`. Additionally, we should ensure\r\nthat each test file cleans up any previous data to maintain test\r\nisolation.\r\n\r\n- **Handling Async Calls:** Test flakiness can occur when Cypress\r\nexecutes faster than async calls, leading to errors when alerts don't\r\nmove to the closed tab in time. To address this, we should avoid\r\nimmediate validation after creating an exception. Instead, refresh the\r\n`Alerts table` to allow time for the alerts to be moved to the\r\nappropriate tab before asserting the conditions.\r\n\r\n For example ` waitForAlerts();` method\r\n\r\n- **Avoiding Specific Alert Counts:** Relying on a specific number of\r\ngenerated or closed alerts for validation is not a robust approach.\r\nDuring test execution, the Rule might generate more alerts than\r\nexpected, leading to test failures. To overcome this, we should focus on\r\nvalidating the expected behavior rather than a fixed alert count.\r\n \r\n For example\r\n ```\r\n goToClosedAlertsOnRuleDetailsPage(); \r\n cy.get(ALERTS_COUNT).should('exist');\r\n ```\r\n\r\n- In the test, when utilizing `cy.task('esArchiverLoad',\r\n'exceptions');`, it can be beneficial to check if the exception list is\r\nnot present in the environment by calling `cy.task('esArchiverUnload',\r\n'exceptions');` beforehand. This cleanup step ensures that any existing\r\nexception list is removed, mitigating the occurrence of the following\r\nerror: `resource_already_exists_exception: index\r\n[.lists-default-000001/U_3_TShzRY-WZD_XqhSBnw] already exists (400).`\r\n\r\n- It does not provide a value at the end of the\r\n`closing_all_matching_alerts.cy.ts` test to verify if alerts are\r\ngenerated again after the `Exception` that closes all matching alerts is\r\ndeleted because the test already begins with the rule generating alerts\r\nuntil the exception is created. Therefore, this part has been excluded.\r\n\r\nBy following these steps for the test, running the new files multiple\r\ntimes locally proved to be beneficial:\r\n\r\n\r\n\r\n successfully \r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"8f8c51a2d6c960007a9dc5f8fb4abd9fad7ec5aa","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","test-failure-flaky","backport:prev-minor","Team:Detection Engine","v8.10.0"],"number":161751,"url":"https://github.com/elastic/kibana/pull/161751","mergeCommit":{"message":"[Security Solution] [Detection Engine] Fix `rule_exception` cypress test flakiness (#161751)\n\n## Summary\r\n\r\n- Fixes #159341 \r\n\r\n**Steps to insure the flakiness** \r\n\r\n- Run the test through Flaky test runner by modifying the\r\n`x-pack/plugins/security_solution/package.json` to only run the\r\n`Rule_exceptions` as the following =>\r\n`./cypress/e2e/exceptions/**/rule_exception.cy.ts`\r\n- Run the test locally multiple times by wrapping the test suite by \r\n `Cypress._.times(50, () => {});`\r\n\r\n**Failing steps**\r\n\r\n- The test only failed in the step mentioned when executed through the\r\nFlaky test runner.\r\n\r\n```\r\n 1) Rule Exceptions workflows from Alert\r\n --\r\n | Should create a Rule exception item from the alert actions overflow menu and close all matching alerts:\r\n | AssertionError: Timed out retrying after 150000ms: Expected to find element: `[data-test-subj=\"alertsStateTableEmptyState\"]`, but never found it.\r\n```\r\n\r\n- By executing it locally multiple times, the test failed at different\r\nsteps on each occasion.\r\n \r\n- Immediately after creating an Exception with the Closing all matching\r\nalerts option, the Alert table was not found empty, indicating a\r\nfailure.\r\n\r\n\r\n\r\n- After removing the exception, the expected number of alerts was not\r\nfound in the Alerts table, resulting in a failure.\r\n\r\n\r\n\r\n- Unrelated to this test, but concerning exceptions, the failure\r\noccurred when attempting to click on \"Submit exception\" due to an error\r\ntoast obstructing the button, similar to the 2nd screenshot provided\r\nbelow.\r\n\r\n\r\n\r\n\r\n\r\n**Steps to mitigate test flakiness:**\r\n\r\n- **Splitting Test Cases:** The rule_exceptions test currently covers\r\nmultiple areas using different Rule types. To improve stability, we\r\nshould divide it into separate test files. For example,\r\n`closing_all_matching_alerts.cy.ts` and\r\n`auto_populate_with_alert_data.cy.ts`. Additionally, we should ensure\r\nthat each test file cleans up any previous data to maintain test\r\nisolation.\r\n\r\n- **Handling Async Calls:** Test flakiness can occur when Cypress\r\nexecutes faster than async calls, leading to errors when alerts don't\r\nmove to the closed tab in time. To address this, we should avoid\r\nimmediate validation after creating an exception. Instead, refresh the\r\n`Alerts table` to allow time for the alerts to be moved to the\r\nappropriate tab before asserting the conditions.\r\n\r\n For example ` waitForAlerts();` method\r\n\r\n- **Avoiding Specific Alert Counts:** Relying on a specific number of\r\ngenerated or closed alerts for validation is not a robust approach.\r\nDuring test execution, the Rule might generate more alerts than\r\nexpected, leading to test failures. To overcome this, we should focus on\r\nvalidating the expected behavior rather than a fixed alert count.\r\n \r\n For example\r\n ```\r\n goToClosedAlertsOnRuleDetailsPage(); \r\n cy.get(ALERTS_COUNT).should('exist');\r\n ```\r\n\r\n- In the test, when utilizing `cy.task('esArchiverLoad',\r\n'exceptions');`, it can be beneficial to check if the exception list is\r\nnot present in the environment by calling `cy.task('esArchiverUnload',\r\n'exceptions');` beforehand. This cleanup step ensures that any existing\r\nexception list is removed, mitigating the occurrence of the following\r\nerror: `resource_already_exists_exception: index\r\n[.lists-default-000001/U_3_TShzRY-WZD_XqhSBnw] already exists (400).`\r\n\r\n- It does not provide a value at the end of the\r\n`closing_all_matching_alerts.cy.ts` test to verify if alerts are\r\ngenerated again after the `Exception` that closes all matching alerts is\r\ndeleted because the test already begins with the rule generating alerts\r\nuntil the exception is created. Therefore, this part has been excluded.\r\n\r\nBy following these steps for the test, running the new files multiple\r\ntimes locally proved to be beneficial:\r\n\r\n\r\n\r\n successfully \r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"8f8c51a2d6c960007a9dc5f8fb4abd9fad7ec5aa"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/161751","number":161751,"mergeCommit":{"message":"[Security Solution] [Detection Engine] Fix `rule_exception` cypress test flakiness (#161751)\n\n## Summary\r\n\r\n- Fixes #159341 \r\n\r\n**Steps to insure the flakiness** \r\n\r\n- Run the test through Flaky test runner by modifying the\r\n`x-pack/plugins/security_solution/package.json` to only run the\r\n`Rule_exceptions` as the following =>\r\n`./cypress/e2e/exceptions/**/rule_exception.cy.ts`\r\n- Run the test locally multiple times by wrapping the test suite by \r\n `Cypress._.times(50, () => {});`\r\n\r\n**Failing steps**\r\n\r\n- The test only failed in the step mentioned when executed through the\r\nFlaky test runner.\r\n\r\n```\r\n 1) Rule Exceptions workflows from Alert\r\n --\r\n | Should create a Rule exception item from the alert actions overflow menu and close all matching alerts:\r\n | AssertionError: Timed out retrying after 150000ms: Expected to find element: `[data-test-subj=\"alertsStateTableEmptyState\"]`, but never found it.\r\n```\r\n\r\n- By executing it locally multiple times, the test failed at different\r\nsteps on each occasion.\r\n \r\n- Immediately after creating an Exception with the Closing all matching\r\nalerts option, the Alert table was not found empty, indicating a\r\nfailure.\r\n\r\n\r\n\r\n- After removing the exception, the expected number of alerts was not\r\nfound in the Alerts table, resulting in a failure.\r\n\r\n\r\n\r\n- Unrelated to this test, but concerning exceptions, the failure\r\noccurred when attempting to click on \"Submit exception\" due to an error\r\ntoast obstructing the button, similar to the 2nd screenshot provided\r\nbelow.\r\n\r\n\r\n\r\n\r\n\r\n**Steps to mitigate test flakiness:**\r\n\r\n- **Splitting Test Cases:** The rule_exceptions test currently covers\r\nmultiple areas using different Rule types. To improve stability, we\r\nshould divide it into separate test files. For example,\r\n`closing_all_matching_alerts.cy.ts` and\r\n`auto_populate_with_alert_data.cy.ts`. Additionally, we should ensure\r\nthat each test file cleans up any previous data to maintain test\r\nisolation.\r\n\r\n- **Handling Async Calls:** Test flakiness can occur when Cypress\r\nexecutes faster than async calls, leading to errors when alerts don't\r\nmove to the closed tab in time. To address this, we should avoid\r\nimmediate validation after creating an exception. Instead, refresh the\r\n`Alerts table` to allow time for the alerts to be moved to the\r\nappropriate tab before asserting the conditions.\r\n\r\n For example ` waitForAlerts();` method\r\n\r\n- **Avoiding Specific Alert Counts:** Relying on a specific number of\r\ngenerated or closed alerts for validation is not a robust approach.\r\nDuring test execution, the Rule might generate more alerts than\r\nexpected, leading to test failures. To overcome this, we should focus on\r\nvalidating the expected behavior rather than a fixed alert count.\r\n \r\n For example\r\n ```\r\n goToClosedAlertsOnRuleDetailsPage(); \r\n cy.get(ALERTS_COUNT).should('exist');\r\n ```\r\n\r\n- In the test, when utilizing `cy.task('esArchiverLoad',\r\n'exceptions');`, it can be beneficial to check if the exception list is\r\nnot present in the environment by calling `cy.task('esArchiverUnload',\r\n'exceptions');` beforehand. This cleanup step ensures that any existing\r\nexception list is removed, mitigating the occurrence of the following\r\nerror: `resource_already_exists_exception: index\r\n[.lists-default-000001/U_3_TShzRY-WZD_XqhSBnw] already exists (400).`\r\n\r\n- It does not provide a value at the end of the\r\n`closing_all_matching_alerts.cy.ts` test to verify if alerts are\r\ngenerated again after the `Exception` that closes all matching alerts is\r\ndeleted because the test already begins with the rule generating alerts\r\nuntil the exception is created. Therefore, this part has been excluded.\r\n\r\nBy following these steps for the test, running the new files multiple\r\ntimes locally proved to be beneficial:\r\n\r\n\r\n\r\n successfully \r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"8f8c51a2d6c960007a9dc5f8fb4abd9fad7ec5aa"}}]}] BACKPORT--> --------- Co-authored-by: kibanamachine <[email protected]>
- Loading branch information
1 parent
072be8c
commit fd35b9a