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.

Sign up for GitHub

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

Jump to bottom

[SIEM] Snapshot tests include Eui ThemeProvider #64092

Closed
rylnd opened this issue Apr 21, 2020 · 1 comment · Fixed by #92462
Closed

[SIEM] Snapshot tests include Eui ThemeProvider #64092

rylnd opened this issue Apr 21, 2020 · 1 comment · Fixed by #92462
Labels
Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM

Comments

@rylnd
Copy link
Contributor

rylnd commented Apr 21, 2020

As evidenced in #62963, we have some snapshot tests that include the EUI theme.

Not only does this cause the snapshot to be large, it also introduces unnecessary noise and churn whenever EUI makes underlying changes. We should modify those (and any other) tests to uncouple from EUI implementation details as much as possible.

We made one pass of these on #58724, but we must have missed these.
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@rylnd rylnd mentioned this issue Apr 21, 2020
Merged
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Oct 27, 2020
@ecezalp ecezalp mentioned this issue Feb 23, 2021
Merged
1 task
ecezalp added a commit to ecezalp/kibana that referenced this issue 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 added a commit to ecezalp/kibana that referenced this issue Feb 24, 2021
@ecezalp
[Security Solution] Replace EUI theme with mocks in jest suites
bc21804 
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 added a commit that referenced this issue Feb 25, 2021
@ecezalp
[Security Solution] Replace EUI theme with mocks in jest suites (#92462)
6b91f48 
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.
ecezalp added a commit to ecezalp/kibana that referenced this issue 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 added a commit that referenced this issue 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
Assignees
No one assigned
Labels
Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Security Solution] Replace EUI theme with mocks in jest suites ecezalp/kibana
4 participants
@rylnd @MindyRS @elasticmachine @ecezalp