[Security Solution] Replace goToRuleDetails with goToTheRuleDetailsOf in e2e tests #164513
Assignees
Labels
8.11 candidate
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
Comments
maximpn
added
test
technical debt
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
This was referenced Aug 23, 2023
maximpn
added a commit
that referenced
this issue
Sep 7, 2023
…uleDetailsOf()` (#165011) **Addresses:** #164513 ## Summary This PR gets rid of Rules Management `goToRuleDetails()` Cypress helper replacing it with `goToRuleDetailsOf()`. ## Details The following has been done - `goToRuleDetails()` replaced with `goToRuleDetailsOf()` where it makes sense (rule creation, scenarios validating something on the rules management table and then on the rule details page and etc). - For quite a number of tests expectations are done on the rule details page but it's opened via rules management table page. In fact this unnecessary step may be flaky as rules table has auto resfresh feature and it should be ideally disabled as well as this requires loading of lazy UI resources and increases test's execution time. It can be avoided by directly opening the details page via ```ts createRule(getNewRule()).then((rule) => visitWithoutDateRange(ruleDetailsUrl(rule.body.id)) ); ``` `goToRuleDetails()` was removed from such tests in favor of opening the rule details page directly. ## Does opening rule detail page directly make the tests less stable? Yes, it can give such side effect but it will help to either find and fix some bugs in our code or make our tests more robust by adding an universal page ready expectation. While working on this PR I had to fix a few problems on the rule details page caused by getting rid of unnecessary opening rule details page via rules management table but it fact it revealed rule details page's problems - If a test needs first thing to enable a rule like editing test for custom query rule the test may fail as `-1` is sent as rule id. In fact it's a page loading problem. Rule switch become available while the rule isn't loaded yet and it can be easily reproduced in prod if the connection is slow. Disabling rule switch while rule isn't loaded yet solves the problem. - Rule exceptions tests turned to be flaky. Edit exception list item context menu closes on its own sometimes. In fact `useEffect` causing HTTP request fired much more often than it should due to exception list types defined in place and regenerated each render causing the list item to be in loading state while the request is in progress. It was fixed by moving the arrays to stable constants. ## Flaky test runner [Security Solution Cypress tests (150 runs)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3017) (there is no indication of negative impact)
|
It was implemented in #165011. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
goToRuleDetails()helper function is used in a number of tests and its functionality opens the first rule's details page. Opening the first rule in the rules table may lead to flaky behavior and generally make tests less explicit. UsinggoToTheRuleDetailsOf()instead should better convey test expecations.Motivation
While working on flaky tests fix it has been noticed having higher deterministic test behavior helps reducing tests flakiness.
goToRuleDetails()clicks on the first rule in the Rules Management table to open its details page and while it looks ok for a number of tests it may be a case non desired rule opened. If it happens in CI it's pretty hard to understand what's wrong and why the test failed. Also taking into account users distinguish rules by name and having two rules with exactly the same name is confusing for users we can rely on rule name uniqueness in tests. This way having tests expecting a concrete rule in the table to click on its name to open rule details page will make the tests behaving much more predictable.The text was updated successfully, but these errors were encountered: