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 Solutions][Alert Details] Add cypress coverage for kpi charts #189758

Merged
merged 5 commits into from
Aug 8, 2024

Conversation

christineweng
Copy link
Contributor

Summary

Added cypress tests for navigation and some cypress clean up.

@christineweng christineweng added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0 labels Aug 1, 2024
@christineweng christineweng self-assigned this Aug 1, 2024
@christineweng
Copy link
Contributor Author

/ci

@christineweng christineweng marked this pull request as ready for review August 2, 2024 20:09
@christineweng christineweng requested a review from a team as a code owner August 2, 2024 20:09
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

I know we don't have a clear direction on this (or at least a written down one), but it seems to me that some of these tests could easily live as Jest tests. The added checks are mostly checking existence of DOM elements, and some attributes and classes...

They ressemble the expandable flyout Cypress tests, which I've been wanting to cleanup/remove for a while now, as they don't provide much value compared to the Jest tests...

Maybe @logeekal or @MadameSheema could chime up here.

I also understand that you're only adding to the existing tests, and going with Jest tests could be more involved and could be done as a separate effort entirely!

Finally, if we decide to keep this change as is (which is perfectly fine with me) we should run the Flaky Test Runner and post the result on the PR description.

@logeekal
Copy link
Contributor

logeekal commented Aug 6, 2024

I 100% agree with Phillipe all those exists assertion tests are worth moving to Jest and thereby avoiding the flakyness of Cypress. Either we can do it in this PR since most of the tests alerts_charts.cy.ts are re-written or I am also okay creating separate issue and then doing it there.

@christineweng christineweng requested a review from a team as a code owner August 6, 2024 16:38
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#6686

[✅] Security Solution Investigations - Cypress: 100/100 tests passed.
[✅] [Serverless] Security Solution Investigations - Cypress: 100/100 tests passed.

see run history

@christineweng christineweng removed the request for review from a team August 7, 2024 16:45
@christineweng christineweng enabled auto-merge (squash) August 8, 2024 19:00
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #78 / endpoint Endpoint permissions: when running with user/role [t1_analyst] should display endpoint data on Host Details

Metrics [docs]

✅ unchanged

History

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

cc @christineweng

@christineweng christineweng merged commit 92b8970 into elastic:main Aug 8, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants