Skip to content

[ML] fixing and unmuting testHRDSplit test #55349

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

Merged
merged 3 commits into from
Apr 17, 2020

Conversation

benwtrent
Copy link
Member

@benwtrent benwtrent commented Apr 16, 2020

This fixes the long muted testHRDSplit. Some minor adjustments for modern day elasticsearch changes :).

The cause of the failure is that a new by field entering the model with an exceptionally high count does not cause an anomaly. We have since stopped combining the rare and by in this manner. New entries in a by field are not anomalous because we have no history on them yet.

closes #32966

@benwtrent benwtrent added >test Issues or PRs that are addressing/adding tests :ml Machine learning v8.0.0 v7.8.0 labels Apr 16, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

@benwtrent benwtrent force-pushed the feature/ml-fix-test-hrd-test branch from 526afae to bda0117 Compare April 16, 2020 20:47
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

If you replace line 337:

    if (responseBody.contains("\"count\":2")) {

with:

    assertThat(responseBody, containsString("\"count\":2"));

then the else clause on lines 359-362 can be removed.

@benwtrent benwtrent requested a review from droberts195 April 17, 2020 11:46
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

@benwtrent
Copy link
Member Author

@elasticmachine update branch

@benwtrent benwtrent merged commit 7adbc35 into elastic:master Apr 17, 2020
@benwtrent benwtrent deleted the feature/ml-fix-test-hrd-test branch April 17, 2020 13:04
benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Apr 17, 2020
This fixes the long muted testHRDSplit. Some minor adjustments for modern day elasticsearch changes :). 

The cause of the failure is that a new `by` field entering the model with an exceptionally high count does not cause an anomaly. We have since stopped combining the `rare` and `by` in this manner. New entries in a `by` field are not anomalous because we have no history on them yet. 

closes elastic#32966
benwtrent added a commit that referenced this pull request Apr 17, 2020
This fixes the long muted testHRDSplit. Some minor adjustments for modern day elasticsearch changes :). 

The cause of the failure is that a new `by` field entering the model with an exceptionally high count does not cause an anomaly. We have since stopped combining the `rare` and `by` in this manner. New entries in a `by` field are not anomalous because we have no history on them yet. 

closes #32966
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml Machine learning >test Issues or PRs that are addressing/adding tests v7.8.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] PainlessDomainSplitIT fails after multibucket
4 participants