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

[jest] refactor config check #135960

Merged
merged 3 commits into from
Jul 8, 2022
Merged

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Jul 7, 2022

We received a report from the core team that some of their tests were executing multiple times. This lead to discovering that the current config check improperly required that config files were included in places where they are not needed, causing many test files to be tested twice on CI.

The issue stems from an attempt to be helpful and tell contributors where config files should live. Unfortunately, as things have changed and we have needed to split up jest configs, this logic grew incorrect. I've tried to fix it several times but I'm tired of trying to make it work and don't think that jest config creation is something most people need a lot of help with anymore (especially once we get to packages for all the things).

This PR reworks that check to flip the logic around a bit, find all the jest test and config files in the repo, then use Jest APIs to determine the list of test files for each Jest config. Any test that isn't selected by a jest config will produce an error, and any test file that is selected by multiple configs will also produce an error.

IMO this is a much simpler thing to write correctly, and the logic seems sound to me for now.

Gist of the errors found by this PR before deleting config files: https://gist.github.com/spalger/be739eb4290756df4be83dd6b5410d51

@spalger spalger added Team:Operations Team label for Operations Team release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Jul 7, 2022
@spalger spalger requested review from a team as code owners July 7, 2022 22:42
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jul 8, 2022

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

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

@spalger spalger merged commit 1134d35 into elastic:main Jul 8, 2022
@spalger spalger deleted the fix/overlapping-jest-configs branch July 8, 2022 13:54
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.3 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.3:
- Bump Node.js to 16.16.0 (#135926)

Manual backport

To create the backport manually run:

node scripts/backport --pr 135960

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 11, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 135960 locally

spalger pushed a commit to spalger/kibana that referenced this pull request Jul 11, 2022
This was referenced Jul 11, 2022
spalger pushed a commit that referenced this pull request Jul 11, 2022
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 11, 2022
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:Operations Team label for Operations Team v8.3.3 v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants