[Security Solution] Restructure Cypress tests and fix code ownership #153645

banderror · 2023-03-24T12:45:24Z

Summary

Restructure Cypress tests for the Detection Engine into meaningful folders that can clearly tell how they relate to certain features.

Each folder should be unambiguously owned by a single area team.

Update the CODEOWNERS file and assign each of the 3 area teams to the folders they own. Not only folders with tests should be assigned, but also folders with helper code:

cypress/e2e/...
cypress/screens/...
cypress/tasks/...

For example, we could have something like this:

cypress
    e2e
        detection_response -> @elastic/security-detections-response 
            rule_management -> @elastic/security-detection-rule-management
            rule_execution_logic -> @elastic/security-detection-engine 
            rule_exceptions -> @elastic/security-detection-engine 
            ...

The text was updated successfully, but these errors were encountered:

…61900) ## Summary A very small first piece to address #153645 - Starts adding new folder structure in cypress tests `detections_response/rules_management` - Adds `detections_response/rules_management/bulk_actions` - Reshuffled screens and tasks to now also have a corresponding `screens/rules_bulk_actions.ts` and `tasks/rules_bulk_actions.ts` - We chatted and decided to try to organize tests by subdomains rather than by teams. Anticipated upcoming changes will add something like: - `detections_response/rules_management` - `/bulk_actions.ts` - `/rules_table.ts` - `detections_response/prebuilt_rules` - `detections_response/rule_creation` - `detections_response/rule_details` - `detections_response/rule_edit` - `detections_response/exceptions` - `/shared_exception_lists` - `/alerts_table` - `/alerts_details` - `/rule_details` --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>

…astic#161900) ## Summary A very small first piece to address elastic#153645 - Starts adding new folder structure in cypress tests `detections_response/rules_management` - Adds `detections_response/rules_management/bulk_actions` - Reshuffled screens and tasks to now also have a corresponding `screens/rules_bulk_actions.ts` and `tasks/rules_bulk_actions.ts` - We chatted and decided to try to organize tests by subdomains rather than by teams. Anticipated upcoming changes will add something like: - `detections_response/rules_management` - `/bulk_actions.ts` - `/rules_table.ts` - `detections_response/prebuilt_rules` - `detections_response/rule_creation` - `detections_response/rule_details` - `detections_response/rule_edit` - `detections_response/exceptions` - `/shared_exception_lists` - `/alerts_table` - `/alerts_details` - `/rule_details` --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>

@yctercero

