Skip to content

Commit

Permalink
[Security Solution] [Fix] Cypress test flakyness in Alert page Contro…
Browse files Browse the repository at this point in the history
…ls (#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
  • Loading branch information
logeekal committed May 2, 2023
1 parent 0bf005a commit 6203acb
Show file tree
Hide file tree
Showing 9 changed files with 214 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
addNewFilterGroupControlValues,
deleteFilterGroupControl,
discardFilterGroupControls,
editFilterGroupControl,
editFilterGroupControls,
saveFilterGroupControls,
} from '../../tasks/common/filter_group';
Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"]';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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');
};
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,18 @@ export const FilterGroupContextMenu = () => {
[toggleContextMenu]
);

const resetSelection = useCallback(() => {
const resetSelection = useCallback(async () => {
if (!controlGroupInputUpdates) return;

// remove existing embeddables
controlGroup?.updateInput({
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,
Expand All @@ -68,9 +69,8 @@ export const FilterGroupContextMenu = () => {
dataViewId: dataViewId ?? '',
...control,
});
});
}

controlGroup?.reload();
switchToViewMode();
setShowFiltersChangedBanner(false);
}, [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,15 +334,15 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
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<
ControlPanelState<OptionsListEmbeddableInput>
>;
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
);
Expand All @@ -354,7 +354,7 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
}

// add persitable controls
controlGroup?.addOptionsListControl({
await controlGroup?.addOptionsListControl({
title: control.title,
hideExclude: true,
hideSort: true,
Expand All @@ -367,21 +367,22 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
...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]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> = {
Expand Down Expand Up @@ -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();
Expand All @@ -115,6 +117,7 @@ export const createStartServicesMock = (
apm,
cases,
unifiedSearch,
dataViews: dataViewServiceMock,
data: {
...data,
dataViews: {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<ComponentProps<typeof DetectionPageFilterSet>> = {}) => {
return (
<TestProviders>
<DetectionPageFilterSet
chainingSystem="HIERARCHICAL"
dataViewId=""
onFilterChange={onFilterChangeMock}
{...props}
/>
</TestProviders>
);
};

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(<TestComponent />);
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(<TestComponent />);
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(<TestComponent />);
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(<TestComponent />);

await waitFor(() => {
expect(onFilterChangeMock).toHaveBeenNthCalledWith(1, [
{
...filtersToTest[0],
meta: {
...filtersToTest[0].meta,
disabled: false,
},
},
]);
});
});
});
Loading

0 comments on commit 6203acb

Please sign in to comment.