[Security Solution] Improves detections tests #77295

MadameSheema · 2020-09-13T17:29:31Z

Summary

In this PR we are improving some of the the detections Cypress following tests with the following changes:

For Creates and activates a new rule test:
- The rule populates alerts
- We assert that the rule is generating alerts
- We assert that the values of the generated alert are the expected ones
For Creates and activates a new custom rule with override option:
- The rule populates alerts
- We assert that the rule is generating alerts
- We assert that the values of the generated alert are the expected ones taking into account the override functionality
For Creates and activates a new threshold rule:
The rule populates alerts
We assert that the rule is generating alerts
We assert that the values of the generated alert are the expected ones

Also in general we are trying to improve the readability.

MadameSheema · 2020-09-14T14:11:36Z

…n' test

# Conflicts: # x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts # x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_ml.spec.ts # x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_override.spec.ts # x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_threshold.spec.ts # x-pack/plugins/security_solution/cypress/screens/rule_details.ts # x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts

MadameSheema · 2020-09-17T13:24:51Z

@elasticmachine merge upstream

…into improves-detections

elasticmachine · 2020-09-17T19:35:50Z

Pinging @elastic/siem (Team:SIEM)

rylnd

There looks to be a bad merge conflict in there, so we should address that before merging this.

I had a few nits about lets and I still have a concern about synchronizing the rule execution, as well.

rylnd · 2020-09-18T19:12:50Z

x-pack/plugins/security_solution/cypress/integration/alerts.spec.ts

-          cy.get(SHOWING_ALERTS)
-            .invoke('text')
-            .should('eql', `Showing ${expectedNumberOfAlerts.toString()} alerts`);
+          cy.get(NUMBER_OF_ALERTS).should('have.text', expectedNumberOfAlerts.toString());


I assume this is the preference for text assertions, now? Is it more robust, or better errors, or both?

Yes this is the preference now when possible, is more robust because should clause in Cypress will continue to retry its specified assertions until it times out, and also improves the readability of the tests.

rylnd · 2020-09-18T19:18:06Z

x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts

+  cy.get(LOOK_BACK_TIME_TYPE).select(rule.lookBack.timeType);
+};
+
+export const expectDefineFormToRepopulateAndContinue = (rule: CustomRule) => {


I think these came from a bad merge resolution; they were removed in #77090

mmmm you are right! Thanks for catching this :)

rylnd · 2020-09-18T19:22:59Z

x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts

