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

[7.x] [Security Solution][Detections] Integration test for Editing a Rule (#77090) #77545

Merged
merged 1 commit into from
Sep 15, 2020

Conversation

rylnd
Copy link
Contributor

@rylnd rylnd commented Sep 15, 2020

Backports the following commits to 7.x:

…lastic#77090)

* Add cypress test around editing a detection rule

Right now this just navigates around and verifies that the form is
correctly repopulated; next step will be to modify/asset some changes.

* Add assertions for editing a rule

We already were asserting on the population of the Edit form after
creation; this additionally makes modifications, saves them, and asserts
the resulting values on the Rule Details page.

* Remove unused imports

* Inline our cypress expectations

So that expectation failures are less obfuscated, the decision was
previously made to abstract user navigation into functions, but to leave
expectations directly within the test body.

* Dynamically assert Rule Details based on titles

Rule Details are unfortunately unstructured: they're an array of <dt>s
and <dd>s without any hierarchy. To address this, tests
were previously hardcoding the order of these fields, and assertions
were performed by querying for all <dd>s and then indexing with the
hardcoded number (e.g. ABOUT_FALSE_POSITIVES).

However, in addition to being unstructured, these fields are also
_dynamic_, and will be present/absent depending on the data of the given
rule. Thus, we started needing multiple orderings for the different
combinations of rule fields/rule types.

In the absence of refactoring how we build rule details, I'm introducing
a simple helper function to fetch the relevant <dd> by the corresponding
<dt>s text. This should be more robust to change and more declarative.

* Fix bad merge conflict

Lots of these variables no longer exist upstream and this new test
needed to be refactored.

Co-authored-by: Elastic Machine <[email protected]>
@rylnd rylnd added the backport label Sep 15, 2020
@kibanamachine
Copy link
Contributor

💛 Build succeeded, but was flaky


Test Failures

re-orders columns via drag and drop.Events Viewer Events columns re-orders columns via drag and drop

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has failed 7 times on tracked branches: https://github.com/elastic/kibana/issues/70757

AssertionError: Timed out retrying: expected '@timestampmessagehost.nameevent.moduleevent.datasetevent.actionuser.namesource.ipdestination.ip' to equal 'message@timestamphost.nameevent.moduleevent.datasetevent.actionuser.namesource.ipdestination.ip'
    at Context.eval (http://localhost:6111/__cypress/tests?p=cypress/integration/events_viewer.spec.ts:876:61)

Build metrics

async chunks size

id value diff baseline
securitySolution 10.0MB +317.0B 10.0MB

distributable file count

id value diff baseline
default 47284 +2 47282

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

@rylnd rylnd merged commit d326247 into elastic:7.x Sep 15, 2020
@rylnd rylnd deleted the backport/7.x/pr-77090 branch September 15, 2020 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants