[CI] Filter empty jest tests before grouping#242440
Conversation
1ad38bc to
92330cd
Compare
bb6314d to
b2b8aa4
Compare
|
Pinging @elastic/kibana-operations (Team:Operations) |
| const THIS_PATH = | ||
| 'src/platform/packages/shared/kbn-test/src/functional_test_runner/lib/config/run_check_ftr_configs_cli.ts'; | ||
|
|
||
| const IGNORED_FOLDERS = ['.buildkite/']; |
There was a problem hiding this comment.
this check was getting false positive on the newly added get_tests_from_config.ts
| set -euo pipefail | ||
|
|
||
| source .buildkite/scripts/common/util.sh | ||
| source .buildkite/scripts/bootstrap.sh |
There was a problem hiding this comment.
this is needed, so that the jest resolver can interpret our own plugins (such as @kbn/test)
| enabled?: Array<string | { [configPath: string]: { queue: string } }>; | ||
| } | ||
|
|
||
| function getEnabledFtrConfigs(patterns?: string[], solutions?: string[]) { |
There was a problem hiding this comment.
These functions are just moved to the bottom of the file, so the main function is first
| ignore: [...DISABLED_JEST_CONFIGS, '**/node_modules/**'], | ||
| }) | ||
| : []; | ||
| const jestUnitConfigs = await filterEmptyJestConfigs( |
There was a problem hiding this comment.
this is the core of the change, the rest is just readability refactors
| @@ -0,0 +1,79 @@ | |||
| /* | |||
There was a problem hiding this comment.
@elastic/appex-qa - moved this to another file, not to bloat that already big file, most of the functionality doesn't overlap, so I think it's cleaner this way
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where Jest unit test grouping via ci-stats became inefficient after empty test configs were excluded from execution. The main change filters out empty Jest configs before requesting grouping, which reduces parallel test groups from ~53 back to the optimal ~23.
Key changes:
- Introduced
filterEmptyJestConfigsfunction that uses Jest's internal APIs to detect configs with no test files - Moved shared utility functions (
getRequiredEnv,runBatchedPromises) to common location - Refactored Scout test grouping logic into dedicated module for better separation of concerns
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.buildkite/scripts/steps/test/pick_test_group_run_order.sh |
Added bootstrap step to support Jest internal utilities resolution |
.buildkite/scripts/steps/test/scout_test_run_builder.ts |
Updated to use refactored Scout grouping function and replaced process.cwd() with getKibanaDir() |
.buildkite/pipeline-utils/utils.ts |
Added shared utility functions getRequiredEnv and runBatchedPromises |
.buildkite/pipeline-utils/scout/pick_scout_test_group_run_order.ts |
Extracted Scout test grouping logic from main file into dedicated module |
.buildkite/pipeline-utils/scout/index.ts |
Export file for Scout utilities |
.buildkite/pipeline-utils/index.ts |
Added Scout module export |
.buildkite/pipeline-utils/ci-stats/pick_test_group_run_order.ts |
Refactored to use filterEmptyJestConfigs, moved helper functions to end of file, updated imports |
.buildkite/pipeline-utils/ci-stats/index.ts |
Added export for new get_tests_from_config module |
.buildkite/pipeline-utils/ci-stats/get_tests_from_config.ts |
New file implementing empty Jest config detection using Jest internal APIs |
src/platform/packages/shared/kbn-test/src/functional_test_runner/lib/config/run_check_ftr_configs_cli.ts |
Added filtering for .buildkite/ folders in config discovery |
src/platform/packages/shared/kbn-developer-toolbar/jest.config.js |
Fixed incorrect path reference from private to shared |
csr
left a comment
There was a problem hiding this comment.
LGTM, thanks for looking into the test config grouping and the fix!
|
/ci |
|
Starting backport for target branches: 8.19, 9.1, 9.2 https://github.com/elastic/kibana/actions/runs/19369827128 |
💚 Build Succeeded
Metrics [docs]
History
|
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
|
Skipping backport, as the cause wasn't backported |
## Summary It seems the jest unit test grouping through `ci-stats` is broken. It's probably because we no longer run and report empty test configs (after elastic#236549 we skip empty configs), thus CI stats will no longer be able to have an accurate guess as to how long they will take. Calculating with the default guessed config runtimes results in weird buckets. We went from ~23 jest unit test groups to ~53 (again, probably because the empty configs, that would run in a few seconds are counted as if they take a few minutes + overhead). This PR updates the `pick_test_group_run_order` logic, such that we no longer ask grouping for the configs that don't contain any test files to run. The result reset the jest unit parallel group count back to 23. For this, I'm using jest's internal utilities to search for test files, and had to bootstrap the step, so that the resolution doesn't break. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
## Summary It seems the jest unit test grouping through `ci-stats` is broken. It's probably because we no longer run and report empty test configs (after elastic#236549 we skip empty configs), thus CI stats will no longer be able to have an accurate guess as to how long they will take. Calculating with the default guessed config runtimes results in weird buckets. We went from ~23 jest unit test groups to ~53 (again, probably because the empty configs, that would run in a few seconds are counted as if they take a few minutes + overhead). This PR updates the `pick_test_group_run_order` logic, such that we no longer ask grouping for the configs that don't contain any test files to run. The result reset the jest unit parallel group count back to 23. For this, I'm using jest's internal utilities to search for test files, and had to bootstrap the step, so that the resolution doesn't break. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 4889045) # Conflicts: # .buildkite/pipeline-utils/ci-stats/pick_test_group_run_order.ts # .buildkite/pipeline-utils/scout/index.ts # .buildkite/pipeline-utils/scout/pick_scout_test_group_run_order.ts # .buildkite/pipelines/fips/fips_pipeline.ts # .buildkite/scripts/steps/test/pick_test_group_run_order.sh # src/platform/packages/shared/kbn-test-saml-auth/jest.config.js
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
Summary
It seems the jest unit test grouping through
ci-statsis broken. It's probably because we no longer run and report empty test configs (after #236549 we skip empty configs), thus CI stats will no longer be able to have an accurate guess as to how long they will take. Calculating with the default guessed config runtimes results in weird buckets. We went from ~23 jest unit test groups to ~53 (again, probably because the empty configs, that would run in a few seconds are counted as if they take a few minutes + overhead).This PR updates the
pick_test_group_run_orderlogic, such that we no longer ask grouping for the configs that don't contain any test files to run. The result reset the jest unit parallel group count back to 23.For this, I'm using jest's internal utilities to search for test files, and had to bootstrap the step, so that the resolution doesn't break.