+    .invoke('text')
+    .then((ruleStatus) => {
+      if (ruleStatus !== 'succeeded') {
+        cy.get('[data-test-subj=refreshButton]').click();


Without the commented-out assertion below, there's nothing to synchronize the results of the refresh, right? Here's how I'm picturing it:

page is loaded, rule has not finished executing

we click refresh

We continue on with the test, with no guarantee that the rule has executed

I'll take a deeper look to this.

rylnd · 2020-09-18T19:26:01Z

x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_eql.spec.ts

+eqlRule.referenceUrls.forEach((url) => {
+  expectedUrls = expectedUrls + url;
+});
+let expectedFalsePositives = '';


If you're not expecting these values to change it would be nice use const with a .map or .reduce for these guys. I know these are copy/pasted from elsewhere, but I've got the const version in the EQL rule tests.

MadameSheema · 2020-09-20T18:20:49Z

@elasticmachine merge upstream

MadameSheema · 2020-09-24T10:38:05Z

@elasticmachine merge upstream

MadameSheema · 2020-09-24T15:31:07Z

@elasticmachine merge upstream

MadameSheema · 2020-09-24T16:16:12Z

@elasticmachine merge upstream

rylnd

This looks great! I had one minor quibble about the ordering of test assertions, but nothing that should block this. LGTM.

rylnd · 2020-09-24T15:59:52Z

x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts

-        // cy.get('[data-test-subj=ruleStatus]').should('have.text', 'succeeded')
-      }
-    });
+  cy.get(RULE_STATUS).should((ruleStatus) => {


🥇 💯 I was hoping cypress would have have a nice way to do this, but this is better than I expected!

rylnd · 2020-09-24T16:04:07Z

x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts

-      getDescriptionForTitle('Additional look-back time').invoke('text').should('eql', '1m');
+      getDetails(RUNS_EVERY_DETAILS).should(
+        'have.text',
+        `${newRule.runsEvery.interval}${newRule.runsEvery.type}`


👍 thanks for adding these scheduling fields!

rylnd · 2020-09-24T16:18:54Z

x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts


    goToRuleDetails();
+    waitForTheRuleToBeExecuted();


I would put this guard as close to the dependent assertions as possible: since (I think) it's only required for the alerts assertions, I would move this a bit further down.

MadameSheema · 2020-09-24T19:11:32Z

@elasticmachine merge upstream

MadameSheema · 2020-09-26T19:48:57Z

@elasticmachine merge upstream

…into improves-detections

MadameSheema · 2020-09-27T15:35:38Z

@elasticmachine merge upstream

MadameSheema · 2020-09-29T07:00:20Z

@elasticmachine merge upstream

kibanamachine · 2020-09-29T08:33:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: f683221

Metrics [docs]

async chunks size

id	value	diff	baseline
`securitySolution`	10.2MB	+38.0B	10.2MB

History

💔 Build #77458 failed 1dec5b2
💔 Build #77257 failed 3b8ca5e
💔 Build #77247 failed 2fe6861
💔 Build #77093 failed 26e6298
💔 Build #76985 failed da955b6

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

* improves 'Creates and activates a new custom rule' test * fixes constant problem * improves 'Creates and activates a new custom rule with override option' test * improves 'Creates and activates a new threshold rule' test * refactor * fixes type check issue * improves assertions * removes unused code * changes variables for constants * improves 'waitForTheRuleToBeExecuted' test * improves readability * fixes jenkins error * refactor * refactor Co-authored-by: Elastic Machine <[email protected]>

* master: (365 commits) making expression debug info serializable (elastic#78727) fix lodahs imports in app-arch code (elastic#78582) Make Field a React.lazy export (elastic#78483) [Security Solution] Improves detections tests (elastic#77295) [TSVB] Different field format on different series is ignored (elastic#78138) RFC: Improve saved object migrations (elastic#66056) [Security Solution] Fixes url timeline flaky test (elastic#78556) adds retryability feature (elastic#78611) Aligns several module versions across the repository (elastic#78327) Empty prompt and loading spinner for service map (elastic#78382) Change progress bar to spinner (elastic#78460) [QA][Code Coverage] Coverage teams lookup w/o Additional Config (elastic#77111) Slim down core bundle (elastic#75912) [Alerting] retry internal OCC calls within alertsClient (elastic#77838) [kbn/optimizer] only build xpack examples when building xpack plugins (elastic#78656) [Ingest Manager] Ingest setup upgrade (elastic#78081) [Ingest Manager] Surface saved object client 10,000 limitation to bulk actions UI (elastic#78520) fix name without a category or if field end with .text (elastic#78655) [Security Solution] [Detections] Log message enhancements (elastic#78429) [ML]DF Analytics exploration: default filter of results page by `defaultIsTraining` value in url (elastic#78303) ...

* improves 'Creates and activates a new custom rule' test * fixes constant problem * improves 'Creates and activates a new custom rule with override option' test * improves 'Creates and activates a new threshold rule' test * refactor * fixes type check issue * improves assertions * removes unused code * changes variables for constants * improves 'waitForTheRuleToBeExecuted' test * improves readability * fixes jenkins error * refactor * refactor Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

…-to-timeline * 'master' of github.com:elastic/kibana: (22 commits) update apm index pattern (elastic#78732) 78024: move transform out of dataset (elastic#78216) [QA][Code Coverage] Upload the coverage static site before ingestion (elastic#78695) [Discover] Make _source field not clickable (elastic#78698) [Fleet] Rename Ingest Manager => Fleet, Fleet => Agents in the UI (elastic#78685) [APM] Review feedback from distribution + transaction metrics (elastic#78752) [Ingest pipelines] Add ability to stop pipeline simulation (elastic#78183) [CSM] Fix core vital legend background (elastic#78273) [Usage Collection] [schema] Support spreads + `canvas` definition (elastic#78481) fix lodash imports (elastic#78456) [Maps] Add layer type preview icons (elastic#78650) [APM] Use transaction metrics for distribution charts (elastic#78484) [Uptime] Ml anomaly alert edit (elastic#76909) [ML] Limit exposing shared static code through ml/public/index.ts. (elastic#77745) making expression debug info serializable (elastic#78727) fix lodahs imports in app-arch code (elastic#78582) Make Field a React.lazy export (elastic#78483) [Security Solution] Improves detections tests (elastic#77295) [TSVB] Different field format on different series is ignored (elastic#78138) RFC: Improve saved object migrations (elastic#66056) ...

elasticmachine · 2021-09-23T14:25:56Z

Pinging @elastic/security-solution (Team: SecuritySolution)

MadameSheema force-pushed the improves-detections branch from 8c2af58 to 836b7cc Compare September 14, 2020 05:03

MadameSheema added 5 commits September 15, 2020 21:46

improves 'Creates and activates a new custom rule' test

ee00b9f

fixes constant problem

5e3f5d5

improves 'Creates and activates a new custom rule with override optio…

0b5afe1

…n' test

improves 'Creates and activates a new threshold rule' test

0c34e9b

MadameSheema force-pushed the improves-detections branch from afa7850 to d97ef1a Compare September 16, 2020 12:41

MadameSheema added 2 commits September 17, 2020 12:19

refactor

56c4c92

fixes type check issue

5ac8324

elasticmachine and others added 3 commits September 17, 2020 07:24

Merge branch 'master' into improves-detections

579fb54

improves assertions

9486073

Merge branch 'improves-detections' of github.com:MadameSheema/kibana …

f41a9e8

…into improves-detections

MadameSheema requested review from rylnd, spong, dhurley14, yctercero and FrankHassanabad September 17, 2020 19:34

MadameSheema self-assigned this Sep 17, 2020

MadameSheema added Team:SIEM v7.10.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 17, 2020

MadameSheema marked this pull request as ready for review September 17, 2020 19:35

MadameSheema requested review from a team as code owners September 17, 2020 19:35

rylnd requested changes Sep 18, 2020

View reviewed changes

MadameSheema added 2 commits September 23, 2020 21:29

improves 'waitForTheRuleToBeExecuted' test

6fe2d60

improves readability

905ee85

Merge branch 'master' into improves-detections

c061b7a

Merge branch 'master' into improves-detections

da955b6

Merge branch 'master' into improves-detections

6a2ec50

rylnd approved these changes Sep 24, 2020

View reviewed changes

elasticmachine and others added 2 commits September 24, 2020 15:11

Merge branch 'master' into improves-detections

26e6298

fixes jenkins error

492cf98

elasticmachine and others added 3 commits September 26, 2020 15:49

Merge branch 'master' into improves-detections

2fe6861

refactor

1e7dde8

Merge branch 'improves-detections' of github.com:MadameSheema/kibana …

3b8ca5e

…into improves-detections

refactor

1dec5b2

Merge branch 'master' into improves-detections

f683221

MadameSheema merged commit 7285026 into elastic:master Sep 29, 2020

MadameSheema deleted the improves-detections branch September 29, 2020 08:35

MadameSheema mentioned this pull request Sep 29, 2020

[7.x] [Security Solution] Improves detections tests (#77295) #78739

Merged

MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Improves detections tests #77295

[Security Solution] Improves detections tests #77295

MadameSheema commented Sep 13, 2020 •

edited

Loading

MadameSheema commented Sep 14, 2020

MadameSheema commented Sep 17, 2020

elasticmachine commented Sep 17, 2020

rylnd left a comment

rylnd Sep 18, 2020

MadameSheema Sep 20, 2020

rylnd Sep 18, 2020

MadameSheema Sep 20, 2020

rylnd Sep 18, 2020

MadameSheema Sep 20, 2020

rylnd Sep 18, 2020

MadameSheema commented Sep 20, 2020

MadameSheema commented Sep 24, 2020

MadameSheema commented Sep 24, 2020

MadameSheema commented Sep 24, 2020

rylnd left a comment

rylnd Sep 24, 2020

rylnd Sep 24, 2020

rylnd Sep 24, 2020

MadameSheema commented Sep 24, 2020

MadameSheema commented Sep 26, 2020

MadameSheema commented Sep 27, 2020

MadameSheema commented Sep 29, 2020

kibanamachine commented Sep 29, 2020

elasticmachine commented Sep 23, 2021

[Security Solution] Improves detections tests #77295

[Security Solution] Improves detections tests #77295

Conversation

MadameSheema commented Sep 13, 2020 • edited Loading

Summary

MadameSheema commented Sep 14, 2020

MadameSheema commented Sep 17, 2020

elasticmachine commented Sep 17, 2020

rylnd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MadameSheema commented Sep 20, 2020

MadameSheema commented Sep 24, 2020

MadameSheema commented Sep 24, 2020

MadameSheema commented Sep 24, 2020

rylnd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MadameSheema commented Sep 24, 2020

MadameSheema commented Sep 26, 2020

MadameSheema commented Sep 27, 2020

MadameSheema commented Sep 29, 2020

kibanamachine commented Sep 29, 2020

💚 Build Succeeded

Metrics [docs]

async chunks size

History

elasticmachine commented Sep 23, 2021

MadameSheema commented Sep 13, 2020 •

edited

Loading