[Security Solution] Simple diff algorithm test plan #184484
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dplumlee
added
release_note:skip
8 tasks
|
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 |
8 tasks
|
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
reviewed
May 31, 2024
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
dplumlee
force-pushed
the
single-line-field-diff-test-plan
branch
from
12c3e16
to
87f239a
Compare
nikitaindik
approved these changes
Jun 3, 2024
|
@elasticmachine merge upstream |
banderror
reviewed
Jun 4, 2024
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
dplumlee
force-pushed
the
single-line-field-diff-test-plan
branch
from
bc6f22b
to
90c5bac
Compare
3 tasks
banderror
reviewed
Jun 6, 2024
...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
Comment on lines
+894
to
+900
| 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
Outdated
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
banderror
approved these changes
Jun 10, 2024
|
@elasticmachine merge upstream |
dplumlee
added a commit
that referenced
this pull request
Jun 13, 2024
) ## 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 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Adds test plan in accordance to #180158
Adds test cases for simple diff algorithm to prebuilt rules'
Installation and upgradetest plan