[7.10] [Security Solution][Detections] Fix EQL cypress tests (#80440)#81211
Merged
rylnd merged 1 commit intoelastic:7.10from Oct 20, 2020
Merged
[7.10] [Security Solution][Detections] Fix EQL cypress tests (#80440)#81211rylnd merged 1 commit intoelastic:7.10from
rylnd merged 1 commit intoelastic:7.10from
Conversation
* Unskip EQL tests
These _should_ be fixed with the latest ES on master, but let's see if
CI disagrees.
* Wait until alerts have populated on Rule Details
Occasionally our tests hit a scenario where the rule has executed (its
status is "succeeded"), but the generated alerts have not populated in
the same time frame. In this case the test fails oddly, saying that the
"alert count" element is not there when it is.
I attempted to improve the error message by using a .should() with a
callback, but that lead to even stranger behavior as the .should() would
fail once (expected), and then not be able to find the element a second
time. :(
So we instead focus on fixing the real problem, here: wait until alerts
populate (have a non-zero count) before performing the assertion.
Because the page will not update automatically, we can't rely on
cypress' retryability and must instead assert, click Refresh, and assert
again, much like we're doing while waiting for the rule to execute. And
like `waitForTheRuleToBeExecuted`, we're using a while loop that has no
guarantee of ever exiting :(
* More robust cypress assertions
* Uses should with a text matcher instead of using invoke('text')
* Use of not.equal between a string and an element may have been a false
positive
* Perform cypress loops in a manner guaranteed to exit
We have a few tasks that require polling for some background work to be
completed. The basic form is: assert the byproduct, or refresh the page
and try again.
We were previously doing this with a while loop, which was not
guaranteed to ever complete, leading to cryptic failures if the process
ever hung.
Instead, this implements a safer polling mechanism with a definite
termination similar to the cypress-wait-until plugin.
* Update other specs that are asserting on alerts
* Do not automatically refresh the page
* This is only necessary if we're not in the state we need. The
`waitFor` helper functions automatically reload whatever needs to be
reloaded, so we're delegating this task to them.
* Ensure we wait for alerts to be nonzero before our assertion
* Otherwise we get some strange behavior around this field's
availability; see previous commits
* Remove unused import
* Fix false positive in Rule Creation specs
Threat Match Rules introduced an additional query input, causing our
CUSTOM_QUERY_INPUT to be ambiguous.
However, instead of failing due to the ambiguity, the behavior of
cypress seems to be to pass! While I haven't yet tracked down the cause
of these false positives, disambiguating these selectors is the
immediate fix.
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
# x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_eql.spec.ts
Contributor
💚 Build SucceededMetrics [docs]
To update your PR or re-run it, just comment with: |
Contributor
Author
|
Mentioned there, but: this also backports #79287 to 7.10. |
spalger
pushed a commit
that referenced
this pull request
Oct 20, 2020
* Unskip EQL tests
These _should_ be fixed with the latest ES on master, but let's see if
CI disagrees.
* Wait until alerts have populated on Rule Details
Occasionally our tests hit a scenario where the rule has executed (its
status is "succeeded"), but the generated alerts have not populated in
the same time frame. In this case the test fails oddly, saying that the
"alert count" element is not there when it is.
I attempted to improve the error message by using a .should() with a
callback, but that lead to even stranger behavior as the .should() would
fail once (expected), and then not be able to find the element a second
time. :(
So we instead focus on fixing the real problem, here: wait until alerts
populate (have a non-zero count) before performing the assertion.
Because the page will not update automatically, we can't rely on
cypress' retryability and must instead assert, click Refresh, and assert
again, much like we're doing while waiting for the rule to execute. And
like `waitForTheRuleToBeExecuted`, we're using a while loop that has no
guarantee of ever exiting :(
* More robust cypress assertions
* Uses should with a text matcher instead of using invoke('text')
* Use of not.equal between a string and an element may have been a false
positive
* Perform cypress loops in a manner guaranteed to exit
We have a few tasks that require polling for some background work to be
completed. The basic form is: assert the byproduct, or refresh the page
and try again.
We were previously doing this with a while loop, which was not
guaranteed to ever complete, leading to cryptic failures if the process
ever hung.
Instead, this implements a safer polling mechanism with a definite
termination similar to the cypress-wait-until plugin.
* Update other specs that are asserting on alerts
* Do not automatically refresh the page
* This is only necessary if we're not in the state we need. The
`waitFor` helper functions automatically reload whatever needs to be
reloaded, so we're delegating this task to them.
* Ensure we wait for alerts to be nonzero before our assertion
* Otherwise we get some strange behavior around this field's
availability; see previous commits
* Remove unused import
* Fix false positive in Rule Creation specs
Threat Match Rules introduced an additional query input, causing our
CUSTOM_QUERY_INPUT to be ambiguous.
However, instead of failing due to the ambiguity, the behavior of
cypress seems to be to pass! While I haven't yet tracked down the cause
of these false positives, disambiguating these selectors is the
immediate fix.
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
# x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_eql.spec.ts
(cherry picked from commit 3fc1f8c)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backports the following commits to 7.10: