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

[Alerting] add functional tests for index threshold alertType #60597

Merged
merged 1 commit into from
Mar 19, 2020
Merged

[Alerting] add functional tests for index threshold alertType #60597

merged 1 commit into from
Mar 19, 2020

Conversation

pmuellr
Copy link
Member

@pmuellr pmuellr commented Mar 19, 2020 •
edited
Loading

Adds function tests for the alerting built-in alertType index threshold.

resolves #58902

@pmuellr pmuellr added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Mar 19, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

pmuellr
pmuellr commented Mar 19, 2020
View reviewed changes
x-pack/test/alerting_api_integration/common/lib/es_test_index_tool.ts
@@ -4,20 +4,18 @@
* you may not use this file except in compliance with the Elastic License.
*/

export const ES_TEST_INDEX_NAME = '.kibaka-alerting-test-data';
export const ES_TEST_INDEX_NAME = '.kibana-alerting-test-data';
Copy link
Member Author

Choose a reason for hiding this comment

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

been meaning to change this for a while :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ go kibaka!

pmuellr
pmuellr commented Mar 19, 2020
View reviewed changes
...tegration/spaces_only/tests/alerting/builtin_alert_types/index_threshold/create_test_data.ts
@@ -8,53 +8,50 @@ import { times } from 'lodash';
import { v4 as uuid } from 'uuid';
import { ESTestIndexTool, ES_TEST_INDEX_NAME } from '../../../../../common/lib';

// date to start writing data
export const START_DATE = '2020-01-01T00:00:00Z';
Copy link
Member Author

Choose a reason for hiding this comment

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

I realized when using this module again, that I had used "start date" as a parameter, but that the function writes documents from that date, and then more moving in the past. So, it's actually an "end date". Renamed things as appropriate, but other than that, the date logic is unchanged.

The functional change for this module is to make the number of groups written parameterized. Previously it was always 2, now you can pass in the number of groups to create.

@pmuellr pmuellr marked this pull request as ready for review March 19, 2020 15:39
@pmuellr pmuellr requested a review from a team as a code owner March 19, 2020 15:39
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

  • 💚 Build #34516 succeeded ff05aec8b146537837a9916f56ff1316309e994c
  • 💔 Build #34420 failed 0a84d33f43e7aaf552374793184e95cc2b9ac576

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

YulNaumenko
YulNaumenko approved these changes Mar 19, 2020
View reviewed changes
Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM. We need to create a proper issue to cover a UI part of index threshold.

@pmuellr
Copy link
Member Author

pmuellr commented Mar 19, 2020 •
edited
Loading

I've started a flaky test suite runner on this, as this seems like it could be flaky:

mikecote
mikecote approved these changes Mar 19, 2020
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

Code LGTM!

x-pack/test/alerting_api_integration/common/lib/es_test_index_tool.ts
private readonly es: any,
private readonly retry: any,
private readonly index: string = ES_TEST_INDEX_NAME
) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL, you can define right in the constructor!

Copy link
Member Author

Choose a reason for hiding this comment

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

Not often we get to do this, since we often pass an object with props as the constructor arg, and I don't think you can use this trick for that. But I try to use it wherever I can!

@pmuellr
Copy link
Member Author

pmuellr commented Mar 19, 2020

Flaky test results: all good!

image

@pmuellr pmuellr merged commit d5989e8 into elastic:master Mar 19, 2020
@pmuellr pmuellr mentioned this pull request Mar 19, 2020
Merged
pmuellr added a commit to pmuellr/kibana that referenced this pull request Mar 19, 2020
@pmuellr
[Alerting] add functional tests for index threshold alertType (elasti…
21e602f 
…c#60597)

resolves elastic#58902
pmuellr added a commit that referenced this pull request Mar 20, 2020
@pmuellr
[Alerting] add functional tests for index threshold alertType (#60597) (
c7df9c8 
#60707)

resolves #58902
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 20, 2020
@gmmorris
Merge branch 'master' into alerting/api-constant
377acfc 
* master: (52 commits)
  [SIEM] Fix types in rules tests (elastic#60736)
  [Alerting] prevent flickering when fields are updated in an alert (elastic#60666)
  License checks for actions plugin (elastic#59070)
  Implemented ability to clear and properly validate alert interval (elastic#60571)
  WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj (elastic#60568)
  [Maps] Update layer dependencies to NP (elastic#59585)
  [Discover] Remove StateManagementConfigProvider (elastic#60221)
  [ML] Listing all categorization wizard checks (elastic#60502)
  [Upgrade Assistant] First iteration of batch reindex docs (elastic#59887)
  [SIEM] Export timeline (elastic#58368)
  [SIEM] Add support for actions and throttle in Rules (elastic#59641)
  Fix ace a11y listener (elastic#60639)
  Add addInfo toast to core notifications service (elastic#60574)
  fix test description (elastic#60638)
  [SIEM] Cypress screenshots upload to google cloud (elastic#60556)
  [canvas/shareable_runtime] sync sass loaders with kbn/optimizer (elastic#60653)
  [SIEM] Fixes Modification of ML Rules (elastic#60662)
  [SIEM] [Case] Bulk status update, add comment avatar, id => title in breadcrumbs (elastic#60410)
  [Alerting] add functional tests for index threshold alertType (elastic#60597)
  [Ingest]EMT-248: add post action request handler and resources (elastic#60581)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 20, 2020
@gmmorris
Merge branch 'alerting/tls-warning' of github.com:gmmorris/kibana int…
f04878a 
…o alerting/tls-warning

* 'alerting/tls-warning' of github.com:gmmorris/kibana: (32 commits)
  [ML] Listing all categorization wizard checks (elastic#60502)
  [Upgrade Assistant] First iteration of batch reindex docs (elastic#59887)
  [SIEM] Export timeline (elastic#58368)
  [SIEM] Add support for actions and throttle in Rules (elastic#59641)
  Fix ace a11y listener (elastic#60639)
  Add addInfo toast to core notifications service (elastic#60574)
  fix test description (elastic#60638)
  [SIEM] Cypress screenshots upload to google cloud (elastic#60556)
  [canvas/shareable_runtime] sync sass loaders with kbn/optimizer (elastic#60653)
  [SIEM] Fixes Modification of ML Rules (elastic#60662)
  [SIEM] [Case] Bulk status update, add comment avatar, id => title in breadcrumbs (elastic#60410)
  [Alerting] add functional tests for index threshold alertType (elastic#60597)
  [Ingest]EMT-248: add post action request handler and resources (elastic#60581)
  Return incident's url (elastic#60617)
  [Endpoint] TEST: GET alert details - boundary test for first alert retrieval (elastic#60320)
  [ML] Transforms: Fix pivot preview table mapping. (elastic#60609)
  [Endpoint] Log random seed for sample data CLI to console (elastic#60646)
  Use common event model for determining if event is v0 or v1 (elastic#60667)
  Disables PR Project Assigner workflow
  [Reporting] Allow reports to be deleted in Management > Kibana > Reporting (elastic#60077)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikecote mikecote mikecote approved these changes

@YulNaumenko YulNaumenko YulNaumenko approved these changes

Assignees
No one assigned
Labels
backported Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[alerting] more functional tests for index threshold alertType
5 participants
@pmuellr @elasticmachine @kibanamachine @mikecote @YulNaumenko