Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security solution][Endpoint] Add unit tests for fleet event filters/trusted apps cards #101034

Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f6934d2
Shows event filters card on fleet page
dasansol92 May 26, 2021
3864d72
Uses aggs instead of while loop to retrieve summary data
dasansol92 May 26, 2021
64e9d65
Add request and response types in the lists package
dasansol92 May 26, 2021
06a8f0c
Fixes old import
dasansol92 May 26, 2021
41c0bd7
Merge branch 'master' into feature/olm-add_event_filters_summary_card…
dasansol92 May 26, 2021
0ad0902
Removes old i18n keys
dasansol92 May 26, 2021
662c59b
Removes more old i18n keys
dasansol92 May 26, 2021
aca55cd
Use consts for exception lists url and endpoint event filter list id
dasansol92 May 26, 2021
84d46a4
Uses event filters service to retrieve summary data
dasansol92 May 27, 2021
e42bf61
Fixes addressed pr comments such as changing the route without unders…
dasansol92 May 27, 2021
9f6b620
Uses useMemo instead of useState to memoize object
dasansol92 May 27, 2021
5b17fd5
Add new e2e test for summart endpoint
dasansol92 May 27, 2021
39c8133
Handle api errors on event filters and trusted apps summary api calls
dasansol92 May 27, 2021
b0d77b2
Add api error message to the toast
dasansol92 May 27, 2021
9324dc3
Fix wrong i18n key
dasansol92 May 27, 2021
0cc6766
Change span tag by react fragment
dasansol92 May 27, 2021
29014e8
Uses styled components instead of modify compontent style directly an…
dasansol92 May 28, 2021
8993d4e
Adds curls script for summary route
dasansol92 May 28, 2021
31538c7
Merge branch 'master' into feature/olm-add_event_filters_summary_card…
kibanamachine May 28, 2021
bed96f6
Adds new unit tests for fleet card components
dasansol92 Jun 1, 2021
940457d
Fixes some warnings on ui
dasansol92 Jun 1, 2021
9086f2f
Merge branch 'feature/olm-add_event_filters_summary_card_to_the_fleet…
dasansol92 Jun 1, 2021
eaae47d
Merge branch 'master' into feature/olm-add_event_filters_summary_card…
dasansol92 Jun 1, 2021
a13e57e
Adds some syntax and readibility nits comming from pr comments
dasansol92 Jun 1, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
* 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 React from 'react';
import { ThemeProvider } from 'styled-components';
import { I18nProvider } from '@kbn/i18n/react';
import { ExceptionItemsSummary } from './exception_items_summary';
import * as reactTestingLibrary from '@testing-library/react';
import { getMockTheme } from '../../../../../../../../public/common/lib/kibana/kibana_react.mock';
import { GetExceptionSummaryResponse } from '../../../../../../../../common/endpoint/types';

const mockTheme = getMockTheme({
eui: {
paddingSizes: { m: '2' },
},
});

const getStatValue = (el: reactTestingLibrary.RenderResult, stat: string) => {
return el.getByText(stat)!.nextSibling?.lastChild?.textContent;
};

