[Security Solution] Unskip remaining Jest tests from RAC rules migration #122677

madirey · 2022-01-11T17:17:36Z

Summary

This gives us the remaining coverage from when tests were skipped during the process of migrating to the new rules and alerts schema for 8.0.

Most of the tests would have already been passing, aside from internal unrelated changes that occurred while the tests were disabled. For those tests, I've documented the commits that necessitated test changes below.

Tests

Memory event code: Fields were removed here
Behavior event code: Fields were removed here
Threat indicator path: Default value was removed here
Prepackaged rules: Rule registry is always enabled
Update rules: Mocked function was removed here

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

…rity-solution-tests

kibana-ci · 2022-01-11T22:02:28Z

💛 Build succeeded, but was flaky

Metrics [docs]

✅ unchanged

History

💔 Build #16776 failed 7e406d5

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

cc @madirey

elasticmachine · 2022-01-11T22:44:15Z

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

spong · 2022-01-12T18:46:42Z

...curity_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts

-      getAlertMock(isRuleRegistryEnabled, getQueryRuleParams())
-    );
+    clients.rulesClient.find.mockResolvedValue(getFindResultWithSingleHit(true));
+    clients.rulesClient.update.mockResolvedValue(getAlertMock(true, getQueryRuleParams()));


Can getAlertMock be refactored to remove the isRuleRegistryEnabled param now that we're in 8.0, or are there situations where it can still be disabled (and tested)?

spong

One nit about if we can refactor getMockRule, but other than that LGTM! Thanks for looping back to unskip these and the additional details in the description @madirey! 🙂 🎉

rylnd

I had the same question about getAlertMock as Garret, but otherwise LGTM!

rylnd · 2022-01-12T18:54:29Z

...plugins/security_solution/public/common/components/event_details/alert_summary_view.test.tsx

@@ -107,7 +107,7 @@ describe('AlertSummaryView', () => {
    );
    expect(container.querySelector('div[data-test-subj="summary-view"]')).toMatchSnapshot();


For posterity: it looks like these component/snapshot changes are due to the work done in #120347.

rylnd · 2022-01-12T19:00:48Z

.../plugins/security_solution/public/detections/components/rules/step_about_rule/index.test.tsx

@@ -149,14 +148,19 @@ describe.skip('StepAboutRuleComponent', () => {
      </ThemeProvider>
    );

+    wrapper


Should these DOM interactions not be contained within the act below? My understanding was that act allows the test framework to register/process/flush any changes to the DOM. If it works it works, but my concern is that these might break in the future. Happy to be corrected, though!

…ion (elastic#122677) * Regenerate snapshot of memory event summary rows * Regenerate snapshot of behavior event summary rows * Unskip StepAboutRuleComponent tests * Unskip add_prepackaged_rules tests * Unskip update_rules tests (cherry picked from commit c9dcd93)

kibanamachine · 2022-01-13T13:36:49Z

💚 All backports created successfully

Status	Branch	Result
✅	8.0

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