[Security Solution] Tests: Filter by rule execution status #160502

nikitaindik · 2023-06-26T10:05:00Z

Resolves: #138903

Summary

Adds an E2E Cypress test to check filtering by execution status in the rules table.

elasticmachine · 2023-06-26T14:05:42Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

elasticmachine · 2023-06-26T14:05:44Z

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

maximpn

@nikitaindik thank you for adding tests 👍 The changes look good to me, I just left some nit comments.

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

x-pack/plugins/security_solution/cypress/e2e/detection_rules/rule_fiters.cy.ts

x-pack/plugins/security_solution/cypress/tasks/api_calls/elasticsearch.ts

yctercero · 2023-07-03T22:11:18Z

Running this through the flakey test runner by the way! Just to make sure. 8.10 we're working through test flakes/failures so want to try and scrutinize (that sounds too harsh 😅 ) new tests a bit.

https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2547

yctercero

LGTM - we have rule failures in the flakey test runner but unrelated it seems. This test past all 50 runs.

banderror

LGTM overall @nikitaindik, I just left a few comments.

x-pack/plugins/security_solution/cypress/screens/alerts_detection_rules.ts

x-pack/plugins/security_solution/cypress/tasks/api_calls/elasticsearch.ts

x-pack/plugins/security_solution/cypress/tasks/api_calls/rules.ts

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

banderror · 2023-07-05T23:02:41Z

x-pack/plugins/security_solution/cypress/e2e/detection_rules/rule_fiters.cy.ts

+    deleteIndex('test_index');
+
+    createIndex('test_index', {
+      '@timestamp': {
+        type: 'date',
+      },
+    });
+
+    indexDocument('test_index', {});
+
+    createRule(
+      getNewRule({
+        name: 'Successful rule',
+        rule_id: 'successful_rule',
+        index: ['test_index'],
+      })
+    );
+
+    createRule(
+      getNewRule({
+        name: 'Warning rule',
+        rule_id: 'warning_rule',
+        index: ['non_existent_index'],
+      })
+    );
+
+    createRule(
+      getNewRule({
+        name: 'Failed rule',
+        rule_id: 'failed_rule',
+        index: ['test_index'],
+        // Setting a crazy large "Additional look-back time" to force a failure
+        from: 'now-9007199254746990s',
+      })
+    );
+
+    waitForRulesToFinishExecution(['successful_rule', 'warning_rule', 'failed_rule']);
+
+    visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
+
+    waitForRulesTableToBeLoaded();


I'm wondering if we need this in beforeEach -- were you thinking about expanding the rules you created with more properties in the future, to be able to test other filtering scenarios?

If we keep it as is, let's maybe also do some cleanup after the test:

afterEach(() => { deleteAlertsAndRules(); deleteIndex('test_index'); });

Thanks! I think it's a good idea to move non-common actions inside their respective it blocks.

Can please elaborate on why it would be beneficial to also clean up in afterEach? Wouldn't we do same actions twice then?

Can please elaborate on why it would be beneficial to also clean up in afterEach? Wouldn't we do same actions twice then?

@nikitaindik Yep, this is a good question. If we don't clean up, the rules we create in this test will continue to run in the next (and other) tests until a test cleans them up. This can have 2 downsides:

The tests left running after this test will be generating load on the system when other tests will be starting/running. This could increase flakiness.

If you ever need to debug those tests, you will see weird rule executions in the logs not related to the test you're debugging. This can be confusing.

banderror · 2023-07-05T23:03:59Z

x-pack/plugins/security_solution/cypress/e2e/detection_rules/rule_fiters.cy.ts

+        // Setting a crazy large "Additional look-back time" to force a failure
+        from: 'now-9007199254746990s',


Interesting! Just curious what failure does it cause?

Complains about invalid time value. Ideally the UI shouldn't allow sending invalid input, right?

Ideally the UI shouldn't allow sending invalid input, right?

@nikitaindik Yep. Neither the rule creation/editing UI nor the API should allow invalid parameter values. It would be great if you could open a bug ticket for this.

kibana-ci · 2023-07-06T16:40:45Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 6cd5f2b

Failed CI Steps

Test Failures

[job] [logs] Explore - Security Solution Tests #2 / Entity Analytics Dashboard With anomalies data enables a job
[job] [logs] Security Solution Tests #6 / Export rules shows a modal saying that no rules can be exported if all the selected rules are prebuilt

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	11.0MB	11.0MB	+146.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	14	16	+2
`securitySolution`	410	414	+4
total			+6

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	15	17	+2
`securitySolution`	489	493	+4
total			+6

History

💔 Build #140459 failed 664f0c7bc98cd2145d52dda8cd9f0135a0cb7a9a
💚 Build #139646 succeeded 494f880d934f337ddde4bb3cf7cd824a234a98f8
💔 Build #139467 failed ccb807fcf70bf03963512d44112cb906d4cda20f
💛 Build #138066 was flaky 985dc825404eaf9d59835a58ff871d224e9574e5
💔 Build #138028 failed b23c9648da1c8637b686bc07fa97eb1ffee4900e

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

cc @nikitaindik

banderror

Awesome, thanks for all the fixes @nikitaindik! 🎉
The only outstanding comment is the one about afterEach. Please merge at will if/when you address it.

