From 6203acbe0516fed363d99b8c7a067b57d48e5f62 Mon Sep 17 00:00:00 2001 From: Jatin Kathuria Date: Tue, 2 May 2023 19:24:11 +0200 Subject: [PATCH] [Security Solution] [Fix] Cypress test flakyness in Alert page Controls (#155988) ## Summary Handles : #153685 and #153686 This PR tries to fix the flakyness of cypress tests. Although, this issue in cypress is very difficult to reproduce, I noticed that it is coming mainly when adding extra control. And it looks like during the course of dev one of the API of adding a control called `addOptionsListControl` was changed to be a promise, therefore, mainly the change is to await the promise before adding a new control. ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- .../detection_page_filters.cy.ts | 32 +++- .../cypress/screens/common/filter_group.ts | 6 +- .../cypress/tasks/common/filter_group.ts | 23 +++ .../components/filter_group/context_menu.tsx | 12 +- .../filter_group/filter_group.test.tsx | 3 - .../common/components/filter_group/index.tsx | 19 +-- .../common/lib/kibana/kibana_react.mock.ts | 3 + .../detection_page_filters/index.test.tsx | 144 ++++++++++++++++++ .../detection_page_filters/index.tsx | 2 +- 9 files changed, 214 insertions(+), 30 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detections/components/detection_page_filters/index.test.tsx diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/detection_page_filters.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/detection_page_filters.cy.ts index c16ccf35edbccb..a1a064eee6dc18 100644 --- a/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/detection_page_filters.cy.ts +++ b/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/detection_page_filters.cy.ts @@ -39,6 +39,7 @@ import { addNewFilterGroupControlValues, deleteFilterGroupControl, discardFilterGroupControls, + editFilterGroupControl, editFilterGroupControls, saveFilterGroupControls, } from '../../tasks/common/filter_group'; @@ -97,7 +98,7 @@ const assertFilterControlsWithFilterObject = (filterObject = DEFAULT_DETECTION_P }); }; -describe.skip('Detections : Page Filters', { testIsolation: false }, () => { +describe('Detections : Page Filters', { testIsolation: false }, () => { before(() => { cleanKibana(); login(); @@ -116,10 +117,23 @@ describe.skip('Detections : Page Filters', { testIsolation: false }, () => { }); context('Alert Page Filters Customization ', { testIsolation: false }, () => { - it('Add New Controls', () => { + beforeEach(() => { + resetFilters(); + }); + it('should be able to delete Controls', () => { + waitForPageFilters(); + editFilterGroupControls(); + deleteFilterGroupControl(3); + cy.get(CONTROL_FRAMES).should((sub) => { + expect(sub.length).lt(4); + }); + discardFilterGroupControls(); + }); + it('should be able to add new Controls', () => { const fieldName = 'event.module'; const label = 'EventModule'; editFilterGroupControls(); + deleteFilterGroupControl(3); addNewFilterGroupControlValues({ fieldName, label, @@ -129,18 +143,20 @@ describe.skip('Detections : Page Filters', { testIsolation: false }, () => { discardFilterGroupControls(); cy.get(CONTROL_FRAME_TITLE).should('not.contain.text', label); }); - it('Delete Controls', () => { - waitForPageFilters(); + it('should be able to edit Controls', () => { + const fieldName = 'event.module'; + const label = 'EventModule'; editFilterGroupControls(); - deleteFilterGroupControl(3); - cy.get(CONTROL_FRAMES).should((sub) => { - expect(sub.length).lt(4); - }); + editFilterGroupControl({ idx: 3, fieldName, label }); + cy.get(CONTROL_FRAME_TITLE).should('contain.text', label); + cy.get(FILTER_GROUP_SAVE_CHANGES_POPOVER).should('be.visible'); discardFilterGroupControls(); + cy.get(CONTROL_FRAME_TITLE).should('not.contain.text', label); }); it('should not sync to the URL in edit mode but only in view mode', () => { cy.url().then((urlString) => { editFilterGroupControls(); + deleteFilterGroupControl(3); addNewFilterGroupControlValues({ fieldName: 'event.module', label: 'Event Module' }); cy.url().should('eq', urlString); saveFilterGroupControls(); diff --git a/x-pack/plugins/security_solution/cypress/screens/common/filter_group.ts b/x-pack/plugins/security_solution/cypress/screens/common/filter_group.ts index f78fbcfa9e4760..b36229a5cbeaaa 100644 --- a/x-pack/plugins/security_solution/cypress/screens/common/filter_group.ts +++ b/x-pack/plugins/security_solution/cypress/screens/common/filter_group.ts @@ -45,12 +45,12 @@ export const DETECTION_PAGE_FILTER_GROUP_CONTEXT_MENU = '[data-test-subj="filter export const DETECTION_PAGE_FILTER_GROUP_RESET_BUTTON = '[data-test-subj="filter-group__context--reset"]'; -export const FILTER_GROUP_CONTEXT_EDIT_CONTROLS = '[data-test-subj="filter_group__context--edit"]'; +export const FILTER_GROUP_CONTEXT_EDIT_CONTROLS = '[data-test-subj="filter-group__context--edit"]'; -export const FILTER_GROUP_CONTEXT_SAVE_CONTROLS = '[data-test-subj="filter_group__context--save"]'; +export const FILTER_GROUP_CONTEXT_SAVE_CONTROLS = '[data-test-subj="filter-group__context--save"]'; export const FILTER_GROUP_CONTEXT_DISCARD_CHANGES = - '[data-test-subj="filter_group__context--discard"]'; + '[data-test-subj="filter-group__context--discard"]'; export const FILTER_GROUP_ADD_CONTROL = '[data-test-subj="filter-group__add-control"]'; diff --git a/x-pack/plugins/security_solution/cypress/tasks/common/filter_group.ts b/x-pack/plugins/security_solution/cypress/tasks/common/filter_group.ts index e006e2cc030a4f..e12d705ab1ca7a 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/common/filter_group.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/common/filter_group.ts @@ -22,6 +22,7 @@ import { DETECTION_PAGE_FILTERS_LOADING, OPTION_LISTS_LOADING, FILTER_GROUP_CONTEXT_DISCARD_CHANGES, + FILTER_GROUP_CONTROL_ACTION_EDIT, } from '../../screens/common/filter_group'; import { waitForPageFilters } from '../alerts'; @@ -90,3 +91,25 @@ export const deleteFilterGroupControl = (idx: number) => { cy.get(FILTER_GROUP_CONTROL_CONFIRM_DIALOG).should('be.visible'); cy.get(FILTER_GROUP_CONTROL_CONFIRM_BTN).trigger('click'); }; + +export const editFilterGroupControl = ({ + idx, + fieldName, + label, +}: { + idx: number; + fieldName: string; + label: string; +}) => { + cy.get(CONTROL_FRAME_TITLE).eq(idx).trigger('mouseover'); + cy.get(FILTER_GROUP_CONTROL_ACTION_EDIT(idx)).trigger('click', { force: true }); + const { FIELD_SEARCH, FIELD_PICKER, FIELD_LABEL, SAVE } = FILTER_GROUP_EDIT_CONTROL_PANEL_ITEMS; + cy.get(FIELD_SEARCH).type(fieldName); + cy.get(FIELD_PICKER(fieldName)).should('exist').trigger('click'); + + cy.get(FIELD_LABEL).should('have.value', fieldName); + cy.get(FIELD_LABEL).clear(); + cy.get(FIELD_LABEL).type(label).should('have.value', label); + cy.get(SAVE).trigger('click'); + cy.get(FILTER_GROUP_EDIT_CONTROLS_PANEL).should('not.exist'); +}; diff --git a/x-pack/plugins/security_solution/public/common/components/filter_group/context_menu.tsx b/x-pack/plugins/security_solution/public/common/components/filter_group/context_menu.tsx index bf672a5fabfa4c..b7dc2f6f32457f 100644 --- a/x-pack/plugins/security_solution/public/common/components/filter_group/context_menu.tsx +++ b/x-pack/plugins/security_solution/public/common/components/filter_group/context_menu.tsx @@ -48,7 +48,7 @@ export const FilterGroupContextMenu = () => { [toggleContextMenu] ); - const resetSelection = useCallback(() => { + const resetSelection = useCallback(async () => { if (!controlGroupInputUpdates) return; // remove existing embeddables @@ -56,9 +56,10 @@ export const FilterGroupContextMenu = () => { panels: {}, }); - initialControls.forEach((control, idx) => { - controlGroup?.addOptionsListControl({ - controlId: String(idx), + for (let counter = 0; counter < initialControls.length; counter++) { + const control = initialControls[counter]; + await controlGroup?.addOptionsListControl({ + controlId: String(counter), hideExclude: true, hideSort: true, hidePanelTitles: true, @@ -68,9 +69,8 @@ export const FilterGroupContextMenu = () => { dataViewId: dataViewId ?? '', ...control, }); - }); + } - controlGroup?.reload(); switchToViewMode(); setShowFiltersChangedBanner(false); }, [ diff --git a/x-pack/plugins/security_solution/public/common/components/filter_group/filter_group.test.tsx b/x-pack/plugins/security_solution/public/common/components/filter_group/filter_group.test.tsx index f066b022af04e7..5da07f8add8647 100644 --- a/x-pack/plugins/security_solution/public/common/components/filter_group/filter_group.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/filter_group/filter_group.test.tsx @@ -462,14 +462,11 @@ describe(' Filter Group Component ', () => { controlGroupMock.addOptionsListControl.mockClear(); controlGroupMock.updateInput.mockClear(); - controlGroupMock.reload.mockClear(); fireEvent.click(screen.getByTestId(TEST_IDS.CONTEXT_MENU.RESET)); await waitFor(() => { // blanks the input expect(controlGroupMock.updateInput.mock.calls.length).toBe(2); - expect(controlGroupMock.reload.mock.calls.length).toBe(1); - expect(controlGroupMock.addOptionsListControl.mock.calls.length).toBe(4); }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/filter_group/index.tsx b/x-pack/plugins/security_solution/public/common/components/filter_group/index.tsx index 533bf19146dad9..a4c61fb9c98a61 100644 --- a/x-pack/plugins/security_solution/public/common/components/filter_group/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/filter_group/index.tsx @@ -334,7 +334,7 @@ const FilterGroupComponent = (props: PropsWithChildren) => { setShowFiltersChangedBanner(false); }, [controlGroup, switchToViewMode, getStoredControlInput, hasPendingChanges]); - const upsertPersistableControls = useCallback(() => { + const upsertPersistableControls = useCallback(async () => { const persistableControls = initialControls.filter((control) => control.persist === true); if (persistableControls.length > 0) { const currentPanels = Object.values(controlGroup?.getInput().panels ?? []) as Array< @@ -342,7 +342,7 @@ const FilterGroupComponent = (props: PropsWithChildren) => { >; const orderedPanels = currentPanels.sort((a, b) => a.order - b.order); let filterControlsDeleted = false; - persistableControls.forEach((control) => { + for (const control of persistableControls) { const controlExists = currentPanels.some( (currControl) => control.fieldName === currControl.explicitInput.fieldName ); @@ -354,7 +354,7 @@ const FilterGroupComponent = (props: PropsWithChildren) => { } // add persitable controls - controlGroup?.addOptionsListControl({ + await controlGroup?.addOptionsListControl({ title: control.title, hideExclude: true, hideSort: true, @@ -367,21 +367,22 @@ const FilterGroupComponent = (props: PropsWithChildren) => { ...control, }); } - }); - orderedPanels.forEach((panel) => { + } + + for (const panel of orderedPanels) { if (panel.explicitInput.fieldName) - controlGroup?.addOptionsListControl({ + await controlGroup?.addOptionsListControl({ selectedOptions: [], fieldName: panel.explicitInput.fieldName, dataViewId: dataViewId ?? '', ...panel.explicitInput, }); - }); + } } }, [controlGroup, dataViewId, initialControls]); - const saveChangesHandler = useCallback(() => { - upsertPersistableControls(); + const saveChangesHandler = useCallback(async () => { + await upsertPersistableControls(); switchToViewMode(); setShowFiltersChangedBanner(false); }, [switchToViewMode, upsertPersistableControls]); diff --git a/x-pack/plugins/security_solution/public/common/lib/kibana/kibana_react.mock.ts b/x-pack/plugins/security_solution/public/common/lib/kibana/kibana_react.mock.ts index 0383b4a700420c..702e3f2bc39e8d 100644 --- a/x-pack/plugins/security_solution/public/common/lib/kibana/kibana_react.mock.ts +++ b/x-pack/plugins/security_solution/public/common/lib/kibana/kibana_react.mock.ts @@ -45,6 +45,7 @@ import { triggersActionsUiMock } from '@kbn/triggers-actions-ui-plugin/public/mo import { mockApm } from '../apm/service.mock'; import { cloudExperimentsMock } from '@kbn/cloud-experiments-plugin/common/mocks'; import { guidedOnboardingMock } from '@kbn/guided-onboarding-plugin/public/mocks'; +import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks'; import { of } from 'rxjs'; const mockUiSettings: Record = { @@ -105,6 +106,7 @@ export const createStartServicesMock = ( const fleet = fleetMock.createStartMock(); const unifiedSearch = unifiedSearchPluginMock.createStartContract(); const cases = mockCasesContract(); + const dataViewServiceMock = dataViewPluginMocks.createStartContract(); cases.helpers.getUICapabilities.mockReturnValue(noCasesPermissions()); const triggersActionsUi = triggersActionsUiMock.createStart(); const cloudExperiments = cloudExperimentsMock.createStartMock(); @@ -115,6 +117,7 @@ export const createStartServicesMock = ( apm, cases, unifiedSearch, + dataViews: dataViewServiceMock, data: { ...data, dataViews: { diff --git a/x-pack/plugins/security_solution/public/detections/components/detection_page_filters/index.test.tsx b/x-pack/plugins/security_solution/public/detections/components/detection_page_filters/index.test.tsx new file mode 100644 index 00000000000000..1035f08bb902f4 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/detection_page_filters/index.test.tsx @@ -0,0 +1,144 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { ComponentProps } from 'react'; +import React from 'react'; + +import { render, screen, waitFor } from '@testing-library/react'; +import { TestProviders } from '../../../common/mock'; +import { DetectionPageFilterSet } from '.'; +import { TEST_IDS } from '../../../common/components/filter_group/constants'; +import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock'; +import type { DataView } from '@kbn/data-views-plugin/common'; +import { useKibana } from '../../../common/lib/kibana'; +import { FilterGroup } from '../../../common/components/filter_group'; +import { getMockedFilterGroupWithCustomFilters } from '../../../common/components/filter_group/mocks'; +import type { DataViewsServicePublic } from '@kbn/data-views-plugin/public'; + +jest.mock('../../../common/components/filter_group'); + +jest.mock('../../../common/lib/kibana'); + +const basicKibanaServicesMock = createStartServicesMock(); + +const getFieldByNameMock = jest.fn(() => true); + +const mockedDataViewServiceGetter = jest.fn(() => { + return Promise.resolve({ + getFieldByName: getFieldByNameMock, + } as unknown as DataView); +}); + +const getKibanaServiceWithMockedGetter = ( + mockedDataViewGetter: DataViewsServicePublic['get'] = mockedDataViewServiceGetter +) => { + return { + ...basicKibanaServicesMock, + dataViews: { + ...basicKibanaServicesMock.dataViews, + clearInstanceCache: jest.fn(), + get: mockedDataViewGetter, + }, + }; +}; + +const kibanaServiceDefaultMock = getKibanaServiceWithMockedGetter(); + +const onFilterChangeMock = jest.fn(); + +const TestComponent = (props: Partial> = {}) => { + return ( + + + + ); +}; + +describe('Detection Page Filters', () => { + beforeAll(() => { + (FilterGroup as jest.Mock).mockImplementation(getMockedFilterGroupWithCustomFilters()); + (useKibana as jest.Mock).mockReturnValue({ + services: { + ...kibanaServiceDefaultMock, + }, + }); + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + it('should renders correctly', async () => { + render(); + expect(screen.getByTestId(TEST_IDS.FILTER_LOADING)).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByTestId(TEST_IDS.MOCKED_CONTROL)).toBeInTheDocument(); + }); + }); + + it('should check all the fields till any absent field is found', async () => { + render(); + expect(screen.getByTestId(TEST_IDS.FILTER_LOADING)).toBeInTheDocument(); + await waitFor(() => { + expect(getFieldByNameMock).toHaveBeenCalledTimes(4); + expect(kibanaServiceDefaultMock.dataViews.clearInstanceCache).toHaveBeenCalledTimes(0); + }); + }); + + it('should stop checking fields if blank field is found and clear the cache', async () => { + const getFieldByNameLocalMock = jest.fn(() => false); + const mockGetter = jest.fn(() => + Promise.resolve({ getFieldByName: getFieldByNameLocalMock } as unknown as DataView) + ); + const modifiedKibanaServicesMock = getKibanaServiceWithMockedGetter(mockGetter); + (useKibana as jest.Mock).mockReturnValueOnce({ services: modifiedKibanaServicesMock }); + + render(); + expect(screen.getByTestId(TEST_IDS.FILTER_LOADING)).toBeInTheDocument(); + await waitFor(() => { + expect(getFieldByNameLocalMock).toHaveBeenCalledTimes(1); + expect(modifiedKibanaServicesMock.dataViews.clearInstanceCache).toHaveBeenCalledTimes(1); + expect(screen.getByTestId(TEST_IDS.MOCKED_CONTROL)).toBeInTheDocument(); + }); + }); + + it('should call onFilterChange', async () => { + const filtersToTest = [ + { + meta: { + index: 'security-solution-default', + key: 'kibana.alert.severity', + }, + query: { + match_phrase: { + 'kibana.alert.severity': 'low', + }, + }, + }, + ]; + (FilterGroup as jest.Mock).mockImplementationOnce( + getMockedFilterGroupWithCustomFilters(filtersToTest) + ); + render(); + + await waitFor(() => { + expect(onFilterChangeMock).toHaveBeenNthCalledWith(1, [ + { + ...filtersToTest[0], + meta: { + ...filtersToTest[0].meta, + disabled: false, + }, + }, + ]); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/detections/components/detection_page_filters/index.tsx b/x-pack/plugins/security_solution/public/detections/components/detection_page_filters/index.tsx index 1454c63ffb3989..d096f56f7faa58 100644 --- a/x-pack/plugins/security_solution/public/detections/components/detection_page_filters/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/detection_page_filters/index.tsx @@ -41,8 +41,8 @@ const FilterItemSetComponent = (props: FilterItemSetProps) => { setLoadingPageFilters(false); return; } - setLoadingPageFilters(false); } + setLoadingPageFilters(false); })(); }, [dataViewId, dataViewService]);