[Security Solution] Add snooze rule tests #158195
Conversation
maximpn
added
release_note:skip
maximpn
force-pushed
the
add-rule-snoozing-tests
branch
2 times, most recently
from
6032fb5 to
a2535e0
Compare
maximpn
added
the
Feature:Rule Management
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
| export const INTERNAL_ALERTING_API_FIND_RULES_PATH = `${INTERNAL_BASE_ALERTING_API_PATH}/rules/_find`; | ||
| export const INTERNAL_BASE_ALERTING_API_PATH = '/internal/alerting' as const; | ||
| export const INTERNAL_ALERTING_SNOOZE_RULE = | ||
| `${INTERNAL_BASE_ALERTING_API_PATH}/rule/{id}/_snooze` as const; |
There was a problem hiding this comment.
TIL this was possible! Wow.
Kinda confusing, especially with the const INTERNAL_BASE_ALERTING_API_PATH presumably getting interpolated in the definition, but {id} left for later interpolation.
But does seem to make sense when reading.
| @@ -69,3 +71,6 @@ export const hostDetailsUrl = (hostName: string) => | |||
| export const userDetailsUrl = (userName: string) => `/app/security/users/${userName}/allUsers`; | |||
|
|
|||
| export const exceptionsListDetailsUrl = (listId: string) => `${EXCEPTIONS_LIST_URL}/${listId}`; | |||
|
|
|||
| export const internalAlertingSnoozeRule = (ruleId: string) => | |||
There was a problem hiding this comment.
Since this file refers to frontend navigation routes, I would place this method somewhere else.
I realize there's no central file within the Cypress folder that hold all API routes, so maybe we can create one?
A new x-pack/plugins/security_solution/cypress/tasks/api_calls/routes.ts file can be an option.
There was a problem hiding this comment.
Right, I moved it to a separate file.
| deleteConnectors(); | ||
| }); | ||
|
|
||
| it('ensures actions are visible', () => { |
There was a problem hiding this comment.
Is this as a seperate test needed? It is not concretely related to snoozing.
If you want to have the test fail if the actions are not visible, you could move the cy.get(actionFormSelector(0)).should('be.visible'); assertion to the next test ("adds an action to a snoozed rule"), which already contains the whole setup before the assertions.
There was a problem hiding this comment.
Good point. I've removed this test as the next one testing that it's possible to create new actions actually covers this case too.
|
Also, would be nice to have the test plan linked in the PR description, for future reference. |
maximpn
force-pushed
the
add-rule-snoozing-tests
branch
3 times, most recently
from
29ae3ec to
d732a38
Compare
maximpn
force-pushed
the
add-rule-snoozing-tests
branch
from
d732a38 to
2fdbcf2
Compare
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @maximpn |
**Addresses:** #159349 ## Summary This PR fixes and re-enables rule snoozing Cypress test `Rule editing page / actions tab - adds an action to a snoozed rule`. ## Details Basically the test wasn't flaky it just failed on the `main` branch and was skipped. The cause lays in interlacing [Rule snooze tests PR](#158195) with [Rule Editing page refactoring PR](#157749). Two PRs were merged independently to the `main` branch (while the tests PR was merged the last) so combining them lead to a test failure while each one separately didn't cause any problems. ### The root cause The root cause is in a combination of factors. [useForm](https://github.com/elastic/kibana/blob/main/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts#L33) hook has a flaw it can't update its state when new `defaultValue` comes in. The same issue also in [useField](https://github.com/elastic/kibana/blob/main/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts#L77) hook. Rule Editing page fetched a fresh rule's data every time it's rendered via [useRule](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx#L581). As `useRule` hook is based on `React Query` it returns stale data during the first render while firing an HTTP request for the fresh data. Obviously the problem happens if the stale data is passed to `useForm` hook as `defaultValue` and when fresh data is fetched and passed again to `useForm` hook the latter just ignores it. Staying longer on the Rule Details page helps to avoid the problem as fetched rule's data is cached and returned as stale data on the Rule Editing page after transition. As stale and fresh data are the same the test would pass. Anyway this behaviour can be reproduced in prod with a slow internet connection. ### What was done to fix the problem? Functionality has been added to update the cached rule's data (React Query cache) upon mutation successful update rule mutation. The mutation if fired by pressing "Save changes" button on the Rule Editing page. It is possible as update rule endpoint returns an updated rule in its response. Along the way it turned out update rule endpoint's and read rule endpoint's responses weren't properly typed so it lead to types mismatch. To fix it `updateRule`'s and `UseMutationOptions`'s return types were changed to `Rule` instead of `RuleResponse`. ### Misc Along the way it turned out `cy.get(RULE_INDICES).should('be.visible');` isn't required anymore to access rule actions form.
Addresses: #158196
Summary
This PR mostly implements a test plan to cover rule snoozing functionality by automation tests.