-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[SIEM] [Cases] Case API tests #65777
Conversation
Pinging @elastic/siem (Team:SIEM) |
x-pack/test/case_api_integration/basic/tests/cases/comments/delete_comment.ts
Outdated
Show resolved
Hide resolved
await deleteCasesUserActions(es); | ||
}); | ||
|
||
it('should delete a comment', async () => { |
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 think we should add two tests that check error handling: 1) test if the comment does not exist 2) test if the comment does not belong to the specific case
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're actually not allowing this in the UI and could remove the endpoint entirely.... it's lucky to get one test
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.
That's true but users can call the API directly if they want to, so we should test the API independently of the UI, to make sure that the API is doing what we expect without considering the UI.
x-pack/test/case_api_integration/basic/tests/cases/comments/find_comments.ts
Outdated
Show resolved
Hide resolved
x-pack/test/case_api_integration/basic/tests/cases/comments/patch_comment.ts
Show resolved
Hide resolved
x-pack/test/case_api_integration/basic/tests/cases/comments/patch_comment.ts
Show resolved
Hide resolved
}); | ||
}); | ||
|
||
it('filters by tags', async () => { |
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.
Maybe we should at a test that filters by reporters or one that filters by both tags and reporters
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 need to figure out how to post from multiple users in the func tests to do this
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.
Very good job creating those integration tests! Thank you!
Some comments:
- I think we should test unhappy paths to the tests.
- User actions should be tested more. We can do this by either checking that the user action has been created on a specific route like (post_case) or by checking all possible user actions in
get_all_user_actions
test - Some of the routes depend of the connector id in some way. Shouldn't we test that?
skipping CI while i work on the user actions tests |
|
@elasticmachine merge upstream |
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.
Thank you for making all the changes!! The tests in this PR are amazing!
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* master: (46 commits) [Drilldowns][chore] Remove some any's from components. Remove `PlaceContext` from components (elastic#65854) [functional/services] import By/until from module (elastic#66015) [Drilldowns][IE] fix welcome bar layout in IE (elastic#65676) Inspect action shows on dashboard for every chart (elastic#65998) Fix heigt calc in calc issue for ie11 (elastic#66010) [Flights] Delay Bucket - Error notification on opening sample visualization (elastic#66028) [SIEM] [Security] unified code structure phase 0 (elastic#65965) [Maps] Organize layers into subfolders (elastic#65513) skip flaky suite (elastic#59849) Cleanup prefill and edit flow. (elastic#66105) Fix major severity service map ring colors (elastic#66124) [DOCS] Improves formatting in action types (elastic#65932) [DOCS] APM Agent config: Setting values must be string (elastic#65875) Change default cert age limit value. (elastic#65918) [DOCS] Removed saved object options (elastic#66072) [SIEM] [Cases] Case API tests (elastic#65777) Add example of of local plugin installation (elastic#65986) skip flaky suite (elastic#65741) [SIEM][Detections] Restrict ML rule modification to ML Admins (elastic#65583) [Reporting/Test] Add Functional test for download CSV (elastic#65401) ...
…ine-editor * 'master' of github.com:elastic/kibana: (37 commits) [APM] Correct relative paths in scripts (#66159) [Uptime] Enable deselection of stale filters (#65523) [Drilldowns][chore] Remove some any's from components. Remove `PlaceContext` from components (#65854) [functional/services] import By/until from module (#66015) [Drilldowns][IE] fix welcome bar layout in IE (#65676) Inspect action shows on dashboard for every chart (#65998) Fix heigt calc in calc issue for ie11 (#66010) [Flights] Delay Bucket - Error notification on opening sample visualization (#66028) [SIEM] [Security] unified code structure phase 0 (#65965) [Maps] Organize layers into subfolders (#65513) skip flaky suite (#59849) Cleanup prefill and edit flow. (#66105) Fix major severity service map ring colors (#66124) [DOCS] Improves formatting in action types (#65932) [DOCS] APM Agent config: Setting values must be string (#65875) Change default cert age limit value. (#65918) [DOCS] Removed saved object options (#66072) [SIEM] [Cases] Case API tests (#65777) Add example of of local plugin installation (#65986) skip flaky suite (#65741) ...
Pinging @elastic/security-solution (Team: SecuritySolution) |
Summary
Long overdue API integration tests for the case API
Checklist
Delete any items that are not applicable to this PR.