-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Security Solution] Parallelize slow Rule Management tests #234930
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] Parallelize slow Rule Management tests #234930
Conversation
968bdf0 to
5ed1d9c
Compare
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
jbudz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this!
maximpn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nikitaindik Thanks for splitting our tests into smaller groups to make them running faster 🙏
To be honest I have concerns with merging common_fields and type_specific_fields dirs into a single fields directory. Common/type specific fields separation simplifies navigation and mirrors folders structure in API integration tests. We can keep the previous folder's structure and use the same approach you implemented in this PR by either providing the hardcoded number of groups or scan for folders starting with config_. There are Node.js libs to scan folders structure so it's not necessary to implement it from scratch.
Nit - It'd be nice to see tests run time comparison in before/after fashion.
...tion_tests__/rules_upgrade/upgrade_rule_after_preview/fields/base.jest.integration.config.js
Outdated
Show resolved
Hide resolved
| */ | ||
| function getTestsForConfig(configNumber) { | ||
| const groupIndex = configNumber - 1; | ||
| const configTestFiles = allTestFiles.filter((file, index) => index % totalGroups === groupIndex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation could be simplified by using Lodash's chunk utility. A test group may pick a chunk by number. It should work the same way as the current implementation but it's gonna be shorter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried doing it with chunk, but I think it turned out even a bit more complicated.
const configTestFiles = chunk(testFiles, Math.ceil(testFiles.length / totalGroups))[groupIndex];
Also one minor downside of the chunk approach is that if we later add more groups, it won't balance them nicely without writing additional code.
Like, let's say if you have 13 files and 4 groups, it'll distribute them like: [4, 4, 4, 1] and not [4, 3, 3, 3].
|
buildkite test this |
|
@maximpn I've refactored according to what we discussed. Please take another look. |
maximpn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nikitaindik Thanks for addressing my comments 🙏
I have some last thoughts though since field test files were moved under fields folder. It should be considered as an optional comment. We could do one of the following
- As field test files are under
fieldsfoldercommon_fieldsandtype_specific_fieldscould be renamed tocommonandtype_specific - It's still possible to preserve the previous folders structure by including
diff_view_options.test.tsandrule_upgrade_button.test.tsinto dynamic groups.
|
@maximpn Thanks for giving it another pass! 👍 I've renamed the dirs to get rid of the |
⏳ Build in-progress, with failures
Failed CI StepsHistory
cc @nikitaindik |
|
Starting backport for target branches: 8.19, 9.1 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
…34930) **Resolves: elastic#232644 This PR splits long-running API integration and Jest configs into smaller configs to reduce overall runtime. We can’t rely on Buildkite’s `parallelism` parameter here, since it’s only works for Cypress steps. ## Changes - **API Integration tests** - Split the prebuilt rules config into 3 smaller configs. - **Rule Upgrade Field Tests (Jest)** - Merged `common_fields` and `type_specific_fields` dirs into a single `fields` directory. - Replaced 2 configs with 4 smaller ones. - Implemented round-robin logic to evenly distribute tests across the 4 configs. - **Alternative**: manually distribute tests into numbered directories. - *Pros*: simpler setup. - *Cons*: requires deciding where each new test should live when adding new fields. - Reviewers, tell me if adding this distribution logic is an overkill in your opinion. (cherry picked from commit 928d4b2) # Conflicts: # x-pack/solutions/security/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/customization_enabled/index.ts
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…34930) **Resolves: elastic#232644 This PR splits long-running API integration and Jest configs into smaller configs to reduce overall runtime. We can’t rely on Buildkite’s `parallelism` parameter here, since it’s only works for Cypress steps. ## Changes - **API Integration tests** - Split the prebuilt rules config into 3 smaller configs. - **Rule Upgrade Field Tests (Jest)** - Merged `common_fields` and `type_specific_fields` dirs into a single `fields` directory. - Replaced 2 configs with 4 smaller ones. - Implemented round-robin logic to evenly distribute tests across the 4 configs. - **Alternative**: manually distribute tests into numbered directories. - *Pros*: simpler setup. - *Cons*: requires deciding where each new test should live when adding new fields. - Reviewers, tell me if adding this distribution logic is an overkill in your opinion. (cherry picked from commit 928d4b2) # Conflicts: # x-pack/solutions/security/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/customization_enabled/index.ts
…4930) (#235565) # Backport This will backport the following commits from `main` to `9.1`: - [[Security Solution] Parallelize slow Rule Management tests (#234930)](#234930) <!--- Backport version: 10.0.2 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Nikita Indik","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-09-18T13:27:31Z","message":"[Security Solution] Parallelize slow Rule Management tests (#234930)\n\n**Resolves: https://github.com/elastic/kibana/issues/232644**\n\nThis PR splits long-running API integration and Jest configs into\nsmaller configs to reduce overall runtime.\nWe can’t rely on Buildkite’s `parallelism` parameter here, since it’s\nonly works for Cypress steps.\n\n## Changes\n- **API Integration tests**\n - Split the prebuilt rules config into 3 smaller configs.\n\n- **Rule Upgrade Field Tests (Jest)**\n- Merged `common_fields` and `type_specific_fields` dirs into a single\n`fields` directory.\n - Replaced 2 configs with 4 smaller ones.\n- Implemented round-robin logic to evenly distribute tests across the 4\nconfigs.\n- **Alternative**: manually distribute tests into numbered directories.\n - *Pros*: simpler setup. \n- *Cons*: requires deciding where each new test should live when adding\nnew fields.\n- Reviewers, tell me if adding this distribution logic is an overkill in\nyour opinion.","sha":"928d4b224ffaadc6f171482f6b95e14e76da912d","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","backport:version","v9.2.0","v8.19.5","v9.1.5"],"title":"[Security Solution] Parallelize slow Rule Management tests","number":234930,"url":"https://github.com/elastic/kibana/pull/234930","mergeCommit":{"message":"[Security Solution] Parallelize slow Rule Management tests (#234930)\n\n**Resolves: https://github.com/elastic/kibana/issues/232644**\n\nThis PR splits long-running API integration and Jest configs into\nsmaller configs to reduce overall runtime.\nWe can’t rely on Buildkite’s `parallelism` parameter here, since it’s\nonly works for Cypress steps.\n\n## Changes\n- **API Integration tests**\n - Split the prebuilt rules config into 3 smaller configs.\n\n- **Rule Upgrade Field Tests (Jest)**\n- Merged `common_fields` and `type_specific_fields` dirs into a single\n`fields` directory.\n - Replaced 2 configs with 4 smaller ones.\n- Implemented round-robin logic to evenly distribute tests across the 4\nconfigs.\n- **Alternative**: manually distribute tests into numbered directories.\n - *Pros*: simpler setup. \n- *Cons*: requires deciding where each new test should live when adding\nnew fields.\n- Reviewers, tell me if adding this distribution logic is an overkill in\nyour opinion.","sha":"928d4b224ffaadc6f171482f6b95e14e76da912d"}},"sourceBranch":"main","suggestedTargetBranches":["8.19","9.1"],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/234930","number":234930,"mergeCommit":{"message":"[Security Solution] Parallelize slow Rule Management tests (#234930)\n\n**Resolves: https://github.com/elastic/kibana/issues/232644**\n\nThis PR splits long-running API integration and Jest configs into\nsmaller configs to reduce overall runtime.\nWe can’t rely on Buildkite’s `parallelism` parameter here, since it’s\nonly works for Cypress steps.\n\n## Changes\n- **API Integration tests**\n - Split the prebuilt rules config into 3 smaller configs.\n\n- **Rule Upgrade Field Tests (Jest)**\n- Merged `common_fields` and `type_specific_fields` dirs into a single\n`fields` directory.\n - Replaced 2 configs with 4 smaller ones.\n- Implemented round-robin logic to evenly distribute tests across the 4\nconfigs.\n- **Alternative**: manually distribute tests into numbered directories.\n - *Pros*: simpler setup. \n- *Cons*: requires deciding where each new test should live when adding\nnew fields.\n- Reviewers, tell me if adding this distribution logic is an overkill in\nyour opinion.","sha":"928d4b224ffaadc6f171482f6b95e14e76da912d"}},{"branch":"8.19","label":"v8.19.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.1","label":"v9.1.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
…34930) (#235567) # Backport This will backport the following commits from `main` to `8.19`: - [[Security Solution] Parallelize slow Rule Management tests (#234930)](#234930) <!--- Backport version: 10.0.2 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Nikita Indik","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-09-18T13:27:31Z","message":"[Security Solution] Parallelize slow Rule Management tests (#234930)\n\n**Resolves: https://github.com/elastic/kibana/issues/232644**\n\nThis PR splits long-running API integration and Jest configs into\nsmaller configs to reduce overall runtime.\nWe can’t rely on Buildkite’s `parallelism` parameter here, since it’s\nonly works for Cypress steps.\n\n## Changes\n- **API Integration tests**\n - Split the prebuilt rules config into 3 smaller configs.\n\n- **Rule Upgrade Field Tests (Jest)**\n- Merged `common_fields` and `type_specific_fields` dirs into a single\n`fields` directory.\n - Replaced 2 configs with 4 smaller ones.\n- Implemented round-robin logic to evenly distribute tests across the 4\nconfigs.\n- **Alternative**: manually distribute tests into numbered directories.\n - *Pros*: simpler setup. \n- *Cons*: requires deciding where each new test should live when adding\nnew fields.\n- Reviewers, tell me if adding this distribution logic is an overkill in\nyour opinion.","sha":"928d4b224ffaadc6f171482f6b95e14e76da912d","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","backport:version","v9.2.0","v8.19.5","v9.1.5"],"title":"[Security Solution] Parallelize slow Rule Management tests","number":234930,"url":"https://github.com/elastic/kibana/pull/234930","mergeCommit":{"message":"[Security Solution] Parallelize slow Rule Management tests (#234930)\n\n**Resolves: https://github.com/elastic/kibana/issues/232644**\n\nThis PR splits long-running API integration and Jest configs into\nsmaller configs to reduce overall runtime.\nWe can’t rely on Buildkite’s `parallelism` parameter here, since it’s\nonly works for Cypress steps.\n\n## Changes\n- **API Integration tests**\n - Split the prebuilt rules config into 3 smaller configs.\n\n- **Rule Upgrade Field Tests (Jest)**\n- Merged `common_fields` and `type_specific_fields` dirs into a single\n`fields` directory.\n - Replaced 2 configs with 4 smaller ones.\n- Implemented round-robin logic to evenly distribute tests across the 4\nconfigs.\n- **Alternative**: manually distribute tests into numbered directories.\n - *Pros*: simpler setup. \n- *Cons*: requires deciding where each new test should live when adding\nnew fields.\n- Reviewers, tell me if adding this distribution logic is an overkill in\nyour opinion.","sha":"928d4b224ffaadc6f171482f6b95e14e76da912d"}},"sourceBranch":"main","suggestedTargetBranches":["8.19","9.1"],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/234930","number":234930,"mergeCommit":{"message":"[Security Solution] Parallelize slow Rule Management tests (#234930)\n\n**Resolves: https://github.com/elastic/kibana/issues/232644**\n\nThis PR splits long-running API integration and Jest configs into\nsmaller configs to reduce overall runtime.\nWe can’t rely on Buildkite’s `parallelism` parameter here, since it’s\nonly works for Cypress steps.\n\n## Changes\n- **API Integration tests**\n - Split the prebuilt rules config into 3 smaller configs.\n\n- **Rule Upgrade Field Tests (Jest)**\n- Merged `common_fields` and `type_specific_fields` dirs into a single\n`fields` directory.\n - Replaced 2 configs with 4 smaller ones.\n- Implemented round-robin logic to evenly distribute tests across the 4\nconfigs.\n- **Alternative**: manually distribute tests into numbered directories.\n - *Pros*: simpler setup. \n- *Cons*: requires deciding where each new test should live when adding\nnew fields.\n- Reviewers, tell me if adding this distribution logic is an overkill in\nyour opinion.","sha":"928d4b224ffaadc6f171482f6b95e14e76da912d"}},{"branch":"8.19","label":"v8.19.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.1","label":"v9.1.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
…tests (#235729) Pipeline with the fix passing in MKI: https://buildkite.com/elastic/kibana-serverless-security-solution-quality-gate-rule-management/builds/3397 ## Summary This PR is a follow up to PR #234930 and fixes `ERROR Unable to find config file`. This error occurs when running Rule Management API integration tests in MKI pipelines ([example](https://buildkite.com/elastic/kibana-serverless-security-solution-quality-gate-rule-management/builds/3393#01996093-11c2-4107-84e3-7f0b61565cc7)). ## What causes the error PR #234930 split one of the FTR config files into 3 smaller files, but didn't split the corresponding npm script in [package.json](https://github.com/elastic/kibana/blob/main/x-pack/solutions/security/test/security_solution_api_integration/package.json). ## Changes - Added npm scripts to run new test configs in periodic and quality gate pipelines - Updated pipeline YMLs to use these new scripts - Removed unused npm scripts to run configs locally. We normally test locally by running `./x-pack/scripts/functional_tests_server --config=<something>`.
…34930) **Resolves: elastic#232644 This PR splits long-running API integration and Jest configs into smaller configs to reduce overall runtime. We can’t rely on Buildkite’s `parallelism` parameter here, since it’s only works for Cypress steps. ## Changes - **API Integration tests** - Split the prebuilt rules config into 3 smaller configs. - **Rule Upgrade Field Tests (Jest)** - Merged `common_fields` and `type_specific_fields` dirs into a single `fields` directory. - Replaced 2 configs with 4 smaller ones. - Implemented round-robin logic to evenly distribute tests across the 4 configs. - **Alternative**: manually distribute tests into numbered directories. - *Pros*: simpler setup. - *Cons*: requires deciding where each new test should live when adding new fields. - Reviewers, tell me if adding this distribution logic is an overkill in your opinion.
…tests (elastic#235729) Pipeline with the fix passing in MKI: https://buildkite.com/elastic/kibana-serverless-security-solution-quality-gate-rule-management/builds/3397 ## Summary This PR is a follow up to PR elastic#234930 and fixes `ERROR Unable to find config file`. This error occurs when running Rule Management API integration tests in MKI pipelines ([example](https://buildkite.com/elastic/kibana-serverless-security-solution-quality-gate-rule-management/builds/3393#01996093-11c2-4107-84e3-7f0b61565cc7)). ## What causes the error PR elastic#234930 split one of the FTR config files into 3 smaller files, but didn't split the corresponding npm script in [package.json](https://github.com/elastic/kibana/blob/main/x-pack/solutions/security/test/security_solution_api_integration/package.json). ## Changes - Added npm scripts to run new test configs in periodic and quality gate pipelines - Updated pipeline YMLs to use these new scripts - Removed unused npm scripts to run configs locally. We normally test locally by running `./x-pack/scripts/functional_tests_server --config=<something>`.
**Resolves: #232644 This PR splits long-running API integration and Jest configs into smaller configs to reduce overall runtime. We can’t rely on Buildkite’s `parallelism` parameter here, since it’s only works for Cypress steps. ## Changes - **API Integration tests** - Split the prebuilt rules config into 3 smaller configs. - **Rule Upgrade Field Tests (Jest)** - Merged `common_fields` and `type_specific_fields` dirs into a single `fields` directory. - Replaced 2 configs with 4 smaller ones. - Implemented round-robin logic to evenly distribute tests across the 4 configs. - **Alternative**: manually distribute tests into numbered directories. - *Pros*: simpler setup. - *Cons*: requires deciding where each new test should live when adding new fields. - Reviewers, tell me if adding this distribution logic is an overkill in your opinion.
…tests (#235729) Pipeline with the fix passing in MKI: https://buildkite.com/elastic/kibana-serverless-security-solution-quality-gate-rule-management/builds/3397 ## Summary This PR is a follow up to PR #234930 and fixes `ERROR Unable to find config file`. This error occurs when running Rule Management API integration tests in MKI pipelines ([example](https://buildkite.com/elastic/kibana-serverless-security-solution-quality-gate-rule-management/builds/3393#01996093-11c2-4107-84e3-7f0b61565cc7)). ## What causes the error PR #234930 split one of the FTR config files into 3 smaller files, but didn't split the corresponding npm script in [package.json](https://github.com/elastic/kibana/blob/main/x-pack/solutions/security/test/security_solution_api_integration/package.json). ## Changes - Added npm scripts to run new test configs in periodic and quality gate pipelines - Updated pipeline YMLs to use these new scripts - Removed unused npm scripts to run configs locally. We normally test locally by running `./x-pack/scripts/functional_tests_server --config=<something>`.
…34930) **Resolves: elastic#232644 This PR splits long-running API integration and Jest configs into smaller configs to reduce overall runtime. We can’t rely on Buildkite’s `parallelism` parameter here, since it’s only works for Cypress steps. ## Changes - **API Integration tests** - Split the prebuilt rules config into 3 smaller configs. - **Rule Upgrade Field Tests (Jest)** - Merged `common_fields` and `type_specific_fields` dirs into a single `fields` directory. - Replaced 2 configs with 4 smaller ones. - Implemented round-robin logic to evenly distribute tests across the 4 configs. - **Alternative**: manually distribute tests into numbered directories. - *Pros*: simpler setup. - *Cons*: requires deciding where each new test should live when adding new fields. - Reviewers, tell me if adding this distribution logic is an overkill in your opinion.
…tests (elastic#235729) Pipeline with the fix passing in MKI: https://buildkite.com/elastic/kibana-serverless-security-solution-quality-gate-rule-management/builds/3397 ## Summary This PR is a follow up to PR elastic#234930 and fixes `ERROR Unable to find config file`. This error occurs when running Rule Management API integration tests in MKI pipelines ([example](https://buildkite.com/elastic/kibana-serverless-security-solution-quality-gate-rule-management/builds/3393#01996093-11c2-4107-84e3-7f0b61565cc7)). ## What causes the error PR elastic#234930 split one of the FTR config files into 3 smaller files, but didn't split the corresponding npm script in [package.json](https://github.com/elastic/kibana/blob/main/x-pack/solutions/security/test/security_solution_api_integration/package.json). ## Changes - Added npm scripts to run new test configs in periodic and quality gate pipelines - Updated pipeline YMLs to use these new scripts - Removed unused npm scripts to run configs locally. We normally test locally by running `./x-pack/scripts/functional_tests_server --config=<something>`.
Resolves: #232644
This PR splits long-running API integration and Jest configs into smaller configs to reduce overall runtime.
We can’t rely on Buildkite’s
parallelismparameter here, since it’s only works for Cypress steps.Changes
API Integration tests
Rule Upgrade Field Tests (Jest)
diffable_rule_fields:common_fields->commontype_specific_fields->type_specific