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.

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

[ML] Functional tests - fix and re-enable validation API tests #80617

Merged
merged 2 commits into from
Oct 15, 2020

Conversation

pheyos
Copy link
Member

@pheyos pheyos commented Oct 15, 2020

Summary

This PR fixes and re-enables the recently disabled job validation API tests that validate cardinalities.

Details

According to the docs, the ES cardinality agg is

A single-value metrics aggregation that calculates an approximate count of distinct values.

So we shouldn't validate exact values during tests and we just were lucky in the past that the tests delivered consistent results.

This PR introduces special handling for the cardinality_model_plot_high entry of the validation response body to not check for exact numbers.

Closes #80418

@pheyos pheyos added :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes test-api-integration v7.11.0 labels Oct 15, 2020
@pheyos pheyos self-assigned this Oct 15, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@pheyos
Copy link
Member Author

pheyos commented Oct 15, 2020

Checking test stability in a flaky test runner job ... no failure in 50 runs ✔️

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

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 255 to 257
modelPlotCardinality: 'WILL BE VALIDATED SEPARATELY BELOW',
text: 'WILL BE VALIDATED SEPARATELY BELOW',
status: 'WILL BE VALIDATED SEPARATELY BELOW',
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Could be a const?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 0974a29

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@pheyos pheyos merged commit 36df5a8 into elastic:master Oct 15, 2020
@pheyos pheyos deleted the reenable_cardinality_tests branch October 15, 2020 12:40
pheyos added a commit to pheyos/kibana that referenced this pull request Oct 15, 2020
…ic#80617)

This PR fixes and re-enables the recently disabled job validation API tests that validate cardinalities.
pheyos added a commit that referenced this pull request Oct 15, 2020
… (#80646)

This PR fixes and re-enables the recently disabled job validation API tests that validate cardinalities.
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 15, 2020
* master: (102 commits)
  [Resolver] Fix flaky test (elastic#80576)
  Update Security Solution Bug Report Template (elastic#80668)
  [Observability] Kibana home page Observability link pointing to `/landing` (elastic#80636)
  [APM] Update User Experience app callout code to reflect new name (elastic#80641)
  [APM] Add missing ML privileges (elastic#80553)
  [DOCS] Adds intro line to the ML plugin readme file (elastic#80631)
  [ML] Functional tests - fix and re-enable validation API tests (elastic#80617)
  remove non-existing dependency from uptime plugin (elastic#80623)
  [ML] Fix job selection flyout overflow (elastic#80621)
  Move dashboard code in codeowner files to canvas team (elastic#80345)
  [Security Solution][Detections] Update signals template if outdated and rollover indices (elastic#80019)
  Sort service list by TPM if health is not shown (elastic#80447)
  Add in cluster version for sec telemetry sender. (elastic#80545)
  [Usage Collection] Usage collection add saved objects client to collector fetch context (elastic#80554)
  Change tag from experimental to beta (elastic#80443)
  [Metrics UI] Inventory view cleanup (elastic#79881)
  [Security Solutions][Detection Engine] Critical bug where value lists were not operational (elastic#80368)
  [Security Solution] Fix networkTopNFlow search strategy response (elastic#80362)
  [build] Retry docker pull (elastic#80432)
  add template for Security Solution bugs (elastic#80574)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml release_note:skip Skip the PR/issue when compiling release notes test-api-integration v7.11.0 v8.0.0
Projects
None yet
5 participants