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

[Security Solution][Detections] increases coverage of bulk edit rules action according to test plan #141929

Merged

Conversation

vitaliidm
Copy link
Contributor

@vitaliidm vitaliidm commented Sep 27, 2022

Summary

  • adds missing tests according to test plan (internal document)
  • small refactoring of cypress tasks related to rule actions: create Email connector moved to common tasks from create_rule screen, as it used in bulk editing as well

@vitaliidm vitaliidm self-assigned this Sep 28, 2022
@vitaliidm vitaliidm added test-coverage issues & PRs for improving code test coverage 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 backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.5.0 v8.6.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 28, 2022
@vitaliidm vitaliidm marked this pull request as ready for review October 3, 2022 11:13
@vitaliidm vitaliidm requested a review from a team as a code owner October 3, 2022 11:13
@vitaliidm vitaliidm requested review from a team as code owners October 3, 2022 11:13
@vitaliidm vitaliidm requested a review from xcrzx October 3, 2022 11:13
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@vitaliidm vitaliidm marked this pull request as draft October 3, 2022 11:13
@vitaliidm vitaliidm marked this pull request as ready for review October 3, 2022 12:34
it('Overwrite rule actions in rules', () => {
const expectedActionFrequency = 'On each rule execution';

loadPrebuiltDetectionRulesFromHeaderBtn();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need prebuilt rules for this test case? Installation of all prebuilt rules is a heavy and error-prone operation that often fails due to timeouts. And that's not clear from the test body why we even need prebuilt rules here.

If prebuilt rules are required to check that the "Custom rules cannot be edited" modal is not displayed, I would isolate that in a separate test case and make an explicit check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not only to ensure no modal displayed, but also to ensure no errors happened when successfully applying both add and overwrite actions to custom and prebuilt rules

@vitaliidm
Copy link
Contributor Author

@xcrzx feedback has been addressed

@vitaliidm vitaliidm requested a review from xcrzx October 6, 2022 10:42
@vitaliidm
Copy link
Contributor Author

Files by Code Owner

elastic/security-detections-response

  • x-pack/plugins/security_solution/cypress/screens/common/controls.ts
  • x-pack/plugins/security_solution/cypress/screens/common/rule_actions.ts

elastic/security-detections-response-alerts

  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_actions.cy.ts
  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/custom_query_rule.cy.ts

elastic/security-detections-response-rules

  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_actions.cy.ts
  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/custom_query_rule.cy.ts
  • x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/forms/rule_actions_form.tsx
  • x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/use_bulk_actions.tsx
  • x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/details/index.tsx

elastic/security-solution

  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_actions.cy.ts
  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/custom_query_rule.cy.ts
  • x-pack/plugins/security_solution/cypress/screens/common/controls.ts
  • x-pack/plugins/security_solution/cypress/screens/common/rule_actions.ts
  • x-pack/plugins/security_solution/cypress/screens/create_new_rule.ts
  • x-pack/plugins/security_solution/cypress/screens/rule_details.ts
  • x-pack/plugins/security_solution/cypress/screens/rules_bulk_edit.ts
  • x-pack/plugins/security_solution/cypress/tasks/alerts_detection_rules.ts
  • x-pack/plugins/security_solution/cypress/tasks/api_calls/connectors.ts
  • x-pack/plugins/security_solution/cypress/tasks/api_calls/rules.ts
  • x-pack/plugins/security_solution/cypress/tasks/common/rule_actions.ts
  • x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts
  • x-pack/plugins/security_solution/cypress/tasks/edit_rule.ts
  • x-pack/plugins/security_solution/cypress/tasks/rule_details.ts
  • x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts
  • x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/forms/rule_actions_form.tsx
  • x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/use_bulk_actions.tsx
  • x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/details/index.tsx
  • x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts

elastic/security-threat-hunting

  • x-pack/plugins/security_solution/cypress/screens/common/controls.ts
  • x-pack/plugins/security_solution/cypress/screens/common/rule_actions.ts

@banderror
Copy link
Contributor

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 6.6MB 6.6MB +88.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @vitaliidm

@banderror banderror enabled auto-merge (squash) October 10, 2022 10:57
Copy link
Contributor

@xcrzx xcrzx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for addressing my comments 👍

@banderror banderror merged commit d07301f into elastic:main Oct 10, 2022
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 10, 2022
… action according to test plan (elastic#141929)

## Summary

- adds missing tests according to [test plan](https://docs.google.com/document/d/116x7ITTTJQ6cTiwaGK831_f6Ox7XB3qyLiHxC3Cmf8w/edit#heading=h.tzaw2977z8ue) (internal document)
- small refactoring of cypress tasks related to rule actions: create Email connector moved to common tasks from create_rule screen, as it used in bulk editing as well

(cherry picked from commit d07301f)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@vitaliidm vitaliidm deleted the detections/bulk-edit-rule-actions-tests branch October 10, 2022 12:42
kibanamachine added a commit that referenced this pull request Oct 10, 2022
… action according to test plan (#141929) (#142978)

## Summary

- adds missing tests according to [test plan](https://docs.google.com/document/d/116x7ITTTJQ6cTiwaGK831_f6Ox7XB3qyLiHxC3Cmf8w/edit#heading=h.tzaw2977z8ue) (internal document)
- small refactoring of cypress tasks related to rule actions: create Email connector moved to common tasks from create_rule screen, as it used in bulk editing as well

(cherry picked from commit d07301f)

Co-authored-by: Vitalii Dmyterko <[email protected]>
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022
… action according to test plan (elastic#141929)

## Summary

- adds missing tests according to [test plan](https://docs.google.com/document/d/116x7ITTTJQ6cTiwaGK831_f6Ox7XB3qyLiHxC3Cmf8w/edit#heading=h.tzaw2977z8ue) (internal document)
- small refactoring of cypress tasks related to rule actions: create Email connector moved to common tasks from create_rule screen, as it used in bulk editing as well
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022
… action according to test plan (elastic#141929)

## Summary

- adds missing tests according to [test plan](https://docs.google.com/document/d/116x7ITTTJQ6cTiwaGK831_f6Ox7XB3qyLiHxC3Cmf8w/edit#heading=h.tzaw2977z8ue) (internal document)
- small refactoring of cypress tasks related to rule actions: create Email connector moved to common tasks from create_rule screen, as it used in bulk editing as well
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes 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. test-coverage issues & PRs for improving code test coverage v8.5.0 v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants