[Security Solution] data_source field diff algorithm test plan#189669
Merged
dplumlee merged 7 commits intoelastic:mainfrom Aug 9, 2024
Merged
[Security Solution] data_source field diff algorithm test plan#189669dplumlee merged 7 commits intoelastic:mainfrom
data_source field diff algorithm test plan#189669dplumlee merged 7 commits intoelastic:mainfrom
Conversation
Contributor
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Contributor
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
Contributor
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
8 tasks
3 tasks
jpdjere
reviewed
Aug 5, 2024
...ution/docs/testing/test_plans/detection_response/prebuilt_rules/upgrade_review_algorithms.md
Outdated
Show resolved
Hide resolved
jpdjere
reviewed
Aug 5, 2024
...ution/docs/testing/test_plans/detection_response/prebuilt_rules/upgrade_review_algorithms.md
Outdated
Show resolved
Hide resolved
jpdjere
reviewed
Aug 5, 2024
...ution/docs/testing/test_plans/detection_response/prebuilt_rules/upgrade_review_algorithms.md
Outdated
Show resolved
Hide resolved
da35abf to
0c9ec16
Compare
nikitaindik
reviewed
Aug 6, 2024
Contributor
nikitaindik
left a comment
There was a problem hiding this comment.
Thanks @dplumlee! I have taken a look and left a couple comments. Will take a more thorough look tomorrow.
...ution/docs/testing/test_plans/detection_response/prebuilt_rules/upgrade_review_algorithms.md
Outdated
Show resolved
Hide resolved
...ution/docs/testing/test_plans/detection_response/prebuilt_rules/upgrade_review_algorithms.md
Outdated
Show resolved
Hide resolved
| Examples: | ||
| | algorithm | base_version | current_version | target_version | merged_version | | ||
| | data_source | {type: "index_patterns", "index_patterns": ["one", "two", "three"]} | {type: "index_patterns", "index_patterns": ["two", "one", "four"]} | {type: "index_patterns", "index_patterns": ["one", "two", "five"]} | {type: "index_patterns", "index_patterns": ["one", "two", "four", "five"]} | | ||
| | data_source | {type: "data_view", "data_view_id": "A"} | {type: "index_patterns", "index_patterns": ["one", "one", "two", "three"]} | {type: "index_patterns", "index_patterns": ["one", "two", "five"]} | {type: "index_patterns", "index_patterns": ["one", "two", "three", "five"]} | |
Contributor
There was a problem hiding this comment.
Is this extra "one" in current_version intentional?
Contributor
Author
There was a problem hiding this comment.
Yeah, it's similar to the scalar array diff algorithm where we test deduplication, etc., in the arrays. The tests aren't as exhaustive for this algorithm since we're reusing a lot of logic for array comparison and merging but it's still a good idea to test a little bit of it
nikitaindik
reviewed
Aug 7, 2024
Contributor
nikitaindik
left a comment
There was a problem hiding this comment.
Finished my review. Added a couple more comments. Please take a look when you can.
...ution/docs/testing/test_plans/detection_response/prebuilt_rules/upgrade_review_algorithms.md
Outdated
Show resolved
Hide resolved
...ution/docs/testing/test_plans/detection_response/prebuilt_rules/upgrade_review_algorithms.md
Show resolved
Hide resolved
jpdjere
reviewed
Aug 9, 2024
...ution/docs/testing/test_plans/detection_response/prebuilt_rules/upgrade_review_algorithms.md
Outdated
Show resolved
Hide resolved
jpdjere
reviewed
Aug 9, 2024
...ution/docs/testing/test_plans/detection_response/prebuilt_rules/upgrade_review_algorithms.md
Outdated
Show resolved
Hide resolved
nikitaindik
approved these changes
Aug 9, 2024
Contributor
nikitaindik
left a comment
There was a problem hiding this comment.
Thanks for addressing feedback @dplumlee! LGTM now.
Contributor
|
After that, all good from my side 👍 ✅ |
jpdjere
approved these changes
Aug 9, 2024
dplumlee
added a commit
that referenced
this pull request
Aug 9, 2024
…gorithm (#189744) ## Summary Completes #187659 Switches `data_source` fields to use the implemented diff algorithm assigned to them in #188874 Adds integration tests in accordance to #189669 for the `upgrade/_review` API endpoint for the `data_source` field diff algorithm. ### 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)
This was referenced Jan 31, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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
Related ticket: #187659
Adds test plan for diff algorithm for arrays of scalar values implemented here: #188874
For maintainers