Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing test: adds an action to a snoozed rule - rule snoozing Rule editing page / actions tab adds an action to a snoozed rule #159349

Closed
Tracked by #161507 ...
kibanamachine opened this issue Jun 8, 2023 · 6 comments
Assignees
Labels
8.10 candidate failed-test A test failure on a tracked branch, potentially flaky-test 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.

Comments

@kibanamachine
Copy link
Contributor

kibanamachine commented Jun 8, 2023

A test failed on a tracked branch

AssertionError: Timed out retrying after 150000ms: Expected to find element: `[data-test-subj="alertActionAccordion-0"]`, but never found it.
    at Context.eval (webpack:///./e2e/detection_rules/rule_snoozing.cy.ts:186:38)

First failure: CI Build - main

@kibanamachine kibanamachine added the failed-test A test failure on a tracked branch, potentially flaky-test label Jun 8, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label Jun 8, 2023
@oatkiller oatkiller added Feature:Detection Rules Anything related to Security Solution's Detection Rules and removed needs-team Issues missing a team label labels Jun 8, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label Jun 8, 2023
@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@rylnd rylnd added the Team:Detection Rule Management Security Detection Rule Management Team label Jun 9, 2023
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jun 9, 2023
@rylnd
Copy link
Contributor

rylnd commented Jun 9, 2023

Skipped in #159352.

@oatkiller
Copy link
Contributor

I'm going to remove the team labels and use this issue to debug the team label assignment script.

@oatkiller oatkiller removed Feature:Detection Rules Anything related to Security Solution's Detection Rules Team:Detection Rule Management Security Detection Rule Management Team labels Jun 12, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label Jun 12, 2023
@maximpn maximpn added Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team and removed needs-team Issues missing a team label labels Jun 20, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@banderror banderror added v8.10.0 and removed v8.9.0 labels Jul 8, 2023
maximpn added a commit that referenced this issue Aug 11, 2023
**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.
@maximpn
Copy link
Contributor

maximpn commented Aug 11, 2023

The test was unskipped in #160037

@maximpn maximpn closed this as completed Aug 14, 2023
benakansara pushed a commit to benakansara/kibana that referenced this issue Aug 14, 2023
…ic#160037)

**Addresses:** elastic#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](elastic#158195) with [Rule Editing page refactoring PR](elastic#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.10 candidate failed-test A test failure on a tracked branch, potentially flaky-test 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.
Projects
None yet
Development

No branches or pull requests

6 participants