…n/cypress/e2e/detection_rules` folder (#162373) **Epic:** #153633 **Partially addresses:** #153645 ## Summary This PR builds upon #161900 and moves tests located in the `e2e/detection_rules` folder into `e2e/detection_response` and splits them into multiple sub-folders according to the Detection Engine subdomains we have. It also updates the CODEOWNERS file accordingly. <img width="451" alt="Screenshot 2023-07-25 at 21 03 08" src="https://github.com/elastic/kibana/assets/7359339/fb6052c9-3c5d-4547-98f1-61f44b9f7187"> ## Details Specifically, changes in this PR include: - The `e2e/detections_response` folder was renamed to `e2e/detection_response`. - The `e2e/detections_response/bulk_actions` folder became `e2e/detection_response/rule_management/rule_actions/bulk_actions`. - Cypress tests for rule types (which actually test rule creation for different rule types) were moved to `e2e/detection_response/rule_creation`. - The CODEOWNERS file was updated. Things not addressed in this PR: - No ownership was assigned for `e2e/detection_response/rule_actions`. Will need to figure this out with @yctercero. - No restructuring was done for `security_solution/cypress/screens` and `security_solution/cypress/tasks`. Will be done in follow-up PRs. - No refactoring was done for the tests themselves. Some of this work is also upcoming. The full file structure of the `detection_response` tests looks like this: <img width="452" alt="Screenshot 2023-07-25 at 21 03 44" src="https://github.com/elastic/kibana/assets/7359339/2b89c6d2-9f2d-4cf6-914f-a71c3fa93595">

@yctercero

…n/cypress/e2e/detection_rules` folder (elastic#162373) **Epic:** elastic#153633 **Partially addresses:** elastic#153645 ## Summary This PR builds upon elastic#161900 and moves tests located in the `e2e/detection_rules` folder into `e2e/detection_response` and splits them into multiple sub-folders according to the Detection Engine subdomains we have. It also updates the CODEOWNERS file accordingly. <img width="451" alt="Screenshot 2023-07-25 at 21 03 08" src="https://github.com/elastic/kibana/assets/7359339/fb6052c9-3c5d-4547-98f1-61f44b9f7187"> ## Details Specifically, changes in this PR include: - The `e2e/detections_response` folder was renamed to `e2e/detection_response`. - The `e2e/detections_response/bulk_actions` folder became `e2e/detection_response/rule_management/rule_actions/bulk_actions`. - Cypress tests for rule types (which actually test rule creation for different rule types) were moved to `e2e/detection_response/rule_creation`. - The CODEOWNERS file was updated. Things not addressed in this PR: - No ownership was assigned for `e2e/detection_response/rule_actions`. Will need to figure this out with @yctercero. - No restructuring was done for `security_solution/cypress/screens` and `security_solution/cypress/tasks`. Will be done in follow-up PRs. - No refactoring was done for the tests themselves. Some of this work is also upcoming. The full file structure of the `detection_response` tests looks like this: <img width="452" alt="Screenshot 2023-07-25 at 21 03 44" src="https://github.com/elastic/kibana/assets/7359339/2b89c6d2-9f2d-4cf6-914f-a71c3fa93595">

…astic#161900) ## Summary A very small first piece to address elastic#153645 - Starts adding new folder structure in cypress tests `detections_response/rules_management` - Adds `detections_response/rules_management/bulk_actions` - Reshuffled screens and tasks to now also have a corresponding `screens/rules_bulk_actions.ts` and `tasks/rules_bulk_actions.ts` - We chatted and decided to try to organize tests by subdomains rather than by teams. Anticipated upcoming changes will add something like: - `detections_response/rules_management` - `/bulk_actions.ts` - `/rules_table.ts` - `detections_response/prebuilt_rules` - `detections_response/rule_creation` - `detections_response/rule_details` - `detections_response/rule_edit` - `detections_response/exceptions` - `/shared_exception_lists` - `/alerts_table` - `/alerts_details` - `/rule_details` --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>

@yctercero

…n/cypress/e2e/detection_rules` folder (elastic#162373) **Epic:** elastic#153633 **Partially addresses:** elastic#153645 ## Summary This PR builds upon elastic#161900 and moves tests located in the `e2e/detection_rules` folder into `e2e/detection_response` and splits them into multiple sub-folders according to the Detection Engine subdomains we have. It also updates the CODEOWNERS file accordingly. <img width="451" alt="Screenshot 2023-07-25 at 21 03 08" src="https://github.com/elastic/kibana/assets/7359339/fb6052c9-3c5d-4547-98f1-61f44b9f7187"> ## Details Specifically, changes in this PR include: - The `e2e/detections_response` folder was renamed to `e2e/detection_response`. - The `e2e/detections_response/bulk_actions` folder became `e2e/detection_response/rule_management/rule_actions/bulk_actions`. - Cypress tests for rule types (which actually test rule creation for different rule types) were moved to `e2e/detection_response/rule_creation`. - The CODEOWNERS file was updated. Things not addressed in this PR: - No ownership was assigned for `e2e/detection_response/rule_actions`. Will need to figure this out with @yctercero. - No restructuring was done for `security_solution/cypress/screens` and `security_solution/cypress/tasks`. Will be done in follow-up PRs. - No refactoring was done for the tests themselves. Some of this work is also upcoming. The full file structure of the `detection_response` tests looks like this: <img width="452" alt="Screenshot 2023-07-25 at 21 03 44" src="https://github.com/elastic/kibana/assets/7359339/2b89c6d2-9f2d-4cf6-914f-a71c3fa93595">

) **Relates to:** #153645 ## Summary This PR makes refactoring of navigation utilities and constants to make it transparent and be able to specify correct ownership later on. ## Motivation Engineers need to add new tests and maintain the older ones. Base building blocks of this are constants and utility functions in particular URL constants and `visit()` and `visitWithoutDataRange()` functions. It turned out a simple `visit()` helper function also passes `timerange` as a query param while only some tests need it. Having it as a base utility function (as the name suggests) it also gets used in the new code while `visitWithoutDateRange()` should be preferred. On top of that URL constants are combined in one file without clear ownership and contain a mix of legacy and new urls with some parts looking outdated as navigating to the url causes redirecting to a specific page. Having only relevant URL constants in a common file will help to reduce confusion. As the next step constants should be split into files/folders with clear ownership. Also having `visit()` adding no extra params (besides common for ALL the tests) will make the intention clear. Whenever a time range is needed `visitWithTimeRange()` can be used (ideally accepting a time range defined in a test). And the same stays true for any other pages, e.g. rule details page can have a utility `visitRuleDetailsPage(id: string)` containing some general waiting logic so the following actions operate on a loaded page. ## Details As a step towards clearness and transparent ownership this PR performs refactoring of `x-pack/test/security_solution_cypress/cypress/urls/navigation.ts` and `x-pack/test/security_solution_cypress/cypress/tasks/login.ts` files. The following has been done - all url constants in `x-pack/test/security_solution_cypress/cypress/urls/navigation.ts` were checked and updated to remove duplications, avoid redirections and grouped - legacy urls were moved to the only one test using them to test compatibility (`x-pack/test/security_solution_cypress/cypress/e2e/urls/compatibility.cy.ts`) - `visit()` was renamed to `visitWithTimeRange()` - `visitWithoutDateRange()` was renamed to `visit()` - `visit()` was refactored to accept a query string ## Next steps It's expected teams decompose `x-pack/test/security_solution_cypress/cypress/urls/navigation.ts` into feature specific file(s)/folder(s) with assigned owners. There is no 100% chance a generic wait for a page to be loaded helper function meet requirements for each page. It makes sense to consider adding per feature `visitFeatureAPage()` helper function containing assertions for the page to be loaded. `visitRuleDetailsPage(id: string)` was added for this purpose while waiting for page to be loaded functionality is omitted in this PR to reduce a number of changes.

…tic#166201) **Relates to:** elastic#153645 ## Summary This PR makes refactoring of navigation utilities and constants to make it transparent and be able to specify correct ownership later on. ## Motivation Engineers need to add new tests and maintain the older ones. Base building blocks of this are constants and utility functions in particular URL constants and `visit()` and `visitWithoutDataRange()` functions. It turned out a simple `visit()` helper function also passes `timerange` as a query param while only some tests need it. Having it as a base utility function (as the name suggests) it also gets used in the new code while `visitWithoutDateRange()` should be preferred. On top of that URL constants are combined in one file without clear ownership and contain a mix of legacy and new urls with some parts looking outdated as navigating to the url causes redirecting to a specific page. Having only relevant URL constants in a common file will help to reduce confusion. As the next step constants should be split into files/folders with clear ownership. Also having `visit()` adding no extra params (besides common for ALL the tests) will make the intention clear. Whenever a time range is needed `visitWithTimeRange()` can be used (ideally accepting a time range defined in a test). And the same stays true for any other pages, e.g. rule details page can have a utility `visitRuleDetailsPage(id: string)` containing some general waiting logic so the following actions operate on a loaded page. ## Details As a step towards clearness and transparent ownership this PR performs refactoring of `x-pack/test/security_solution_cypress/cypress/urls/navigation.ts` and `x-pack/test/security_solution_cypress/cypress/tasks/login.ts` files. The following has been done - all url constants in `x-pack/test/security_solution_cypress/cypress/urls/navigation.ts` were checked and updated to remove duplications, avoid redirections and grouped - legacy urls were moved to the only one test using them to test compatibility (`x-pack/test/security_solution_cypress/cypress/e2e/urls/compatibility.cy.ts`) - `visit()` was renamed to `visitWithTimeRange()` - `visitWithoutDateRange()` was renamed to `visit()` - `visit()` was refactored to accept a query string ## Next steps It's expected teams decompose `x-pack/test/security_solution_cypress/cypress/urls/navigation.ts` into feature specific file(s)/folder(s) with assigned owners. There is no 100% chance a generic wait for a page to be loaded helper function meet requirements for each page. It makes sense to consider adding per feature `visitFeatureAPage()` helper function containing assertions for the page to be loaded. `visitRuleDetailsPage(id: string)` was added for this purpose while waiting for page to be loaded functionality is omitted in this PR to reduce a number of changes.

banderror · 2023-10-17T19:15:42Z

Restructuring and fixing ownership has been done for the tests themselves (e2e folder) for both D&R teams, but hasn't been done yet for the helper code (screens and tasks folders). We can address the leftover restructuring later in the next release cycles.

banderror mentioned this issue Mar 24, 2023

[Security Solution] Detection Engine Test Automation and Coverage #153633

Open

banderror self-assigned this Mar 24, 2023

yctercero added Team:Detection Engine Security Solution Detection Engine Area and removed Team:Detection Alerts Security Detection Alerts Area Team Team:Security Solution Platform Security Solution Platform Team labels May 13, 2023

banderror added 8.9 candidate and removed 8.8 candidate labels May 16, 2023

banderror added 8.10 candidate and removed 8.9 candidate labels Jul 6, 2023

banderror assigned yctercero Jul 8, 2023

yctercero mentioned this issue Jul 13, 2023

[Detection Response][Cypress] Reshuffle bulk action cypress tests #161900

Merged

banderror mentioned this issue Jul 25, 2023

[Security Solution] Restructure Cypress tests under security_solution/cypress/e2e/detection_rules folder #162373

Merged

banderror mentioned this issue Jul 31, 2023

[Security Solution] Identify Rule Management Cypress tests ownership #153656

Closed

banderror removed the 8.10 candidate label Aug 22, 2023

banderror added v8.10.0 8.11 candidate labels Aug 22, 2023

banderror assigned maximpn and unassigned banderror and yctercero Aug 31, 2023

banderror added 8.12 candidate and removed 8.11 candidate labels Sep 11, 2023

maximpn mentioned this issue Sep 13, 2023

[Security Solution] Refactor Cypress tests navigation utilities #166201

Merged

banderror unassigned maximpn Oct 17, 2023

banderror removed the 8.12 candidate label Oct 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Restructure Cypress tests and fix code ownership #153645

[Security Solution] Restructure Cypress tests and fix code ownership #153645

banderror commented Mar 24, 2023 •

edited

banderror commented Oct 17, 2023

[Security Solution] Restructure Cypress tests and fix code ownership #153645

[Security Solution] Restructure Cypress tests and fix code ownership #153645

Comments

banderror commented Mar 24, 2023 • edited

Summary

banderror commented Oct 17, 2023

banderror commented Mar 24, 2023 •

edited