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] Coverage overview test plan #165530

Merged
merged 8 commits into from
Sep 28, 2023

Conversation

dplumlee
Copy link
Contributor

@dplumlee dplumlee commented Sep 2, 2023

Summary

Addresses #162248

Adds the coverage overview test plan to the rule management test plan folder

Checklist

Delete any items that are not applicable to this PR.

  • Documentation was added for features that require explanation or tutorials

For maintainers

@dplumlee dplumlee added release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team v8.11.0 labels Sep 2, 2023
@dplumlee dplumlee self-assigned this Sep 2, 2023
@dplumlee dplumlee added the Feature:Rule Management Security Solution Detection Rule Management area label Sep 2, 2023
@dplumlee dplumlee marked this pull request as ready for review September 13, 2023 16:13
@dplumlee dplumlee requested a review from a team as a code owner September 13, 2023 16:13
@dplumlee dplumlee requested a review from maximpn September 13, 2023 16:13
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

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

@dplumlee dplumlee requested a review from vgomez-el September 18, 2023 14:19
Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

@dplumlee good job writing such a detailed test plan 👍

I left a number of comments where I have doubts or suggestions. Feel free to push back if you think I'm not right. My point is we should be specific we expect the dashboard is rendered and not just rule data as it's too generic. While the dashboard is rendered can be expanded in the Terminology section. It also good to know @vgomez-el's opinion.

I also found some misprints (according to my spell check plugin). I'm not sure if you use one but if not I recommend to install one.

@@ -0,0 +1,191 @@
# Coverage Overview Dashboard

This is a test plan for the Mitre Att&ck coverage overview dashboard
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Instead of giving an explanation sentence here it could be just a header part so the header'd look like Coverage Overview (Mitre Att&ck) Dashboard Test Plan

@dplumlee dplumlee requested a review from maximpn September 21, 2023 03:10
Copy link

@vgomez-el vgomez-el left a comment

Choose a reason for hiding this comment

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

In general the test plan looks great, great job @dplumlee. IMO, I am missing the following scenarios:

  • When user clicks on a tile, a popover is displayed, and the popover title links to the proper MITRE ATT&CK technique definition. (I know this can be tedious to automate/ check, but if we offer that functionality, we should assure that it works as expected)
  • Upgrade scenarios from versions 7.17 or lower 8.X versions to assure that upgraded rules are properly displayed on the dashboard
  • Expand/colapse cells behaviour and data properly displayed ( would be nice to include checks for small screens size behaviour)

I know the previous three points are difficult to automate, but at least we should add the scenarios to the test plan, even if they are not suitable for automation

@vgomez-el
Copy link

@dplumlee good job writing such a detailed test plan 👍

I left a number of comments where I have doubts or suggestions. Feel free to push back if you think I'm not right. My point is we should be specific we expect the dashboard is rendered and not just rule data as it's too generic. While the dashboard is rendered can be expanded in the Terminology section. It also good to know @vgomez-el's opinion.

I also found some misprints (according to my spell check plugin). I'm not sure if you use one but if not I recommend to install one.

I do agree with @maximpn with the dashboard is rendered can be expanded in the Terminology section to avoid repeating it too much on the scenarios, improve readability so we all are aligned on the meaning of that expression.

Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

@dplumlee thank you for addressing my comments

@dplumlee dplumlee enabled auto-merge (squash) September 28, 2023 16:31
@dplumlee dplumlee merged commit 6ac03d7 into elastic:main Sep 28, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 28, 2023
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 Feature:Rule Management Security Solution Detection Rule Management area release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants