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.

Sign up for GitHub

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

Jump to bottom

[Security Solution] Fix Rule Details page unit test flakiness with business logic refactor #164825

Merged
merged 7 commits into from
Aug 28, 2023
Merged

[Security Solution] Fix Rule Details page unit test flakiness with business logic refactor #164825

merged 7 commits into from
Aug 28, 2023

Conversation

jpdjere
Copy link
Contributor

@jpdjere jpdjere commented Aug 25, 2023 •
edited
Loading

Fixes: #147064

Summary

  • rule_details_ui/pages/rule_details/index.test.tsx was too heavy and can fail by timeout
  • The unit test was mounting the whole rule details component to test:
    • redirection logic
    • which tabs are visible

This PR:

  • extracts that logic into two new hooks and one component:
    • useLegacyUrlRedirect (for redirection logic)
    • useRuleDetailsTabs (to calculate which tabs should be displayed)
    • LegacyUrlConflictCallOut
  • adds tests for the two hooks and the component, covering all the logic that was previously covered by the rule_details_ui/pages/rule_details/index.test.tsx file
  • deletes the rule_details_ui/pages/rule_details/index.test.tsx file

@jpdjere jpdjere changed the title Fix unit test rule details page [Security Solution] Fix Rule Details page unit test flakiness with refactor Aug 25, 2023
@jpdjere jpdjere self-assigned this Aug 25, 2023
@jpdjere jpdjere added test failed-test A test failure on a tracked branch, potentially flaky-test 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 v8.1.1 v8.10.0 v8.11.0 Feature:Rule Details Security Solution Detection Rule Details page labels Aug 25, 2023
@jpdjere jpdjere requested a review from maximpn August 25, 2023 15:45
@jpdjere jpdjere marked this pull request as ready for review August 25, 2023 15:46
@jpdjere jpdjere requested a review from a team as a code owner August 25, 2023 15:46
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@jpdjere jpdjere changed the title [Security Solution] Fix Rule Details page unit test flakiness with refactor [Security Solution] Fix Rule Details page unit test flakiness with business logic refactor Aug 25, 2023
@jpdjere jpdjere added release_note:skip Skip the PR/issue when compiling release notes and removed v8.1.1 labels Aug 25, 2023
@banderror
Copy link
Contributor

with business logic refactor

@jpdjere Thanks for the fix, but I think it would be too late/risky to backport this to 8.10 :)

@banderror banderror removed the v8.10.0 label Aug 25, 2023
maximpn
maximpn approved these changes Aug 28, 2023
View reviewed changes
Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

LGTM

@jpdjere jpdjere requested a review from a team as a code owner August 28, 2023 10:00
@jpdjere
Copy link
Contributor Author

jpdjere commented Aug 28, 2023

Files by Code Owner

elastic/security-detection-engine

  • x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/exception_item_card/meta.tsx
  • x-pack/plugins/security_solution/public/exceptions/components/list_details_link_anchor/index.tsx

WafaaNasr
WafaaNasr approved these changes Aug 28, 2023
View reviewed changes
Copy link
Contributor

@WafaaNasr WafaaNasr left a comment

Choose a reason for hiding this comment

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

LGTM

@jpdjere jpdjere force-pushed the fix-unit-test-rule-details-page branch from 6430a00 to 6914b9a Compare August 28, 2023 11:05
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #27 / Canvas Canvas app saved search in canvas "before all" hook in "saved search in canvas"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 4473 4476 +3

Async chunks

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

id before after diff
securitySolution 16.0MB 12.5MB -3.4MB
Unknown metric groups

async chunk count

id before after diff
securitySolution 49 50 +1

ESLint disabled in files

id before after diff
securitySolution 66 67 +1

ESLint disabled line counts

id before after diff
securitySolution 450 453 +3

Total ESLint disabled count

id before after diff
securitySolution 516 520 +4

History

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

cc @jpdjere

@jpdjere jpdjere merged commit 59845dc into elastic:main Aug 28, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 28, 2023
@jpdjere jpdjere deleted the fix-unit-test-rule-details-page branch August 28, 2023 15:07
@jpdjere jpdjere added the flake-docs Temp label to gather PRs used to create dev docs label Aug 30, 2023
@jpdjere jpdjere mentioned this pull request Oct 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maximpn maximpn maximpn approved these changes

@WafaaNasr WafaaNasr WafaaNasr approved these changes

Assignees

@jpdjere jpdjere

Labels
backport:skip This commit does not require backporting failed-test A test failure on a tracked branch, potentially flaky-test Feature:Rule Details Security Solution Detection Rule Details page flake-docs Temp label to gather PRs used to create dev docs 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 v8.11.0
Projects
None yet
Milestone
No milestone
7 participants
@jpdjere @elasticmachine @banderror @kibana-ci @maximpn @WafaaNasr @kibanamachine