…60502) **Resolves: elastic#138903 ## Summary Adds an E2E Cypress test to check filtering by execution status in the rules table. <img width="953" alt="Screenshot 2023-06-26 at 14 10 10" src="https://github.com/elastic/kibana/assets/15949146/e1eb67ed-779c-42ad-8194-04a26598cfbc"> (cherry picked from commit c30a7d4)

kibanamachine · 2023-07-11T08:21:32Z

💚 All backports created successfully

Status	Branch	Result
✅	8.9

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…0502) (#161618) # Backport This will backport the following commits from `main` to `8.9`: - [[Security Solution] Tests: Filter by rule execution status (#160502)](#160502)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Nikita Indik <[email protected]>

nikitaindik force-pushed the rule-execution-status-filter-e2e-tests branch from b23c964 to 802ab7f Compare June 26, 2023 12:08

nikitaindik self-assigned this Jun 26, 2023

nikitaindik mentioned this pull request Jun 26, 2023

[Security Solution] Add a filter by rule execution status to the Rules table #138903

Closed

4 tasks

nikitaindik marked this pull request as ready for review June 26, 2023 14:05

nikitaindik requested review from a team as code owners June 26, 2023 14:05

nikitaindik requested a review from maximpn June 26, 2023 14:05

banderror added test test_ui_functional labels Jun 27, 2023

maximpn approved these changes Jun 28, 2023

View reviewed changes

banderror reviewed Jun 28, 2023

View reviewed changes

x-pack/plugins/security_solution/cypress/tasks/api_calls/elasticsearch.ts Outdated Show resolved Hide resolved

nikitaindik force-pushed the rule-execution-status-filter-e2e-tests branch from 985dc82 to ccb807f Compare June 30, 2023 14:16

yctercero reviewed Jul 3, 2023

View reviewed changes

x-pack/plugins/security_solution/cypress/tasks/api_calls/elasticsearch.ts Show resolved Hide resolved

yctercero reviewed Jul 3, 2023

View reviewed changes

x-pack/plugins/security_solution/cypress/tasks/api_calls/elasticsearch.ts Outdated Show resolved Hide resolved

nikitaindik requested a review from yctercero July 4, 2023 13:55

yctercero approved these changes Jul 5, 2023

View reviewed changes

nikitaindik force-pushed the rule-execution-status-filter-e2e-tests branch from 494f880 to 664f0c7 Compare July 5, 2023 21:16

banderror reviewed Jul 5, 2023

View reviewed changes

nikitaindik added 2 commits July 6, 2023 16:49

Basic tests

b4c4a1a

Fix types in E2E tests

2ffff92

nikitaindik added 4 commits July 6, 2023 16:49

Refactor tests a bit

bdec692

Fix test

fca49f9

Simplify waitForRulesToFinishExecution helper function

ec35816

Make the test more readable

6cd5f2b

nikitaindik force-pushed the rule-execution-status-filter-e2e-tests branch from 664f0c7 to 6cd5f2b Compare July 6, 2023 14:49

banderror approved these changes Jul 7, 2023

View reviewed changes

banderror merged commit c30a7d4 into elastic:main Jul 11, 2023

kibanamachine added the v8.10.0 label Jul 11, 2023

kibanamachine mentioned this pull request Jul 11, 2023

[8.9] [Security Solution] Tests: Filter by rule execution status (#160502) #161618

Merged

kibanamachine mentioned this pull request Jul 11, 2023

[Security Solution] Move test plans from /cypress to /docs #161517

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Tests: Filter by rule execution status #160502

[Security Solution] Tests: Filter by rule execution status #160502

nikitaindik commented Jun 26, 2023 •

edited by kibanamachine

Loading

elasticmachine commented Jun 26, 2023

elasticmachine commented Jun 26, 2023

maximpn left a comment

yctercero commented Jul 3, 2023

yctercero left a comment

banderror left a comment

banderror Jul 5, 2023

nikitaindik Jul 6, 2023

banderror Jul 7, 2023

banderror Jul 5, 2023

nikitaindik Jul 6, 2023

banderror Jul 7, 2023

kibana-ci commented Jul 6, 2023

ESLint disabled line counts

Total ESLint disabled count

banderror left a comment

kibanamachine commented Jul 11, 2023

		// Setting a crazy large "Additional look-back time" to force a failure
		from: 'now-9007199254746990s',

[Security Solution] Tests: Filter by rule execution status #160502

[Security Solution] Tests: Filter by rule execution status #160502

Conversation

nikitaindik commented Jun 26, 2023 • edited by kibanamachine Loading

Summary

elasticmachine commented Jun 26, 2023

elasticmachine commented Jun 26, 2023

maximpn left a comment

Choose a reason for hiding this comment

yctercero commented Jul 3, 2023

yctercero left a comment

Choose a reason for hiding this comment

banderror left a comment

Choose a reason for hiding this comment

banderror Jul 5, 2023

Choose a reason for hiding this comment

nikitaindik Jul 6, 2023

Choose a reason for hiding this comment

banderror Jul 7, 2023

Choose a reason for hiding this comment

banderror Jul 5, 2023

Choose a reason for hiding this comment

nikitaindik Jul 6, 2023

Choose a reason for hiding this comment

banderror Jul 7, 2023

Choose a reason for hiding this comment

kibana-ci commented Jul 6, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

banderror left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 11, 2023

💚 All backports created successfully

Questions ?

nikitaindik commented Jun 26, 2023 •

edited by kibanamachine

Loading