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

[Security Solution] Replace EUI theme with mocks in jest suites #92462

Merged
merged 2 commits into from
Feb 25, 2021

Conversation

ecezalp
Copy link
Contributor

@ecezalp ecezalp commented Feb 23, 2021

Summary

Previously there were a large number of jest specs that utilized the
ThemeProvider (from styled-components package) to inject EUI themes
into the mounted components. The full EUI theme is almost never
necessary for unit tests as each tested component usually consumes no
more than a single field or two from the EUI theme. In certain cases,
the theme was not used at all. This change removes all unnecessary
ThemeProviders from the suites, and replaces the imported EUI theme
json files with mock themes customized for each tested component. With
this change, snapshots are now significantly smaller, and tests are
lighter.

Closes #64092.

Checklist

Testing Scenario

To ensure all tests are green, please cd into kibana/x-pack and run node scripts/jest.js security_solution

@ecezalp ecezalp requested a review from a team as a code owner February 23, 2021 16:51
@ecezalp ecezalp self-assigned this Feb 23, 2021
@ecezalp ecezalp added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Feb 23, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@ecezalp ecezalp added release_note:skip Skip the PR/issue when compiling release notes v7.13.0 labels Feb 23, 2021
Previously there were a large number of jest specs that utilized
the ThemeProvider (from styled-components package) to inject EUI
themes into the mounted components. The full EUI theme is almost
never necessary for unit tests as each tested component usually
consumes no more than a single field or two from the EUI theme.
In certain cases, the theme was not used at all. This change is
intended to remove all unnecessary ThemeProviders from the suites,
and replaces the imported EUI theme json files with mock themes
customized for each tested component. With this change, snapshots
are now significantly smaller, and tests are lighter.

Closes elastic#64092.
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @ecezalp

Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for paying down some tech debt here, it's much appreciated!

@@ -1,865 +1,161 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Paginated Table Component rendering it renders the default load more table 1`] = `
<ContextProvider
Copy link
Contributor

Choose a reason for hiding this comment

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

💯


.c3 {
padding: 16px;
background-color: #ece;
Copy link
Contributor

Choose a reason for hiding this comment

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

😉

@ecezalp ecezalp merged commit 6b91f48 into elastic:master Feb 25, 2021
@ecezalp ecezalp deleted the jest-remove-theme branch February 25, 2021 16:00
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 25, 2021
…tiple-searchable-snapshot-actions

* 'master' of github.com:elastic/kibana:
  [Rollup] Fix use of undefined value in JS import (elastic#92791)
  [ILM] Fix replicas not showing  (elastic#92782)
  [Event Log] Extended README.md with the documentation for a REST API and Start plugin contract. (elastic#92562)
  [XY] Enables page reload toast for the legacyChartsLibrary setting (elastic#92811)
  [Security Solution][Case] Improve hooks (elastic#89580)
  [Security Solution] Update wordings and breadcrumb for timelines page (elastic#90809)
  [Security Solution] Replace EUI theme with mocks in jest suites (elastic#92462)
  docs: ✏️ use correct heading level (elastic#92806)
  [ILM ] Fix logic for showing/hiding recommended allocation on Cloud (elastic#90592)
  [Security Solution][Detections] Pull gap detection logic out in preparation for sharing between rule types (elastic#91966)
  [core.savedObjects] Remove _shard_doc tiebreaker since ES now adds it automatically. (elastic#92295)
  docs: ✏️ fix links in embeddable plugin readme (elastic#92778)

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/searchable_snapshot_field/searchable_snapshot_field.tsx
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 1, 2021
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 92462 or prevent reminders by adding the backport:skip label.

1 similar comment
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 92462 or prevent reminders by adding the backport:skip label.

ecezalp added a commit to ecezalp/kibana that referenced this pull request Mar 2, 2021
…tic#92462)

Previously there were a large number of jest specs that utilized
the ThemeProvider (from styled-components package) to inject EUI
themes into the mounted components. The full EUI theme is almost
never necessary for unit tests as each tested component usually
consumes no more than a single field or two from the EUI theme.
In certain cases, the theme was not used at all. This change is
intended to remove all unnecessary ThemeProviders from the suites,
and replaces the imported EUI theme json files with mock themes
customized for each tested component. With this change, snapshots
are now significantly smaller, and tests are lighter.

Closes elastic#64092.
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 3, 2021
ecezalp added a commit that referenced this pull request Mar 3, 2021
…) (#93336)

Previously there were a large number of jest specs that utilized
the ThemeProvider (from styled-components package) to inject EUI
themes into the mounted components. The full EUI theme is almost
never necessary for unit tests as each tested component usually
consumes no more than a single field or two from the EUI theme.
In certain cases, the theme was not used at all. This change is
intended to remove all unnecessary ThemeProviders from the suites,
and replaces the imported EUI theme json files with mock themes
customized for each tested component. With this change, snapshots
are now significantly smaller, and tests are lighter.

Closes #64092.
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: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SIEM] Snapshot tests include Eui ThemeProvider
4 participants