Skip to content

[Detection Engine] Chore: Remove jest snapshots containing mostly EUI components#219026

Merged
rylnd merged 3 commits intoelastic:mainfrom
rylnd:rylnd/remove_eui_snapshots
Apr 30, 2025
Merged

[Detection Engine] Chore: Remove jest snapshots containing mostly EUI components#219026
rylnd merged 3 commits intoelastic:mainfrom
rylnd:rylnd/remove_eui_snapshots

Conversation

@rylnd
Copy link
Contributor

@rylnd rylnd commented Apr 23, 2025 •
edited by kibanamachine
Loading

Summary

This removes some snapshot tests for some UI components related to Security Exceptions.

These tests serve one purpose: to fail when a component's markup changes. While this can be useful, in these cases the snapshots are mostly EUI components, which we trust (but verify) to change over time.

I had made a previous attempt to correct our testing of EUI in #192747, but the recent #218778 demonstrated that there are still issues.

Since these tests already contain other, more direct assertions, and since the components being tested are themselves mostly comprised of EUI components, the approach here is to simply remove the snapshot assertions themselves.

Checklist

@rylnd
Copy link
Contributor Author

rylnd commented Apr 23, 2025

/ci

rylnd added 2 commits April 24, 2025 11:33
@rylnd
Remove generic snapshot tests from component suite
f89be74 
These tests serve one purpose: to fail when a component's markup
changes. While this can be useful, in these cases the snapshots are
_mostly_ EUI components, which we trust (but verify) to change over
time.

I had made a previous attempt to correct our testing of EUI in elastic#192747,
but the recent elastic#218778 demonstrated that there are still issues.

Since these tests already contain other, more directed assertions, the
approach here is to simply remove the snapshot assertions.
@rylnd
Remove more snapshots for mostly EUI components
114d9ae 
See previous commit for more info; this applies the same logic/changes
to other exception-list-components that are clearly mainly testing EUI
components (as evidenced by the contents of the deleted snapshots).
@rylnd rylnd force-pushed the rylnd/remove_eui_snapshots branch from b437cd2 to 114d9ae Compare April 24, 2025 16:33
@rylnd
Copy link
Contributor Author

rylnd commented Apr 24, 2025

/ci

@rylnd rylnd added Team:Detection Engine Security Solution Detection Engine Area backport:all-open Backport to all branches that could still receive a release release_note:skip Skip the PR/issue when compiling release notes chore labels Apr 24, 2025
@rylnd rylnd marked this pull request as ready for review April 24, 2025 21:24
@rylnd rylnd requested a review from a team as a code owner April 24, 2025 21:24
@rylnd rylnd requested a review from vitaliidm April 24, 2025 21:24
@elasticmachine
Copy link
Contributor

elasticmachine commented Apr 24, 2025

Pinging @elastic/security-detection-engine (Team:Detection Engine)

@elasticmachine
Copy link
Contributor

elasticmachine commented Apr 29, 2025

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

@rylnd rylnd merged commit 01555e2 into elastic:main Apr 30, 2025
9 checks passed
@rylnd rylnd deleted the rylnd/remove_eui_snapshots branch April 30, 2025 15:15
@kibanamachine
Copy link
Contributor

kibanamachine commented Apr 30, 2025

Starting backport for target branches: 7.17, 8.17, 8.18, 8.19, 9.0

https://github.com/elastic/kibana/actions/runs/14758070841

@kibanamachine
Copy link
Contributor

kibanamachine commented Apr 30, 2025

💔 All backports failed

Status Branch Result
❌ 7.17 Backport failed because of merge conflicts
❌ 8.17 Backport failed because of merge conflicts
❌ 8.18 Backport failed because of merge conflicts
❌ 8.19 Backport failed because of merge conflicts
❌ 9.0 Backport failed because of merge conflicts

You might need to backport the following PRs to 9.0:
- [Investigations][Timeline] - Run Object.keys less frequently (#219629)
- [Watcher] Fix accessibility issues (#219082)
- Update EUI to v102.0.0 (#219482)
- Update rbush to v4 (main) (manual) (#219613)
- [O11y AI Assistant] Correct quotes in ES|QL queries for function arguments (#217680)
- Use new security URLs in doc link service (#219005)
- [APM] Fix Alerts environment query follow up (#219571)

Manual backport

To create the backport manually run:

node scripts/backport --pr 219026

Questions ?

Please refer to the Backport tool documentation

pborgonovi pushed a commit that referenced this pull request Apr 30, 2025
@rylnd @pborgonovi
[Detection Engine] Chore: Remove jest snapshots containing mostly EUI…
0f9a049 
… components (#219026)

## Summary

This removes some snapshot tests for some UI components related to
Security Exceptions.

These tests serve one purpose: to fail when a component's markup
changes. While this can be useful, in these cases the snapshots are
_mostly_ EUI components, which we trust (but verify) to change over
time.

I had made a previous attempt to correct our testing of EUI in
#192747, but the recent
#218778 demonstrated that there
are still issues.

Since these tests already contain other, more direct assertions, and
since the components being tested are themselves mostly comprised of EUI
components, the approach here is to simply remove the snapshot
assertions themselves.


### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 1, 2025
@kibanamachine
Copy link
Contributor

kibanamachine commented May 1, 2025

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 219026 locally
cc: @rylnd

6 similar comments
@kibanamachine
Copy link
Contributor

kibanamachine commented May 2, 2025

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 219026 locally
cc: @rylnd

@kibanamachine
Copy link
Contributor

kibanamachine commented May 5, 2025

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 219026 locally
cc: @rylnd

@kibanamachine
Copy link
Contributor

kibanamachine commented May 6, 2025

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 219026 locally
cc: @rylnd

@kibanamachine
Copy link
Contributor

kibanamachine commented May 7, 2025

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 219026 locally
cc: @rylnd

@kibanamachine
Copy link
Contributor

kibanamachine commented May 8, 2025

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 219026 locally
cc: @rylnd

@kibanamachine
Copy link
Contributor

kibanamachine commented May 9, 2025

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 219026 locally
cc: @rylnd

@rylnd rylnd added backport:skip This PR does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. backport:all-open Backport to all branches that could still receive a release labels May 9, 2025
@rylnd
Copy link
Contributor Author

rylnd commented May 9, 2025

This PR should only be relevant moving forward, so these backports aren't needed.

akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025
@rylnd @akowalska622
[Detection Engine] Chore: Remove jest snapshots containing mostly EUI…
639d8ff 
… components (elastic#219026)

## Summary

This removes some snapshot tests for some UI components related to
Security Exceptions.

These tests serve one purpose: to fail when a component's markup
changes. While this can be useful, in these cases the snapshots are
_mostly_ EUI components, which we trust (but verify) to change over
time.

I had made a previous attempt to correct our testing of EUI in
elastic#192747, but the recent
elastic#218778 demonstrated that there
are still issues.

Since these tests already contain other, more direct assertions, and
since the components being tested are themselves mostly comprised of EUI
components, the approach here is to simply remove the snapshot
assertions themselves.


### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vitaliidm vitaliidm vitaliidm approved these changes

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting chore release_note:skip Skip the PR/issue when compiling release notes Team:Detection Engine Security Solution Detection Engine Area v9.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rylnd @elasticmachine @kibanamachine @vitaliidm