describe('Fleet event filters card', () => {
const renderComponent: (
stats: GetExceptionSummaryResponse
) => reactTestingLibrary.RenderResult = (stats) => {
const Wrapper: React.FC = ({ children }) => (
<I18nProvider>
<ThemeProvider theme={mockTheme}>{children}</ThemeProvider>
</I18nProvider>
);
const component = reactTestingLibrary.render(<ExceptionItemsSummary stats={stats} />, {
wrapper: Wrapper,
});
return component;
};
it('should renders correctly', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: ...render...

const summary: GetExceptionSummaryResponse = {
windows: 3,
linux: 2,
macos: 2,
total: 7,
};
const component = renderComponent(summary);

expect(component.getByText('Windows')).not.toBeNull();
expect(getStatValue(component, 'Windows')).toEqual(summary.windows.toString());

expect(component.getByText('Linux')).not.toBeNull();
expect(getStatValue(component, 'Linux')).toEqual(summary.linux.toString());

expect(component.getByText('Mac')).not.toBeNull();
expect(getStatValue(component, 'Mac')).toEqual(summary.macos.toString());

expect(component.getByText('Total')).not.toBeNull();
expect(getStatValue(component, 'Total')).toEqual(summary.total.toString());
});
it('should renders correctly when missing some stats', () => {
const summary: Partial<GetExceptionSummaryResponse> = {
windows: 3,
total: 3,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a test case to check if the total is missing? is that a possible case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's possible but in the component each item is parsed in the same way, so if some key is missing, a 0 will be displayed instead

};
const component = renderComponent(summary as GetExceptionSummaryResponse);

expect(component.getByText('Windows')).not.toBeNull();
expect(getStatValue(component, 'Windows')).toEqual('3');

expect(component.getByText('Linux')).not.toBeNull();
expect(getStatValue(component, 'Linux')).toEqual('0');

expect(component.getByText('Mac')).not.toBeNull();
expect(getStatValue(component, 'Mac')).toEqual('0');

expect(component.getByText('Total')).not.toBeNull();
expect(getStatValue(component, 'Total')).toEqual('3');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const ExceptionItemsSummary = memo<ExceptionItemsSummaryProps>(({ stats }
<EuiFlexGroup alignItems="center" justifyContent="spaceAround">
{SUMMARY_KEYS.map((stat) => {
return (
<EuiFlexItem>
<EuiFlexItem key={stat}>
<SummaryStat
value={stats?.[stat] ?? 0}
color={stat === 'total' ? 'primary' : 'default'}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/*
* 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 React from 'react';
import { ThemeProvider } from 'styled-components';
import { I18nProvider } from '@kbn/i18n/react';
import { FleetEventFiltersCard } from './fleet_event_filters_card';
import * as reactTestingLibrary from '@testing-library/react';
import { EventFiltersHttpService } from '../../../../../event_filters/service';
import { useToasts } from '../../../../../../../common/lib/kibana';
import { getMockTheme } from '../../../../../../../../public/common/lib/kibana/kibana_react.mock';
import { GetExceptionSummaryResponse } from '../../../../../../../../common/endpoint/types';

jest.mock('./exception_items_summary');
jest.mock('../../../../../event_filters/service');

jest.mock('../../../../../../../../../../../src/plugins/kibana_react/public', () => {
const originalModule = jest.requireActual(
'../../../../../../../../../../../src/plugins/kibana_react/public'
);
const useKibana = jest.fn().mockImplementation(() => ({
services: {
http: {},
data: {},
notifications: {},
application: {
getUrlForApp: jest.fn(),
},
},
}));

return {
...originalModule,
useKibana,
};
});

jest.mock('../../../../../../../common/lib/kibana');

const mockTheme = getMockTheme({
eui: {
paddingSizes: { m: '2' },
},
});

const EventFiltersHttpServiceMock = EventFiltersHttpService as jest.Mock;
const useToastsMock = useToasts as jest.Mock;

const summary: GetExceptionSummaryResponse = {
windows: 3,
linux: 2,
macos: 2,
total: 7,
};

describe('Fleet event filters card', () => {
let promise: Promise<GetExceptionSummaryResponse>;
let addDanger: jest.Mock = jest.fn();
const renderComponent: () => Promise<reactTestingLibrary.RenderResult> = async () => {
const Wrapper: React.FC = ({ children }) => (
<I18nProvider>
<ThemeProvider theme={mockTheme}>{children}</ThemeProvider>
</I18nProvider>
);
// @ts-ignore
const component = reactTestingLibrary.render(<FleetEventFiltersCard />, { wrapper: Wrapper });
try {
// @ts-ignore
await reactTestingLibrary.act(() => promise);
} catch (err) {
return component;
}
return component;
};
afterEach(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: semantically it makes sense to keep all before... methods before after... methods. So beforeAll followed by beforeEach and then afterEach at the end.

EventFiltersHttpServiceMock.mockReset();
});
beforeEach(() => {
promise = Promise.resolve(summary);
addDanger = jest.fn();
});
beforeAll(() => {
useToastsMock.mockImplementation(() => {
return {
addDanger,
};
});
});
it('should renders correctly', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: ...render...

EventFiltersHttpServiceMock.mockImplementationOnce(() => {
return {
getSummary: () => jest.fn(() => promise),
};
});
const component = await renderComponent();
expect(component.getByText('Event Filters')).not.toBeNull();
expect(component.getByText('Manage event filters')).not.toBeNull();
});
it('should renders an error when api call fails', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: should render an error toast when api call fails

expect(addDanger).toBeCalledTimes(0);
promise = Promise.reject(new Error('error test'));
EventFiltersHttpServiceMock.mockImplementationOnce(() => {
return {
getSummary: () => promise,
};
});
const component = await renderComponent();
expect(component.getByText('Event Filters')).not.toBeNull();
expect(component.getByText('Manage event filters')).not.toBeNull();
await reactTestingLibrary.waitFor(() => expect(addDanger).toBeCalledTimes(1));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { memo, useMemo, useState, useEffect } from 'react';
import React, { memo, useMemo, useState, useEffect, useRef } from 'react';
import { ApplicationStart, CoreStart } from 'kibana/public';
import { EuiPanel, EuiText } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
Expand Down Expand Up @@ -36,12 +36,14 @@ export const FleetEventFiltersCard = memo<PackageCustomExtensionComponentProps>(
const [stats, setStats] = useState<GetExceptionSummaryResponse | undefined>();
const eventFiltersListUrlPath = getEventFiltersListPath();
const eventFiltersApi = useMemo(() => new EventFiltersHttpService(http), [http]);
const isMounted = useRef<boolean>();

useEffect(() => {
isMounted.current = true;
const fetchStats = async () => {
try {
const summary = await eventFiltersApi.getSummary();
setStats(summary);
if (isMounted.current) setStats(summary);
} catch (error) {
toasts.addDanger(
i18n.translate(
Expand All @@ -55,6 +57,9 @@ export const FleetEventFiltersCard = memo<PackageCustomExtensionComponentProps>(
}
};
fetchStats();
return () => {
isMounted.current = false;
};
}, [eventFiltersApi, toasts]);

const eventFiltersRouteState = useMemo(() => {
Expand All @@ -79,7 +84,7 @@ export const FleetEventFiltersCard = memo<PackageCustomExtensionComponentProps>(
return (
<EuiPanel paddingSize="l">
<StyledEuiFlexGridGroup alignItems="baseline" justifyContent="center">
<StyledEuiFlexGridItem gridArea="title" alignItems="flex-start">
<StyledEuiFlexGridItem gridarea="title" alignitems="flex-start">
<EuiText>
<h4>
<FormattedMessage
Expand All @@ -89,10 +94,10 @@ export const FleetEventFiltersCard = memo<PackageCustomExtensionComponentProps>(
</h4>
</EuiText>
</StyledEuiFlexGridItem>
<StyledEuiFlexGridItem gridArea="summary">
<StyledEuiFlexGridItem gridarea="summary">
<ExceptionItemsSummary stats={stats} />
</StyledEuiFlexGridItem>
<StyledEuiFlexGridItem gridArea="link" alignItems="flex-end">
<StyledEuiFlexGridItem gridarea="link" alignitems="flex-end">
<>
<LinkWithIcon
appId={MANAGEMENT_APP_ID}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/*
* 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 React from 'react';
import { ThemeProvider } from 'styled-components';
import { I18nProvider } from '@kbn/i18n/react';
import { FleetTrustedAppsCard } from './fleet_trusted_apps_card';
import * as reactTestingLibrary from '@testing-library/react';
import { TrustedAppsHttpService } from '../../../../../trusted_apps/service';
import { useToasts } from '../../../../../../../common/lib/kibana';
import { getMockTheme } from '../../../../../../../../public/common/lib/kibana/kibana_react.mock';
import { GetExceptionSummaryResponse } from '../../../../../../../../common/endpoint/types';

jest.mock('./exception_items_summary');
jest.mock('../../../../../trusted_apps/service');

jest.mock('../../../../../../../../../../../src/plugins/kibana_react/public', () => {
const originalModule = jest.requireActual(
'../../../../../../../../../../../src/plugins/kibana_react/public'
);
const useKibana = jest.fn().mockImplementation(() => ({
services: {
http: {},
data: {},
notifications: {},
application: {
getUrlForApp: jest.fn(),
},
},
}));

return {
...originalModule,
useKibana,
};
});

jest.mock('../../../../../../../common/lib/kibana');

const mockTheme = getMockTheme({
eui: {
paddingSizes: { m: '2' },
},
});

const TrustedAppsHttpServiceMock = TrustedAppsHttpService as jest.Mock;
const useToastsMock = useToasts as jest.Mock;

const summary: GetExceptionSummaryResponse = {
windows: 3,
linux: 2,
macos: 2,
total: 7,
};

describe('Fleet trusted apps card', () => {
let promise: Promise<GetExceptionSummaryResponse>;
let addDanger: jest.Mock = jest.fn();
const renderComponent: () => Promise<reactTestingLibrary.RenderResult> = async () => {
const Wrapper: React.FC = ({ children }) => (
<I18nProvider>
<ThemeProvider theme={mockTheme}>{children}</ThemeProvider>
</I18nProvider>
);
// @ts-ignore
const component = reactTestingLibrary.render(<FleetTrustedAppsCard />, { wrapper: Wrapper });
try {
// @ts-ignore
await reactTestingLibrary.act(() => promise);
} catch (err) {
return component;
}
return component;
};

afterEach(() => {
TrustedAppsHttpServiceMock.mockReset();
});
beforeEach(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: semantically it makes sense to keep all before.. methods before after... methods. So beforeAll followed by beforeEach and then afterEach at the end.

promise = Promise.resolve(summary);
addDanger = jest.fn();
});
beforeAll(() => {
useToastsMock.mockImplementation(() => {
return {
addDanger,
};
});
});
it('should renders correctly', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: should render...

TrustedAppsHttpServiceMock.mockImplementationOnce(() => {
return {
getTrustedAppsSummary: () => jest.fn(() => promise),
};
});
const component = await renderComponent();
expect(component.getByText('Trusted Applications')).not.toBeNull();
expect(component.getByText('Manage trusted applications')).not.toBeNull();
});
it('should renders an error when api call fails', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: should render...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: I think it's okay to have test descriptions with even more context. This one renders an error toast and should say something like should render an error toast when.... That way one doesn't have to read the code to know what this test is verifying.

expect(addDanger).toBeCalledTimes(0);
promise = Promise.reject(new Error('error test'));
TrustedAppsHttpServiceMock.mockImplementationOnce(() => {
return {
getTrustedAppsSummary: () => promise,
};
});
const component = await renderComponent();
expect(component.getByText('Trusted Applications')).not.toBeNull();
expect(component.getByText('Manage trusted applications')).not.toBeNull();
await reactTestingLibrary.waitFor(() => expect(addDanger).toBeCalledTimes(1));
});
});
Loading