Skip to content

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

Merged
ecezalp merged 2 commits intoelastic:masterfrom
ecezalp:jest-remove-theme
Feb 25, 2021
Merged

[Security Solution] Replace EUI theme with mocks in jest suites#92462
ecezalp merged 2 commits intoelastic:masterfrom
ecezalp:jest-remove-theme

Conversation

@ecezalp
Copy link
Copy Markdown
Contributor

@ecezalp ecezalp commented Feb 23, 2021 •
edited
Loading

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
Copy Markdown
Contributor

elasticmachine commented Feb 23, 2021

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
@ecezalp
[Security Solution] Replace EUI theme with mocks in jest suites
401aff9 
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.
@ecezalp ecezalp added v8.0.0 and removed v7.13.0 labels Feb 23, 2021
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 24, 2021

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @ecezalp

rylnd
rylnd approved these changes Feb 25, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@rylnd rylnd left a comment •
edited
Loading

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!

...security_solution/public/common/components/paginated_table/__snapshots__/index.test.tsx.snap
// Jest Snapshot v1, https://goo.gl/fbAQLP

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

@rylnd rylnd Feb 25, 2021

Choose a reason for hiding this comment

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

💯

...ement/pages/trusted_apps/view/components/trusted_apps_list/__snapshots__/index.test.tsx.snap

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

@rylnd rylnd Feb 25, 2021

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
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into ilm/allow-mul…
67568d9 
…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
Copy Markdown
Contributor

kibanamachine commented Mar 1, 2021

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
Copy Markdown
Contributor

kibanamachine commented Mar 2, 2021

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
@ecezalp
[Security Solution] Replace EUI theme with mocks in jest suites (elas…
77549f5 
…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.
@ecezalp ecezalp mentioned this pull request Mar 2, 2021
Merged
@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
@ecezalp
[Security Solution] Replace EUI theme with mocks in jest suites (#92462…
07537be 
…) (#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

Reviewers

@rylnd rylnd rylnd approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[SIEM] Snapshot tests include Eui ThemeProvider

4 participants

@ecezalp @elasticmachine @kibanamachine @rylnd