-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Unskip enrichments tests #171983
Merged
Merged
Unskip enrichments tests #171983
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
0ff1429
Unskip enrichments tests
nkhristinin 444a483
Merge branch 'main' into enrichment-cypress
kibanamachine b106b04
update mappings and remove lifecycle
nkhristinin a36a58e
Merge branch 'main' into enrichment-cypress
kibanamachine 738b65f
Merge branch 'main' into enrichment-cypress
kibanamachine edde6de
Remove comment
nkhristinin f450001
Merge branch 'main' into enrichment-cypress
kibanamachine 989b5d3
Merge branch 'main' into enrichment-cypress
kibanamachine c40f012
Merge branch 'main' into enrichment-cypress
kibanamachine 37800c9
Merge branch 'main' into enrichment-cypress
kibanamachine efe503d
Merge branch 'main' into enrichment-cypress
kibanamachine 8ea0b87
Remove duplicated test checks
nkhristinin 3afc42f
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please provide an explanation for the issue that caused the test failures, and why removing these ILM settings fixes them? Other developers (myself included; this one was tricky) would benefit greatly from your findings here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding, is that ILM is not supported in serverless, which mean they not supported in cypress tests, I put some related PR/error in description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another idea, that maybe it was partially fixed by this PR, when EA tests were separated to a different execution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nkhristinin it looks like link you provided to the error has expired. Can you please post something more permanent here for posterity? That way if this test fails again, we can validate whether the hypothesized ILM error is to blame.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RE the ILM hypothesis: in my investigation I saw failures in ESS as well, so I think that invalidates ILM as the underlying issue.
I also don't think ILM explains the behavior I was observing: the test fails due to the presence of unexpected data (specifically, historical risk scores).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, what was with the link, but I changed it to the link of failing build in this pr.
The serverless tests are failing because of ILM, I am not sure if it's related to your initial investigation, or if it's a new problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had not encountered that ILM error during my investigation, and since it's specific to serverless it cannot be the root cause of #169154.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's not the root cause of those issues. After more deep look into that, this is my understanding of the timeline:
We did have some flaky tests, which you investigate in this PR
After some time, EA tests were moved to separate executions
When I started to work on those tests, there were constant test failures in serverless, not flaky tests, which this PR if fixed.
Then I ran several flaky test runs, which were successful for enrichments, but had failures not related to this test case:
x100 - errors related to dashboard
x200 - errors related to dashboard
And then I saw that there was one more x200 test run, which had 1 error (test 175) for enrichment tests, but this error also not related to original issues, but something that happened in
before all
hookOriginal error:
New error:
Conclusion:
We have not yet seen that original issue, but there were other flaky tests, so I will continue to run the flaky test runner, and we should have more details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the last test run confirmed that it still flaky -