Skip to content

Commit

Permalink
Various bug fixes and unit tests for AssociatedDetectors (#505)
Browse files Browse the repository at this point in the history
* associated detectors unit tests

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

* fixed some bugs and added unit tests for associated detectors

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

---------

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>
amitgalitz authored Jun 8, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 6567649 commit b4de09a
Showing 8 changed files with 417 additions and 19 deletions.
17 changes: 9 additions & 8 deletions public/action/ad_dashboard_action.tsx
Original file line number Diff line number Diff line change
@@ -14,6 +14,9 @@ import {
} from '../../../../src/plugins/ui_actions/public';
import { isReferenceOrValueEmbeddable } from '../../../../src/plugins/embeddable/public';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { isEmpty } from 'lodash';
import { VisualizeEmbeddable } from '../../../../src/plugins/visualizations/public';
import { isEligibleForVisLayers } from '../../../../src/plugins/vis_augmenter/public';

export const ACTION_AD = 'ad';

@@ -57,15 +60,13 @@ export const createADAction = ({
type: ACTION_AD,
grouping,
isCompatible: async ({ embeddable }: ActionContext) => {
const paramsType = embeddable.vis?.params?.type;
const seriesParams = embeddable.vis?.params?.seriesParams || [];
const series = embeddable.vis?.params?.series || [];
const isLineGraph =
seriesParams.find((item) => item.type === 'line') ||
series.find((item) => item.chart_type === 'line');
const isValidVis = isLineGraph && paramsType !== 'table';
const vis = (embeddable as VisualizeEmbeddable).vis;
return Boolean(
embeddable.parent && isDashboard(embeddable.parent) && isValidVis
embeddable.parent &&
isDashboard(embeddable.parent) &&
vis !== undefined &&
isEligibleForVisLayers(vis) &&
!isEmpty((embeddable as VisualizeEmbeddable).visLayers)
);
},
execute: async ({ embeddable }: ActionContext) => {
Original file line number Diff line number Diff line change
@@ -0,0 +1,389 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import React from 'react';
import { render, waitFor } from '@testing-library/react';
import AssociatedDetectors from '../AssociatedDetectors';
import { createMockVisEmbeddable } from '../../../../../../../../src/plugins/vis_augmenter/public/mocks';
import { FLYOUT_MODES } from '../../../../../../public/components/FeatureAnywhereContextMenu/AnywhereParentFlyout/constants';
import { CoreServicesContext } from '../../../../../../public/components/CoreServices/CoreServices';
import { coreServicesMock, httpClientMock } from '../../../../../../test/mocks';
import {
HashRouter as Router,
RouteComponentProps,
Route,
Switch,
} from 'react-router-dom';
import { Provider } from 'react-redux';
import configureStore from '../../../../../../public/redux/configureStore';
import { VisualizeEmbeddable } from '../../../../../../../../src/plugins/visualizations/public';
import {
setSavedFeatureAnywhereLoader,
setUISettings,
} from '../../../../../services';
import {
generateAugmentVisSavedObject,
VisLayerExpressionFn,
VisLayerTypes,
createSavedAugmentVisLoader,
setUISettings as setVisAugUISettings,
getMockAugmentVisSavedObjectClient,
SavedObjectLoaderAugmentVis,
} from '../../../../../../../../src/plugins/vis_augmenter/public';
import { getAugmentVisSavedObjs } from '../../../../../../../../src/plugins/vis_augmenter/public/utils';
import { uiSettingsServiceMock } from '../../../../../../../../src/core/public/mocks';
import {
PLUGIN_AUGMENTATION_ENABLE_SETTING,
PLUGIN_AUGMENTATION_MAX_OBJECTS_SETTING,
} from '../../../../../../../../src/plugins/vis_augmenter/common';
import userEvent from '@testing-library/user-event';
const fn = {
type: VisLayerTypes.PointInTimeEvents,
name: 'test-fn',
args: {
testArg: 'test-value',
},
} as VisLayerExpressionFn;
const originPlugin = 'test-plugin';

const uiSettingsMock = uiSettingsServiceMock.createStartContract();
setUISettings(uiSettingsMock);
setVisAugUISettings(uiSettingsMock);
const setUIAugSettings = (isEnabled = true, maxCount = 10) => {
uiSettingsMock.get.mockImplementation((key: string) => {
if (key === PLUGIN_AUGMENTATION_MAX_OBJECTS_SETTING) return maxCount;
else if (key === PLUGIN_AUGMENTATION_ENABLE_SETTING) return isEnabled;
else return false;
});
};

setUIAugSettings();

jest.mock('../../../../../services', () => ({
...jest.requireActual('../../../../../services'),

getUISettings: () => {
return {
get: (config: string) => {
switch (config) {
case 'visualization:enablePluginAugmentation':
return true;
case 'visualization:enablePluginAugmentation.maxPluginObjects':
return 10;
default:
throw new Error(
`Accessing ${config} is not supported in the mock.`
);
}
},
};
},
getNotifications: () => {
return {
toasts: {
addDanger: jest.fn().mockName('addDanger'),
addSuccess: jest.fn().mockName('addSuccess'),
},
};
},
}));

jest.mock(
'../../../../../../../../src/plugins/vis_augmenter/public/utils',
() => ({
getAugmentVisSavedObjs: jest.fn(),
})
);
const visEmbeddable = createMockVisEmbeddable(
'test-saved-obj-id',
'test-title',
false
);

const renderWithRouter = (visEmbeddable: VisualizeEmbeddable) => ({
...render(
<Provider store={configureStore(httpClientMock)}>
<Router>
<Switch>
<Route
render={(props: RouteComponentProps) => (
<CoreServicesContext.Provider value={coreServicesMock}>
<AssociatedDetectors
embeddable={visEmbeddable}
closeFlyout={jest.fn()}
setMode={FLYOUT_MODES.associated}
/>
</CoreServicesContext.Provider>
)}
/>
</Switch>
</Router>
</Provider>
),
});
describe('AssociatedDetectors spec', () => {
let augmentVisLoader: SavedObjectLoaderAugmentVis;
let mockDeleteFn: jest.Mock;
let detectorsToAssociate = new Array(2).fill(null).map((_, index) => {
return {
id: `detector_id_${index}`,
name: `detector_name_${index}`,
indices: [`index_${index}`],
totalAnomalies: 5,
lastActiveAnomaly: Date.now() + index,
};
});
//change one of the two detectors to have an ID not matching the ID in saved object
detectorsToAssociate[1].id = '5';

const savedObjects = new Array(2).fill(null).map((_, index) => {
const pluginResource = {
type: 'test-plugin',
id: `detector_id_${index}`,
};
return generateAugmentVisSavedObject(
`valid-obj-id-${index}`,
fn,
`vis-id-${index}`,
originPlugin,
pluginResource
);
});
beforeEach(() => {
mockDeleteFn = jest.fn().mockResolvedValue('someValue');
augmentVisLoader = createSavedAugmentVisLoader({
savedObjectsClient: {
...getMockAugmentVisSavedObjectClient(savedObjects),
delete: mockDeleteFn,
},
} as any) as SavedObjectLoaderAugmentVis;
setSavedFeatureAnywhereLoader(augmentVisLoader);
});
describe('Renders loading component', () => {
test('renders the detector is loading', async () => {
httpClientMock.get = jest.fn().mockResolvedValue({
ok: true,
response: { detectorList: [], totalDetectors: 0 },
});
(getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
Promise.resolve(savedObjects)
);
const { getByText } = renderWithRouter(visEmbeddable);
getByText('Loading detectors...');
getByText('Real-time state');
getByText('Associate a detector');
});
});

describe('renders either one or zero detectors', () => {
test('renders one associated detector', async () => {
httpClientMock.get = jest.fn().mockResolvedValue({
ok: true,
response: {
detectorList: detectorsToAssociate,
totalDetectors: detectorsToAssociate.length,
},
});
(getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
Promise.resolve(savedObjects)
);
const { getByText, queryByText } = renderWithRouter(visEmbeddable);
getByText('Loading detectors...');
await waitFor(() => getByText('detector_name_0'));
getByText('5');
expect(queryByText('detector_name_1')).toBeNull();
}, 80000);
test('renders no associated detectors', async () => {
httpClientMock.get = jest.fn().mockResolvedValue({
ok: true,
response: {
detectorList: [detectorsToAssociate[1]],
totalDetectors: 1,
},
});
(getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
Promise.resolve(savedObjects)
);
const { getByText, findByText } = renderWithRouter(visEmbeddable);
getByText('Loading detectors...');
await waitFor(() =>
findByText(
'There are no anomaly detectors associated with test-title visualization.',
undefined,
{ timeout: 100000 }
)
);
}, 150000);
});

describe('tests unlink functionality', () => {
test('unlinks a single detector', async () => {
httpClientMock.get = jest.fn().mockResolvedValue({
ok: true,
response: {
detectorList: detectorsToAssociate,
totalDetectors: detectorsToAssociate.length,
},
});
(getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
Promise.resolve(savedObjects)
);
const { getByText, queryByText, getAllByTestId } =
renderWithRouter(visEmbeddable);
getByText('Loading detectors...');
await waitFor(() => getByText('detector_name_0'));
getByText('5');
expect(queryByText('detector_name_1')).toBeNull();
userEvent.click(getAllByTestId('unlinkButton')[0]);
await waitFor(() =>
getByText(
'Removing association unlinks detector_name_0 detector from the visualization but does not delete it. The detector association can be restored.'
)
);
userEvent.click(getAllByTestId('confirmUnlinkButton')[0]);
expect(
(
await getAugmentVisSavedObjs(
'valid-obj-id-0',
augmentVisLoader,
uiSettingsMock
)
).length
).toEqual(2);
await waitFor(() => expect(mockDeleteFn).toHaveBeenCalledTimes(1));
}, 100000);
});
});

//I have a new beforeEach because I making a lot more detectors and saved objects for these tests
describe('test over 10 associated objects functionality', () => {
let augmentVisLoader: SavedObjectLoaderAugmentVis;
let mockDeleteFn: jest.Mock;
const detectorsToAssociate = new Array(16).fill(null).map((_, index) => {
const hasAnomaly = Math.random() > 0.5;
return {
id: `detector_id_${index}`,
name: `detector_name_${index}`,
indices: [`index_${index}`],
totalAnomalies: hasAnomaly ? Math.floor(Math.random() * 10) : 0,
lastActiveAnomaly: hasAnomaly ? Date.now() + index : 0,
};
});

const savedObjects = new Array(16).fill(null).map((_, index) => {
const pluginResource = {
type: 'test-plugin',
id: `detector_id_${index}`,
};
return generateAugmentVisSavedObject(
`valid-obj-id-${index}`,
fn,
`vis-id-${index}`,
originPlugin,
pluginResource
);
});
beforeEach(() => {
mockDeleteFn = jest.fn().mockResolvedValue('someValue');
augmentVisLoader = createSavedAugmentVisLoader({
savedObjectsClient: {
...getMockAugmentVisSavedObjectClient(savedObjects),
delete: mockDeleteFn,
},
} as any) as SavedObjectLoaderAugmentVis;
setSavedFeatureAnywhereLoader(augmentVisLoader);
});
test('create 20 detectors and saved objects', async () => {
httpClientMock.get = jest.fn().mockResolvedValue({
ok: true,
response: {
detectorList: detectorsToAssociate,
totalDetectors: detectorsToAssociate.length,
},
});
(getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
Promise.resolve(savedObjects)
);

const { getByText, queryByText, getAllByTestId, findByText } =
renderWithRouter(visEmbeddable);

await waitFor(() =>
findByText('detector_name_1', undefined, { timeout: 200000 })
);
expect(queryByText('detector_name_15')).toBeNull();
// Navigate to next page
await waitFor(() =>
userEvent.click(getAllByTestId('pagination-button-next')[0])
);
await waitFor(() => findByText('detector_name_15'));

expect(queryByText('detector_name_0')).toBeNull();
// Navigate to previous page
await waitFor(() =>
userEvent.click(getAllByTestId('pagination-button-previous')[0])
);
getByText('detector_name_0');
expect(queryByText('detector_name_15')).toBeNull();
}, 200000);

test('searching functionality', async () => {
httpClientMock.get = jest.fn().mockResolvedValue({
ok: true,
response: {
detectorList: detectorsToAssociate,
totalDetectors: detectorsToAssociate.length,
},
});
(getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
Promise.resolve(savedObjects)
);

const { queryByText, getByPlaceholderText, findByText } =
renderWithRouter(visEmbeddable);

// initial load only first 10 detectors
await waitFor(() =>
findByText('detector_name_1', undefined, { timeout: 60000 })
);
expect(queryByText('detector_name_15')).toBeNull();

//Input search event
userEvent.type(getByPlaceholderText('Search...'), 'detector_name_15');
await waitFor(() => {
findByText('detector_name_15');
});
expect(queryByText('detector_name_1')).toBeNull();
}, 100000);

test('sorting functionality', async () => {
httpClientMock.get = jest.fn().mockResolvedValue({
ok: true,
response: {
detectorList: detectorsToAssociate,
totalDetectors: detectorsToAssociate.length,
},
});
(getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
Promise.resolve(savedObjects)
);

const { queryByText, getAllByTestId, findByText } =
renderWithRouter(visEmbeddable);

// initial load only first 10 detectors (string sort means detector_name_0 -> detector_name_9 show up)
await waitFor(() =>
findByText('detector_name_0', undefined, { timeout: 100000 })
);
expect(queryByText('detector_name_15')).toBeNull();

// Sort by name (string sorting)
userEvent.click(getAllByTestId('tableHeaderSortButton')[0]);
await waitFor(() =>
findByText('detector_name_15', undefined, { timeout: 150000 })
);
expect(queryByText('detector_name_9')).toBeNull();
}, 200000);
});
Original file line number Diff line number Diff line change
@@ -62,6 +62,7 @@ export const getColumns = ({ handleUnlinkDetectorAction }) =>
description: 'Remove association',
icon: 'unlink',
onClick: handleUnlinkDetectorAction,
'data-test-subj': 'unlinkButton',
},
],
},
Original file line number Diff line number Diff line change
@@ -413,17 +413,11 @@ function AddAnomalyDetector({
closeFlyout();
})
.catch((error) => {
notifications.toasts.addDanger(
prettifyErrorMessage(
`Error associating selected detector: ${error}`
)
);
notifications.toasts.addDanger(prettifyErrorMessage(error));
});
})
.catch((error) => {
notifications.toasts.addDanger(
prettifyErrorMessage(`Error associating selected detector: ${error}`)
);
notifications.toasts.addDanger(prettifyErrorMessage(error));
});
};

