Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ML] Add API integration testing for AD annotations #73068

Merged
merged 10 commits into from
Jul 31, 2020
Merged

[ML] Add API integration testing for AD annotations #73068

merged 10 commits into from
Jul 31, 2020

Conversation

qn895
Copy link
Member

@qn895 qn895 commented Jul 23, 2020

[ML] Remove filter APIs## Summary

This PR adds API integration testing to the api/ml/annotations endpoints. Related to #63700

Coverage:

  • get annotations
  • create annotations
  • update annotations
  • delete annotations

Checklist

Delete any items that are not applicable to this PR.

@qn895 qn895 added Feature:Anomaly Detection ML anomaly detection v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels Jul 23, 2020
@qn895 qn895 self-assigned this Jul 23, 2020
@peteharverson peteharverson mentioned this pull request Jul 23, 2020
Closed
90 tasks
peteharverson
peteharverson approved these changes Jul 27, 2020
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@qn895
Merge remote-tracking branch 'upstream/master' into annotations-api-c…
e17518c 
…overage

# Conflicts:
#	x-pack/test/api_integration/apis/ml/index.ts
#	x-pack/test/functional/services/ml/api.ts
@pheyos
Copy link
Member

pheyos commented Jul 28, 2020 •
edited
Loading

Checking test stability in a flaky test runner job ...
=> No failure in 50 runs ✔️

pheyos
pheyos reviewed Jul 29, 2020
View reviewed changes
x-pack/test/api_integration/apis/ml/annotations/create_annotations.ts Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/annotations/create_annotations.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/annotations/create_annotations.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/annotations/create_annotations.ts Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/annotations/get_annotations.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/annotations/get_annotations.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/annotations/update_annotations.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/annotations/update_annotations.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/annotations/update_annotations.ts Outdated Show resolved Hide resolved
x-pack/test/functional/services/ml/api.ts Outdated Show resolved Hide resolved
pheyos
pheyos reviewed Jul 29, 2020
View reviewed changes
Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see some duplicated elements across the test files. Might be worth to pull out the commonly used job configs and annotations creation bodies and maybe even some methods, similar to what we have in the jobs area.

qn895 added 3 commits July 29, 2020 12:40
@qn895
[ML] Refactor job configs for annotations & add more checks
f45a447
@qn895
Merge remote-tracking branch 'upstream/master' into annotations-api-c…
59be85e 
…overage

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@qn895
[ML] Refactor createAnnotationRequestBody
46e1641
@qn895 qn895 requested a review from pheyos July 29, 2020 21:43
pheyos
pheyos reviewed Jul 30, 2020
View reviewed changes
Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looking good overall, just left two comments.

x-pack/test/api_integration/apis/ml/annotations/update_annotations.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/annotations/update_annotations.ts Outdated Show resolved Hide resolved
pheyos
pheyos approved these changes Jul 30, 2020
View reviewed changes
Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Thanks for adding these tests @qn895 ! 🎉

@qn895
Copy link
Member Author

qn895 commented Jul 30, 2020

@elasticmachine merge upstream

@qn895
Copy link
Member Author

qn895 commented Jul 31, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@qn895 qn895 merged commit 69844e4 into elastic:master Jul 31, 2020
@qn895 qn895 deleted the annotations-api-coverage branch July 31, 2020 16:22
@qn895 qn895 mentioned this pull request Jul 31, 2020
Merged
qn895 added a commit to qn895/kibana that referenced this pull request Jul 31, 2020
@qn895 @elasticmachine
[ML] Add API integration testing for AD annotations (elastic#73068)
d6d9311 
Co-authored-by: Elastic Machine <[email protected]>
@qn895 qn895 mentioned this pull request Jul 31, 2020
Merged
qn895 added a commit to qn895/kibana that referenced this pull request Jul 31, 2020
@qn895 @elasticmachine
[ML] Add API integration testing for AD annotations (elastic#73068)
e2130d5 
Co-authored-by: Elastic Machine <[email protected]>
qn895 added a commit that referenced this pull request Aug 2, 2020
@qn895 @elasticmachine
[ML] Add API integration testing for AD annotations (#73068) (#73974)
a8ed8f8 
Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
qn895 added a commit that referenced this pull request Aug 2, 2020
@qn895 @elasticmachine
[ML] Add API integration testing for AD annotations (#73068) (#73975)
e58eaf1 
Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peteharverson peteharverson peteharverson approved these changes

@pheyos pheyos pheyos approved these changes

@alvarezmelissa87 alvarezmelissa87 Awaiting requested review from alvarezmelissa87

Assignees

@qn895 qn895

Labels
Feature:Anomaly Detection ML anomaly detection :ml release_note:skip Skip the PR/issue when compiling release notes test-api-integration v7.9.0 v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@qn895 @elasticmachine @pheyos @kibanamachine @peteharverson