[Security Solution] Simple diff algorithm test plan#184484
[Security Solution] Simple diff algorithm test plan#184484banderror merged 10 commits intoelastic:mainfrom
Conversation
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
|
/ci |
💔 Build FailedFailed CI StepsTo update your PR or re-run it, just comment with: cc @dplumlee |
|
@elasticmachine merge upstream |
|
Do we also want to add "base version missing" scenarios here ("-AA", "-AB")? As I understand, it might be the case if base version was removed from the Fleet package. UPD: Just noticed it's included as |
nikitaindik
left a comment
There was a problem hiding this comment.
Hey @dplumlee! I left a few comments. Please take a look when you get a chance.
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
12c3e16 to
87f239a
Compare
nikitaindik
left a comment
There was a problem hiding this comment.
Thanks for answering my questions @dplumlee! Approving.
|
@elasticmachine merge upstream |
banderror
left a comment
There was a problem hiding this comment.
@dplumlee Overall, LGTM 👍
- The set of scenarios looks comprehensive. I'd only extract one case into its own scenario.
- I think we're missing some assertions. I think these scenarios should cover more behavior in the app.
- I'd reword a few things.
I left some comments applied to a single scenario, but they actually apply to all of them. Let me know what you think.
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
bc6f22b to
90c5bac
Compare
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Outdated
Show resolved
Hide resolved
| And <field_name> field should not be returned from the `upgrade/_review` API endpoint | ||
| And <field_name> field should not be shown in the upgrade preview UI | ||
|
|
||
| Examples: | ||
| | field_name | base_version | current_version | target_version | | ||
| | name | "A" | "B" | "B" | | ||
| | risk_score | 1 | 2 | 2 | |
There was a problem hiding this comment.
Nit: Just want to point out that this case is similar to the one described in #180154, and we might want to revisit it later. But for now let's keep it as is.
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Show resolved
Hide resolved
...lution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
Show resolved
Hide resolved
|
@elasticmachine merge upstream |
) ## Summary Completes related tickets: #180160 and #180158 Switches fields to use the diff algorithms assigned to them in the related tickets Adds integration tests in accordance to #184484 for the `upgrade/_review` API endpoint for the simple diff algorithm. Also changes logic in the `upgrade/_review` API endpoint to return user customized fields in the diffs even if there was not an update for that field. This new logic is described in #180154. We filter out the fields that fall under this new logic so that they are only returned from the API but not displayed in the per-field rule diff flyout as the current UI cannot support them. They are utilized in testing logic and will be implemented in the UI at a later date ### Checklist Delete any items that are not applicable to this PR. - [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 - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
Adds test plan in accordance to #180158
Adds test cases for simple diff algorithm to prebuilt rules'
Installation and upgradetest plan