Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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] Test plan for prebuilt rule customization #204888
[Security Solution] Test plan for prebuilt rule customization #204888
Changes from 1 commit
14c226b
c68f262
8c217b5
bb13567
7418f72
c6b7994
bb668b5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we say that it's a prebuilt rule asset whose "version" field matches the "version" of an installed rule? WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we have separate scenarios for
ruleSource
instead of checking it in every test scenario?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could put it in the assumptions section as we never use custom rules in the customization tests, wdyt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may add such ab assumption but it looks like an overkill since the test plan concerns prebuilt rules.
My comment is rather related to unnecessary test scenario explicitness. Users won't see
ruleSource
in UI. They rather seeModified
badge when the rule is modified. The scenario saysUser can edit a non-customized prebuilt rule from the rule edit page
. It literally means user may open rule editing form for a prebuilt rule, change some fields values and save the rule. After that they should see changed values on the rule details page andModified
badge. CheckingruleSource
on the way is test implementation details and looks optional.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it specify all the occurrences of
Modified
badge? Correct me if I'm wrong but we have it inInstalled Rules
table as well.Rule Update table is shown in case of updates but "Modified" is also shown there. And it's worth mention that in the test plan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do that here, we also have tests for the rule management table and update table themselves that have this test in them. Do we want to expand the scope of these tests to include the table as well as the rule details page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we have a separate test scenario to assert
Modified
badge appears onRules Update
table?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it verify the corresponding rule details pages as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again for this one, maybe we can just have one test specifically about the "modified" badge that checks all the locations in one test but for these it seems out of immediate scope wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified
badge is the only visual output here. I think it should stay.My point is that the test may double check bulk actions affect rule details page as well. Though I gave it a second thought and it looks rather optional.