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

Use EUI test environment build with Jest #55877

Merged
merged 23 commits into from
Mar 10, 2020
Merged

Conversation

thompsongl
Copy link
Contributor

@thompsongl thompsongl commented Jan 24, 2020

Summary

Set Jest's moduleNameMapper config to use EUI's new commonjs test environment build. This build has async & dynamic import functions transformed, removing the need for Kibana to do so. We've also mocked the EuiIcon component to solidify snapshot testing output. Instead of the full svg icon, snapshots will now get a div with relevant identifying attributes.

Sibling/dependent PR: elastic/eui#2698

Undoes the Jest environment changes introduced in #36316 when EUI started using dynamic imports.

Checklist

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

- [ ] This was checked for keyboard-only and screenreader accessibility

For maintainers

- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately

@streamich streamich added the Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. label Feb 7, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-design (Team:Design)

@thompsongl
Copy link
Contributor Author

Update:

We released the new test build as part of [email protected] this week. Once Kibana has been updated to use that version, I will update this PR to have more clear intentions.

@thompsongl
Copy link
Contributor Author

Snapshot test failures are as expected. I'll be updating snapshots next week to match new expected output for EuiIcon

@thompsongl
Copy link
Contributor Author

Waiting on a resolution to #58068 before this can move forward

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

The jest changes here look fine, but the large number of changed snapshots is concerning... Are there multiple changes we could decouple a bit? Why does this change to the config cause so many snapshots to update?

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

ML changes LGTM

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.

Love it! Security changes LGTM

Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

Stack monitoring changes LGTM!

Copy link
Contributor

@spalger spalger 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

@smith smith left a comment

Choose a reason for hiding this comment

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

APM changes look good!

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

@elastic/kibana-app-arch changes are only snapshots.
LGTM

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

LGTM for platform changes

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

LGTM for KibanaApp, 👍 for this Kibana snapshot diet

Copy link
Contributor

@cqliu1 cqliu1 left a comment

Choose a reason for hiding this comment

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

Canvas changes LGTM 👍

@thompsongl thompsongl requested a review from chandlerprall March 9, 2020 19:42
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Changes LGTM; jest configuration looks correct, and the changes to tests' selectors and their snapshots look like what we expect

@thompsongl
Copy link
Contributor Author

@elasticmachine merge upstream

@thompsongl
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@thompsongl thompsongl merged commit 4e52dee into elastic:master Mar 10, 2020
thompsongl added a commit to thompsongl/kibana that referenced this pull request Mar 10, 2020
* wip

* dir name

* src snapshot changes

* x-pack snapshots

* svg element lookups

* snapshot updates

Co-authored-by: Elastic Machine <[email protected]>
thompsongl added a commit that referenced this pull request Mar 10, 2020
* Use EUI test environment build with Jest (#55877)

* wip

* dir name

* src snapshot changes

* x-pack snapshots

* svg element lookups

* snapshot updates

Co-authored-by: Elastic Machine <[email protected]>

* yarn.lock update

Co-authored-by: Elastic Machine <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 11, 2020
* master:
  [ML] Transforms: Use EuiInMemoryTable instead of custom typed table. (elastic#59782)
  Alerting/fix flaky instance test (elastic#58994)
  ci: disable all Mocha rules for tape tests (elastic#59798)
  Fix UX in alerting UI forms when errors occur (elastic#59444)
  [DOCS] Updated and added jump tables (elastic#59774)
  [DOCS] Moved rolled up index content (elastic#59372)
  Regenerate core api docs (elastic#59814)
  [Lens] remove react warnings (elastic#59574)
  The scripts/backport.js file isn't an executable (elastic#59800)
  [Alerting] add more alert properties to action parameter templating (elastic#59718)
  [Design] Branding changes in Elastic to focus more towards the Elastic brand (elastic#58160)
  [SIEM] Adds 'Create new rule' Cypress test (elastic#59790)
  Updating svgo -> css-tree -> mdn-data, all so we get mdn-data > 2.0 (elastic#58913)
  Use EUI test environment build with Jest (elastic#55877)
  update typescript version in all packages to avoid warnings (elastic#59787)
  [SIEM] [Case] Insert timeline into case textarea (elastic#59586)
  [ML] Functional tests - stabilize saved search tests (elastic#59652)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.