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

Consolidates Jest configuration files and scripts #82671

Merged
merged 6 commits into from
Nov 13, 2020

Conversation

tylersmalley
Copy link
Contributor

@tylersmalley tylersmalley commented Nov 4, 2020

Jest tests are currently organized into two main configuration files (src/dev/jest/config.js and x-pack/dev-tools/jest/create_jest_config.js). Both of these are similar, but very slightly due to previously being in separate repositories. This change consolidates the scripts referenced in those configs, moving them to the @kbn/test project.

Blocker to #72569

Reviews: This change will trigger a review from almost all teams, however considering the nature of these changes I don't believe that is necessary. These changes are only in tests, and mostly moving files around.

Overview:

  • Consolidates scripts from src/dev/jest, src/test_utils, and x-pack/test-utils into packages/kbn-test/src/jest
  • Common configuration options from src/dev/jest/config.js and x-pack/dev-tools/jest moved to packages/kbn-test/jest-preset.json.
  • OSS contained an alias for test_utils. Those aliases have been removed in favor of importing these utilities from @kbn/test/jest

@tylersmalley tylersmalley force-pushed the single-jest-config branch 4 times, most recently from 7d060b4 to b11528d Compare November 12, 2020 05:05
@tylersmalley tylersmalley changed the title Single jest config Centralize Jest configuration Nov 12, 2020
Tyler Smalley added 2 commits November 11, 2020 21:41
Jest tests are currently organized into main configuration files (src/dev/jest/config.js and x-pack/dev-tools/jest/create_jest_config.js). Both of these are similar, but very slightly due to  previously being in separate repositories. This change consolidates the scripts referenced in those configs and moves them to the `@kbn/test` project.

OSS contained an alias for `test_utils`. Those aliases have been removed in favor of importing these utilities from `@kbn/test/jest`

Blocker to elastic#72569

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley tylersmalley changed the title Centralize Jest configuration Consolidates Jest configuration Nov 12, 2020
@tylersmalley tylersmalley changed the title Consolidates Jest configuration Consolidates Jest configuration files and scripts Nov 12, 2020
@tylersmalley tylersmalley added Team:Operations Team label for Operations Team v7.11 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Nov 12, 2020
@tylersmalley tylersmalley marked this pull request as ready for review November 12, 2020 15:25
@tylersmalley tylersmalley requested review from a team as code owners November 12, 2020 15:25
@tylersmalley tylersmalley requested a review from a team November 12, 2020 15:25
@tylersmalley tylersmalley requested a review from a team as a code owner November 12, 2020 15:25
@tylersmalley tylersmalley requested a review from a team November 12, 2020 15:25
@tylersmalley tylersmalley requested review from a team as code owners November 12, 2020 15:25
@elasticmachine
Copy link
Contributor

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

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Nov 12, 2020
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

Alerting code changes LGTM

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

ML edits LGTM

Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

Maps changes LGTM
code review

Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

app arch changes LGTM

@spalger spalger added v7.11.0 and removed v7.11 labels Nov 12, 2020
"@kbn/utils": "link:../kbn-utils"
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please install the editorconfig plugin in your editor so that this doesn't keep getting flipped back in forth in every PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe we have come to a consensus on if we should have newlines or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I swapped it because the few I checked had trailing newlines, and I hate seeing the red notice in the Github UI. However, looks like we mostly don't use newlines so I will revert this one.

find packages -name "package.json" | xargs tail -n 1

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's not exactly enforced but the contributing guides says to install the editorconfig plugin and that config has a rule for package.json specifically (also npm/yarn don't add the extra newline IIRC) https://github.com/elastic/kibana/blob/master/.editorconfig#L11-L14

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently there is an editorconfig plugin for eslint, so if we wanted to pass those files through eslint too we could, but that's a separate issue.

Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

Security LGTM

Tyler Smalley added 4 commits November 12, 2020 14:07
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@tylersmalley
Copy link
Contributor Author

Have received quite a few reviews, but still, some remain. Per the description, I am going to move forward with merging due to the nature of the changes. If any issues are raised, I am happy to address them in a follow-up.

@tylersmalley tylersmalley merged commit aba2068 into elastic:master Nov 13, 2020
tylersmalley pushed a commit to tylersmalley/kibana that referenced this pull request Nov 13, 2020
Jest tests are currently organized into main configuration files (src/dev/jest/config.js and x-pack/dev-tools/jest/create_jest_config.js). Both of these are similar, but very slightly due to  previously being in separate repositories. This change consolidates the scripts referenced in those configs and moves them to the `@kbn/test` project.

OSS contained an alias for `test_utils`. Those aliases have been removed in favor of importing these utilities from `@kbn/test/jest`

Blocker to elastic#72569

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
tylersmalley pushed a commit that referenced this pull request Nov 13, 2020
Jest tests are currently organized into main configuration files (src/dev/jest/config.js and x-pack/dev-tools/jest/create_jest_config.js). Both of these are similar, but very slightly due to  previously being in separate repositories. This change consolidates the scripts referenced in those configs and moves them to the `@kbn/test` project.

OSS contained an alias for `test_utils`. Those aliases have been removed in favor of importing these utilities from `@kbn/test/jest`

Blocker to #72569

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 16, 2020
* master:
  [Security Solution][Detections] Adds framework for replacing API schemas (elastic#82462)
  [Enterprise Search] Share Loading component (elastic#83246)
  Consolidates Jest configuration files and scripts (elastic#82671)
  APM header changes (elastic#82870)
  [Security Solutions] Adds a default for indicator match custom query of *:* (elastic#81727)
  [Security Solution] Note 10k object paging limit on Endpoint list (elastic#82889)
  [packerCache] fix gulp usage, don't archive node_modules (elastic#83327)
  Upgrade Node.js to version 12 (elastic#61587)
  [Actions] Removing placeholders and updating validation messages on connector forms (elastic#82734)
  [Fleet] Rename ingest_manager_api_integration tests fleet_api_integration (elastic#83011)
  [DOCS] Updates Discover docs (elastic#82773)
  [ML] Data frame analytics: Adds map view (elastic#81666)
  enables actions scoped within the stack to register at Basic license (elastic#82931)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.