Original file line number Diff line number Diff line change
@@ -206,7 +206,7 @@ export function AssociateExisting(
options={options}
selectedOptions={selectedOptions}
onChange={(selectedOptions) => {
let detector = {} as DetectorListItem | undefined;
let detector = undefined as DetectorListItem | undefined;

if (selectedOptions && selectedOptions.length) {
const match = existingDetectorsAvailableToAssociate.find(
@@ -234,7 +234,7 @@ export function AssociateExisting(
</EuiText>
<EuiSpacer size="s" />
<EuiHealth color={stateToColorMap.get(detector.curState)}>
{renderTime(detector.enabledTime)}
Running since {renderTime(detector.enabledTime)}
</EuiHealth>
</EuiFlexItem>
<EuiFlexItem grow={false}>
2 changes: 2 additions & 0 deletions public/expressions/__tests__/overlay_anomalies.test.ts
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ import {
} from '../../pages/utils/__tests__/constants';
import {
DETECTOR_HAS_BEEN_DELETED,
PLUGIN_EVENT_TYPE,
START_OR_END_TIME_INVALID_ERROR,
VIS_LAYER_PLUGIN_TYPE,
} from '../constants';
@@ -104,6 +105,7 @@ describe('overlay_anomalies spec', () => {
type: 'Anomaly Detectors',
urlPath: `anomaly-detection-dashboards#/detectors/${ANOMALY_RESULT_SUMMARY_DETECTOR_ID}/results`,
},
pluginEventType: PLUGIN_EVENT_TYPE,
type: 'PointInTimeEvents',
};
const pointInTimeEventsVisLayer =
2 changes: 1 addition & 1 deletion public/expressions/helpers.ts
Original file line number Diff line number Diff line change
@@ -83,7 +83,7 @@ export const convertAnomaliesToPointInTimeEventsVisLayer = (
type: VisLayerTypes.PointInTimeEvents,
pluginResource: ADPluginResource,
events: events,
pluginEventType: PLUGIN_EVENT_TYPE
pluginEventType: PLUGIN_EVENT_TYPE,
} as PointInTimeEventsVisLayer;
};

11 changes: 11 additions & 0 deletions public/services.ts
Original file line number Diff line number Diff line change
@@ -34,3 +34,14 @@ export const [getUiActions, setUiActions] =

export const [getUISettings, setUISettings] =
createGetterSetter<IUiSettingsClient>('UISettings');

// This is primarily used for mocking this module and each of its fns in tests.
export default {
getSavedFeatureAnywhereLoader,
getUISettings,
getUiActions,
getEmbeddable,
getNotifications,
getOverlays,
setUISettings,
};

0 comments on commit b4de09a

Please sign in to comment.