From 22acbf220f33ca4d71248b87714e7d2520f4e6e0 Mon Sep 17 00:00:00 2001 From: Kaituo Li Date: Thu, 17 Oct 2024 16:16:39 -0700 Subject: [PATCH] Fix preview API not considering rules and imputation options (#898) This commit resolves a bug where the preview API did not account for suppression rules and imputation options. The fix involves passing additional parameters to handle these configurations correctly. Testing: * e2e testing completed. * UT added to cover the new logic. Signed-off-by: Kaituo Li --- .../workflows/remote-integ-tests-workflow.yml | 42 ++++-- .../containers/ConfigureModel.tsx | 2 + .../containers/SampleAnomalies.tsx | 8 +- .../__tests__/SampleAnomalies.test.tsx | 128 ++++++++++++++++++ .../utils/__tests__/helpers.test.tsx | 12 +- public/pages/ConfigureModel/utils/helpers.ts | 6 +- 6 files changed, 174 insertions(+), 24 deletions(-) create mode 100644 public/pages/ConfigureModel/containers/__tests__/SampleAnomalies.test.tsx diff --git a/.github/workflows/remote-integ-tests-workflow.yml b/.github/workflows/remote-integ-tests-workflow.yml index 63b87b46..7450f85f 100644 --- a/.github/workflows/remote-integ-tests-workflow.yml +++ b/.github/workflows/remote-integ-tests-workflow.yml @@ -71,36 +71,48 @@ jobs: node-version: ${{ steps.tool-versions.outputs.node_version }} registry-url: 'https://registry.npmjs.org' - - name: Setup Opensearch Dashboards + - name: Install correct yarn version for OpenSearch Dashboards + id: setup-yarn run: | npm uninstall -g yarn - echo "Installing yarn ${{ steps.tool-versions.outputs.yarn_version }}" - npm i -g yarn@${{ steps.tool-versions.outputs.yarn_version }} - yarn cache clean - yarn add sha.js - working-directory: OpenSearch-Dashboards - shell: bash + echo "Installing yarn ${{ steps.versions_step.outputs.yarn_version }}" + npm i -g yarn@${{ steps.versions_step.outputs.yarn_version }} - - name: Boodstrap Opensearch Dashboards + - name: Yarn Cache + uses: actions/cache@v4 + with: + path: | + OpenSearch-Dashboards/**/target + OpenSearch-Dashboards/**/node_modules + key: ${{ runner.OS }}-build-${{ hashFiles('OpenSearch-Dashboards/**/yarn.lock') }} + restore-keys: | + ${{ runner.OS }}-build- + + - name: Bootstrap OpenSearch Dashboards run: | + cd OpenSearch-Dashboards yarn osd bootstrap --single-version=loose - working-directory: OpenSearch-Dashboards/plugins/anomaly-detection-dashboards-plugin + + - name: Compile OpenSearch Dashboards + run: | + cd OpenSearch-Dashboards + node scripts/build_opensearch_dashboards_platform_plugins --no-examples --workers=10 --verbose - name: Run Opensearch Dashboards with AD Installed run: | nohup yarn start --no-base-path --no-watch --server.host="0.0.0.0" | tee dashboard.log & working-directory: OpenSearch-Dashboards - - name : Check If OpenSearch Dashboards Is Ready + - name: Check If OpenSearch Dashboards Is Ready if: ${{ runner.os == 'Linux' }} run: | - if timeout 600 grep -q "bundles compiled successfully after" <(tail -n0 -f dashboard.log); then - echo "OpenSearch Dashboards compiled successfully." + cd ./OpenSearch-Dashboards + if timeout 60 grep -q "http server running" <(tail -n +1 -f dashboard.log); then + echo "OpenSearch Dashboards started successfully." else - echo "Timeout for 600 seconds reached. OpenSearch Dashboards did not finish compiling." + echo "Timeout of 60 seconds reached. OpenSearch Dashboards did not start successfully." exit 1 - fi - working-directory: OpenSearch-Dashboards + fi& - name: Show OpenSearch Dashboards Logs if: always() diff --git a/public/pages/ConfigureModel/containers/ConfigureModel.tsx b/public/pages/ConfigureModel/containers/ConfigureModel.tsx index c7867869..1c0c0cd3 100644 --- a/public/pages/ConfigureModel/containers/ConfigureModel.tsx +++ b/public/pages/ConfigureModel/containers/ConfigureModel.tsx @@ -456,6 +456,8 @@ export function ConfigureModel(props: ConfigureModelProps) { categoryFields={formikProps.values.categoryField} errors={formikProps.errors} setFieldTouched={formikProps.setFieldTouched} + imputationOption={formikProps.values.imputationOption} + suppressionRules={formikProps.values.suppressionRules} /> ) : null} diff --git a/public/pages/ConfigureModel/containers/SampleAnomalies.tsx b/public/pages/ConfigureModel/containers/SampleAnomalies.tsx index 9954573e..5a06dbdc 100644 --- a/public/pages/ConfigureModel/containers/SampleAnomalies.tsx +++ b/public/pages/ConfigureModel/containers/SampleAnomalies.tsx @@ -44,7 +44,7 @@ import { } from '../../utils/anomalyResultUtils'; import { focusOnFirstWrongFeature } from '../utils/helpers'; import { prepareDetector } from '../utils/helpers'; -import { FeaturesFormikValues } from '../models/interfaces'; +import { FeaturesFormikValues, ImputationFormikValues, RuleFormikValues} from '../models/interfaces'; import { BASE_DOCS_LINK } from '../../../utils/constants'; import { prettifyErrorMessage } from '../../../../server/utils/helpers'; import { CoreStart } from '../../../../../../src/core/public'; @@ -59,6 +59,8 @@ interface SampleAnomaliesProps { categoryFields: string[]; errors: any; setFieldTouched: any; + imputationOption?: ImputationFormikValues; + suppressionRules?: RuleFormikValues[]; } export function SampleAnomalies(props: SampleAnomaliesProps) { @@ -183,7 +185,9 @@ export function SampleAnomalies(props: SampleAnomaliesProps) { props.shingleSize, props.categoryFields, newDetector, - true + true, + props.imputationOption, + props.suppressionRules, ); setPreviewDone(false); setZoomRange({ ...dateRange }); diff --git a/public/pages/ConfigureModel/containers/__tests__/SampleAnomalies.test.tsx b/public/pages/ConfigureModel/containers/__tests__/SampleAnomalies.test.tsx new file mode 100644 index 00000000..39d91482 --- /dev/null +++ b/public/pages/ConfigureModel/containers/__tests__/SampleAnomalies.test.tsx @@ -0,0 +1,128 @@ +// SPDX-License-Identifier: Apache-2.0 + +import React from 'react'; +import { Provider } from 'react-redux'; +import { + HashRouter as Router, + RouteComponentProps, + Route, + Switch, +} from 'react-router-dom'; +import { render, fireEvent, waitFor } from '@testing-library/react'; +import { SampleAnomalies } from '../SampleAnomalies'; +import configureStore from '../../../../redux/configureStore'; +import { httpClientMock, coreServicesMock } from '../../../../../test/mocks'; +import { mockedStore } from '../../../../redux/utils/testUtils'; +import { CoreServicesContext } from '../../../../components/CoreServices/CoreServices'; +import { prepareDetector, focusOnFirstWrongFeature } from '../../utils/helpers'; +import { createMemoryHistory } from 'history'; +import { + FeaturesFormikValues, + ImputationFormikValues, + RuleFormikValues, +} from '../../../../pages/ConfigureModel/models/interfaces'; +import { getRandomDetector } from '../../../../redux/reducers/__tests__/utils'; +import { FEATURE_TYPE } from '../../../../models/interfaces'; + +// Mock the helper functions +jest.mock('../../utils/helpers', () => ({ + ...jest.requireActual('../../utils/helpers'), + prepareDetector: jest.fn(() => ({ + // mock a detector object with at least an id, preventing the undefined error when detector.id is accessed in getSampleAdResult. + id: 'test-detector-id', + name: 'test-detector', + description: 'test-detector-description', + })), + focusOnFirstWrongFeature: jest.fn(() => false), +})); + +// Mock the Redux actions if necessary +jest.mock('../../../../redux/reducers/previewAnomalies', () => ({ + previewDetector: jest.fn(() => async () => Promise.resolve()), +})); + +describe(' spec', () => { + beforeEach(() => { + jest.clearAllMocks(); + console.error = jest.fn(); + console.warn = jest.fn(); + }); + + test('calls prepareDetector with imputationOption and suppressionRules when previewDetector button is clicked', async () => { + // Set up the mock props + const mockDetector = getRandomDetector(); + + const mockFeatureList: FeaturesFormikValues[] = [ + { + featureId: 'feature1', + featureName: 'Feature 1', + featureEnabled: true, + aggregationBy: 'sum', + aggregationOf: [{ label: 'bytes' }], + featureType: FEATURE_TYPE.SIMPLE, + aggregationQuery: '', + }, + ]; + + const mockImputationOption: ImputationFormikValues = { + imputationMethod: 'zero', + }; + + const mockSuppressionRules: RuleFormikValues[] = [ + { + featureName: '', + absoluteThreshold: undefined, + relativeThreshold: undefined, + aboveBelow: 'above', + }, + ]; + + const props = { + detector: mockDetector, + featureList: mockFeatureList, + shingleSize: 8, + categoryFields: ['category1'], + errors: {}, + setFieldTouched: jest.fn(), + imputationOption: mockImputationOption, + suppressionRules: mockSuppressionRules, + }; + + // Create a mock history and store + const history = createMemoryHistory(); + const initialState = { + anomalies: { + anomaliesResult: { + anomalies: [], + }, + }, + }; + const store = mockedStore(); + + const { getByText } = render( + + + + + + + + ); + + // Simulate clicking the previewDetector button + fireEvent.click(getByText('Preview anomalies')); + + // Wait for async actions to complete + await waitFor(() => { + expect(prepareDetector).toHaveBeenCalledWith( + props.featureList, + props.shingleSize, + props.categoryFields, + expect.anything(), // newDetector (could be the same as props.detector or modified) + true, + props.imputationOption, + props.suppressionRules + ); + }); + }); +}); diff --git a/public/pages/ConfigureModel/utils/__tests__/helpers.test.tsx b/public/pages/ConfigureModel/utils/__tests__/helpers.test.tsx index 0f8798c0..c78e4332 100644 --- a/public/pages/ConfigureModel/utils/__tests__/helpers.test.tsx +++ b/public/pages/ConfigureModel/utils/__tests__/helpers.test.tsx @@ -30,12 +30,12 @@ describe('featuresToFormik', () => { aggregationQuery: '', }; const randomPositiveInt = Math.ceil(Math.random() * 100); - const apiRequest = prepareDetector( - [newFeature], - randomPositiveInt, - randomDetector, - false - ); + // const apiRequest = prepareDetector( + // [newFeature], + // randomPositiveInt, + // randomDetector, + // false + // ); // expect(apiRequest.featureAttributes).toEqual([ // { // featureId: newFeature.featureId, diff --git a/public/pages/ConfigureModel/utils/helpers.ts b/public/pages/ConfigureModel/utils/helpers.ts index c88c135e..7df261a1 100644 --- a/public/pages/ConfigureModel/utils/helpers.ts +++ b/public/pages/ConfigureModel/utils/helpers.ts @@ -340,7 +340,9 @@ export function prepareDetector( shingleSizeValue: number, categoryFields: string[], ad: Detector, - forPreview: boolean = false + forPreview: boolean = false, + imputationOption?: ImputationFormikValues, + suppressionRules?: RuleFormikValues[] ): Detector { const detector = cloneDeep(ad); const featureAttributes = formikToFeatures(featureValues, forPreview); @@ -354,6 +356,8 @@ export function prepareDetector( ...detector.uiMetadata, features: { ...featuresToUIMetadata(featureValues) }, }, + imputationOption: formikToImputationOption(imputationOption), + rules: formikToRules(suppressionRules), }; }