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 tests for /filters and /calendars #72564

Merged
merged 8 commits into from
Jul 23, 2020
Merged

[ML] Add API integration tests for /filters and /calendars #72564

merged 8 commits into from
Jul 23, 2020

Conversation

qn895
Copy link
Member

@qn895 qn895 commented Jul 21, 2020 •
edited
Loading

Summary

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

Filters

  • GET /api/ml/filters
  • GET /api/ml/filters/{filterId}
  • PUT /api/ml/filters (create)
  • PUT /api/ml/filters/{filterId} (update)
  • DELETE /api/ml/filters/{filterId}

Calendars & calendar events

  • GET /api/ml/calendars
  • GET /api/ml/calendars/{filterId}
  • PUT /api/ml/calendars (create)
  • PUT /api/ml/calendars/{filterId} (update)
  • DELETE /api/ml/calendars/{filterId}

Checklist

Delete any items that are not applicable to this PR.

@qn895 qn895 added :ml 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 21, 2020
@qn895 qn895 self-assigned this Jul 21, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@qn895 qn895 added the v7.10.0 label Jul 21, 2020
@pheyos
Copy link
Member

pheyos commented Jul 21, 2020 •
edited
Loading

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

@peteharverson peteharverson mentioned this pull request Jul 21, 2020
Closed
90 tasks
peteharverson
peteharverson approved these changes Jul 21, 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.

A couple of minor comments, but otherwise LGTM.

x-pack/test/api_integration/apis/ml/calendars/update_calendars.ts Outdated
const updatedCalendar = getCalendarResult.body.calendars[0];
const updatedEvents = getEventsResult.body.events;

// TODO: need to implement update description in the API
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you want to do this as part of this PR?

x-pack/test/api_integration/apis/ml/filters/update_filters.ts Outdated
const validFilters = [
{
filterId: 'filter_power',
requestBody: { description: 'Test delete filter #1', items },
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy and paste error? Should these be Test update filter #1 etc?

pheyos
pheyos requested changes Jul 21, 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.

Thanks for adding these tests @qn895 !
Left a couple suggestions.

x-pack/test/functional/services/ml/api.ts Outdated Show resolved Hide resolved
x-pack/test/functional/services/ml/api.ts Outdated Show resolved Hide resolved
x-pack/test/functional/services/ml/api.ts Outdated Show resolved Hide resolved
x-pack/test/functional/services/ml/api.ts Outdated Show resolved Hide resolved
x-pack/test/functional/services/ml/api.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/filters/get_filters.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/calendars/update_calendars.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/filters/update_filters.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/filters/update_filters.ts Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/filters/update_filters.ts Outdated Show resolved Hide resolved
@qn895 qn895 requested a review from pheyos July 22, 2020 14:47
pheyos
pheyos reviewed Jul 22, 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.

Good update! Just left two comments around the calendar event check.

x-pack/test/api_integration/apis/ml/calendars/helpers.ts Outdated Show resolved Hide resolved
x-pack/test/api_integration/apis/ml/calendars/helpers.ts Show resolved Hide resolved
pheyos
pheyos approved these changes Jul 23, 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

@qn895
Copy link
Member Author

qn895 commented Jul 23, 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 6d480c7 into elastic:master Jul 23, 2020
@qn895 qn895 deleted the ml-api-filters-calendars-tests branch July 23, 2020 16:38
qn895 added a commit to qn895/kibana that referenced this pull request Jul 23, 2020
@qn895 @elasticmachine
[ML] Add API integration tests for /filters and /calendars (elastic#7…
2269e27 
…2564)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@qn895 qn895 mentioned this pull request Jul 23, 2020
Merged
qn895 added a commit to qn895/kibana that referenced this pull request Jul 23, 2020
@qn895 @elasticmachine
[ML] Add API integration tests for /filters and /calendars (elastic#7…
4d224d9 
…2564)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@qn895 qn895 mentioned this pull request Jul 23, 2020
Merged
qn895 added a commit that referenced this pull request Jul 24, 2020
@qn895 @elasticmachine
[ML] Add API integration tests for /filters and /calendars (#72564) (#…
590be0c 
…73094)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
qn895 added a commit that referenced this pull request Jul 24, 2020
@qn895 @elasticmachine
[ML] Add API integration tests for /filters and /calendars (#72564) (#…
db3cb40 
…73093)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pheyos pheyos pheyos approved these changes

@peteharverson peteharverson peteharverson approved these changes

@darnautov darnautov Awaiting requested review from darnautov

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