Alerting: Improve tests setup (#73316)

2023-08-22 13:50:41 +02:00
parent 050b9d9a85
commit 7631b0cf3f
8 changed files with 175 additions and 75 deletions
@@ -1,22 +1,21 @@
 import { screen, waitFor, waitForElementToBeRemoved, within } from '@testing-library/react';
-import userEvent, { PointerEventsCheckLevel } from '@testing-library/user-event';
+import userEvent from '@testing-library/user-event';
 import React from 'react';
 import { renderRuleEditor, ui } from 'test/helpers/alertingRuleEditor';
 import { clickSelectOption } from 'test/helpers/selectOptionInTest';
 import { byRole } from 'testing-library-selector';

-import { setDataSourceSrv } from '@grafana/runtime';
 import { contextSrv } from 'app/core/services/context_srv';
-import { PromApplication } from 'app/types/unified-alerting-dto';

 import { searchFolders } from '../../manage-dashboards/state/actions';

-import { discoverFeatures } from './api/buildInfo';
 import { fetchRulerRules, fetchRulerRulesGroup, fetchRulerRulesNamespace, setRulerRuleGroup } from './api/ruler';
 import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor';
-import { disableRBAC, mockDataSource, MockDataSourceSrv } from './mocks';
+import { mockApi, mockFeatureDiscoveryApi, setupMswServer } from './mockApi';
+import { disableRBAC, mockDataSource } from './mocks';
 import { fetchRulerRulesIfNotFetchedYet } from './state/actions';
-import * as config from './utils/config';
+import { setupDataSources } from './testSetup/datasources';
+import { buildInfoResponse } from './testSetup/featureDiscovery';

 jest.mock('./components/rule-editor/ExpressionEditor', () => ({
  // eslint-disable-next-line react/display-name
@@ -25,17 +24,9 @@ jest.mock('./components/rule-editor/ExpressionEditor', () => ({
  ),
 }));

-jest.mock('./api/buildInfo');
 jest.mock('./api/ruler');
 jest.mock('../../../../app/features/manage-dashboards/state/actions');

-// there's no angular scope in test and things go terribly wrong when trying to render the query editor row.
-// lets just skip it
-jest.mock('app/features/query/components/QueryEditorRow', () => ({
-  // eslint-disable-next-line react/display-name
-  QueryEditorRow: () => <p>hi</p>,
-}));
-
 jest.mock('./components/rule-editor/util', () => {
  const originalModule = jest.requireActual('./components/rule-editor/util');
  return {
@@ -45,29 +36,19 @@ jest.mock('./components/rule-editor/util', () => {
 });

 const dataSources = {
-  default: mockDataSource(
-    {
-      type: 'prometheus',
-      name: 'Prom',
-      isDefault: true,
-    },
-    { alerting: true }
-  ),
+  default: mockDataSource({ type: 'prometheus', name: 'Prom', isDefault: true }, { alerting: true }),
 };

-jest.mock('@grafana/runtime', () => ({
-  ...jest.requireActual('@grafana/runtime'),
-  getDataSourceSrv: jest.fn(() => ({
-    getInstanceSettings: () => dataSources.default,
-    get: () => dataSources.default,
-  })),
-}));
-
 jest.mock('app/core/components/AppChrome/AppChromeUpdate', () => ({
  AppChromeUpdate: ({ actions }: { actions: React.ReactNode }) => <div>{actions}</div>,
 }));

-jest.spyOn(config, 'getAllDataSources');
+setupDataSources(dataSources.default);
+
+const server = setupMswServer();
+
+mockFeatureDiscoveryApi(server).discoverDsFeatures(dataSources.default, buildInfoResponse.mimir);
+mockApi(server).eval({ results: {} });

 // these tests are rather slow because we have to wait for various API calls and mocks to be called
 // and wait for the UI to be in particular states, drone seems to time out quite often so
@@ -76,10 +57,8 @@ jest.spyOn(config, 'getAllDataSources');
 jest.setTimeout(60 * 1000);

 const mocks = {
-  getAllDataSources: jest.mocked(config.getAllDataSources),
  searchFolders: jest.mocked(searchFolders),
  api: {
-    discoverFeatures: jest.mocked(discoverFeatures),
    fetchRulerRulesGroup: jest.mocked(fetchRulerRulesGroup),
    setRulerRuleGroup: jest.mocked(setRulerRuleGroup),
    fetchRulerRulesNamespace: jest.mocked(fetchRulerRulesNamespace),
@@ -100,8 +79,6 @@ describe('RuleEditor cloud', () => {
  disableRBAC();

  it('can create a new cloud alert', async () => {
-    setDataSourceSrv(new MockDataSourceSrv(dataSources));
-    mocks.getAllDataSources.mockReturnValue(Object.values(dataSources));
    mocks.api.setRulerRuleGroup.mockResolvedValue();
    mocks.api.fetchRulerRulesNamespace.mockResolvedValue([]);
    mocks.api.fetchRulerRulesGroup.mockResolvedValue({
@@ -124,12 +101,7 @@ describe('RuleEditor cloud', () => {
    });
    mocks.searchFolders.mockResolvedValue([]);

-    mocks.api.discoverFeatures.mockResolvedValue({
-      application: PromApplication.Cortex,
-      features: {
-        rulerApiEnabled: true,
-      },
-    });
+    const user = userEvent.setup();

    renderRuleEditor();
    await waitForElementToBeRemoved(screen.getAllByTestId('Spinner'));
@@ -137,10 +109,13 @@ describe('RuleEditor cloud', () => {
    const removeExpressionsButtons = screen.getAllByLabelText('Remove expression');
    expect(removeExpressionsButtons).toHaveLength(2);

+    // Needs to wait for featrue discovery API call to finish - Check if ruler enabled
+    await waitFor(() => expect(screen.getByText('Switch to data source-managed alert rule')).toBeInTheDocument());
+
    const switchToCloudButton = screen.getByText('Switch to data source-managed alert rule');
    expect(switchToCloudButton).toBeInTheDocument();

-    await userEvent.click(switchToCloudButton);
+    await user.click(switchToCloudButton);

    //expressions are removed after switching to data-source managed
    expect(screen.queryAllByLabelText('Remove expression')).toHaveLength(0);
@@ -148,30 +123,30 @@ describe('RuleEditor cloud', () => {
    expect(screen.getByTestId('datasource-picker')).toBeInTheDocument();

    const dataSourceSelect = ui.inputs.dataSource.get();
-    await userEvent.click(byRole('combobox').get(dataSourceSelect));
+    await user.click(byRole('combobox').get(dataSourceSelect));
    await clickSelectOption(dataSourceSelect, 'Prom (default)');
    await waitFor(() => expect(mocks.api.fetchRulerRules).toHaveBeenCalled());

-    await userEvent.type(await ui.inputs.expr.find(), 'up == 1');
+    await user.type(await ui.inputs.expr.find(), 'up == 1');

-    await userEvent.type(ui.inputs.name.get(), 'my great new rule');
+    await user.type(ui.inputs.name.get(), 'my great new rule');
    await clickSelectOption(ui.inputs.namespace.get(), 'namespace2');
    await clickSelectOption(ui.inputs.group.get(), 'group2');

-    await userEvent.type(ui.inputs.annotationValue(0).get(), 'some summary');
-    await userEvent.type(ui.inputs.annotationValue(1).get(), 'some description');
+    await user.type(ui.inputs.annotationValue(0).get(), 'some summary');
+    await user.type(ui.inputs.annotationValue(1).get(), 'some description');

    // TODO remove skipPointerEventsCheck once https://github.com/jsdom/jsdom/issues/3232 is fixed
-    await userEvent.click(ui.buttons.addLabel.get(), { pointerEventsCheck: PointerEventsCheckLevel.Never });
+    await user.click(ui.buttons.addLabel.get());

-    await userEvent.type(getLabelInput(ui.inputs.labelKey(0).get()), 'severity{enter}');
-    await userEvent.type(getLabelInput(ui.inputs.labelValue(0).get()), 'warn{enter}');
+    await user.type(getLabelInput(ui.inputs.labelKey(0).get()), 'severity{enter}');
+    await user.type(getLabelInput(ui.inputs.labelValue(0).get()), 'warn{enter}');

    // save and check what was sent to backend
-    await userEvent.click(ui.buttons.save.get());
+    await user.click(ui.buttons.save.get());
    await waitFor(() => expect(mocks.api.setRulerRuleGroup).toHaveBeenCalled());
    expect(mocks.api.setRulerRuleGroup).toHaveBeenCalledWith(
-      { dataSourceName: 'Prom', apiVersion: 'legacy' },
+      { dataSourceName: 'Prom', apiVersion: 'config' },
      'namespace2',
      {
        name: 'group2',
@@ -0,0 +1,37 @@
+# Alerting testing
+
+## Mocking API requests
+
+We should strive to use **MSW** for mocking API as often as possible.
+It gives us the closest behaviour to the real server.
+
+`public/app/features/alerting/unified/mockApi.ts` contains helper functions that speed up mocking API configuration with MSW.
+
+If you don't find a helper for an endpoint you're looking for, please add it.
+
+**Mocking using MSW forces developers to handle loading states in tests which gives us a chance to discover UI inconsistencies at very early stages**
+
+### Common API requests
+
+- `/buildinfo`
+  Use `mockFeatureDiscoveryApi` and `buildInfoResponse` object to mock the endpoint response according to your needs
+- `api/v1/eval` used by AlertingQueryRunner
+  Use `mockApi.eval` Usually an empty response should do the trick
+
+## Mocking data sources
+
+`public/app/features/alerting/unified/testSetup/datasources.ts` file contains functions facilitating setting up mock data sources.
+
+## Mocking permissions
+
+By default tests should be written with RBAC enabled. This is the most common scenario for our users.
+Testing with RBAC disabled should be considered as an additional option when we already have tests for enabled RBAC.
+
+To enable or disable Role Based Access Control in tests use
+`enableRBAC` or `disableRBAC` from `public/app/features/alerting/unified/mocks.ts`
+
+To grant a permission to a user use `grantUserPermission` from the same file.
+
+## Common patterns
+
+TODO
@@ -3,7 +3,7 @@ import React from 'react';
 import { TestProvider } from 'test/helpers/TestProvider';
 import { byRole, byText } from 'testing-library-selector';

-import { config, locationService, setBackendSrv, setDataSourceSrv } from '@grafana/runtime';
+import { locationService, setBackendSrv } from '@grafana/runtime';
 import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
 import { backendSrv } from 'app/core/services/backend_srv';
 import { contextSrv } from 'app/core/services/context_srv';
@@ -20,13 +20,13 @@ import {
  getGrafanaRule,
  grantUserPermissions,
  mockDataSource,
-  MockDataSourceSrv,
  mockPromAlertingRule,
  mockRulerAlertingRule,
  promRuleFromRulerRule,
 } from '../../mocks';
 import { mockAlertmanagerChoiceResponse } from '../../mocks/alertmanagerApi';
 import { mockPluginSettings } from '../../mocks/plugins';
+import { setupDataSources } from '../../testSetup/datasources';
 import { SupportedPlugin } from '../../types/pluginBridges';
 import * as ruleId from '../../utils/rule-id';

@@ -47,20 +47,6 @@ const mockRoute = (id?: string): GrafanaRouteComponentProps<{ id?: string; sourc
  staticContext: {},
 });

-// jest.mock('../../hooks/useCombinedRule');
-jest.mock('@grafana/runtime', () => ({
-  ...jest.requireActual('@grafana/runtime'),
-  getDataSourceSrv: () => {
-    return {
-      getInstanceSettings: () => ({ name: 'prometheus' }),
-      get: () =>
-        Promise.resolve({
-          filterQuery: () => true,
-        }),
-    };
-  },
-}));
-
 jest.mock('../../hooks/useIsRuleEditable');
 jest.mock('../../api/buildInfo');

@@ -105,15 +91,12 @@ beforeAll(() => {
  // we need to mock this one for the "declare incident" button
  mockPluginSettings(server, SupportedPlugin.Incident);

-  const dsSettings = mockDataSource({
+  const promDsSettings = mockDataSource({
    name: dsName,
    uid: dsName,
  });
-  config.datasources = {
-    [dsName]: dsSettings,
-  };

-  setDataSourceSrv(new MockDataSourceSrv({ [dsName]: dsSettings }));
+  setupDataSources(promDsSettings);

  mockAlertRuleApi(server).rulerRules('grafana', {
    [mockGrafanaRule.namespace.name]: [
@@ -2,8 +2,14 @@ import { rest } from 'msw';
 import { setupServer, SetupServer } from 'msw/node';
 import 'whatwg-fetch';

+import { DataSourceInstanceSettings } from '@grafana/data';
 import { setBackendSrv } from '@grafana/runtime';
-import { PromRulesResponse, RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto';
+import {
+  PromBuildInfoResponse,
+  PromRulesResponse,
+  RulerRuleGroupDTO,
+  RulerRulesConfigDTO,
+} from 'app/types/unified-alerting-dto';

 import { backendSrv } from '../../../core/services/backend_srv';
 import {
@@ -15,6 +21,8 @@ import {
  Route,
 } from '../../../plugins/datasource/alertmanager/types';

+import { AlertingQueryResponse } from './state/AlertingQueryRunner';
+
 class AlertmanagerConfigBuilder {
  private alertmanagerConfig: AlertmanagerConfig = { receivers: [] };

@@ -122,6 +130,14 @@ export function mockApi(server: SetupServer) {
        )
      );
    },
+
+    eval: (response: AlertingQueryResponse) => {
+      server.use(
+        rest.post('/api/v1/eval', (_, res, ctx) => {
+          return res(ctx.status(200), ctx.json(response));
+        })
+      );
+    },
  };
 }

@@ -149,6 +165,24 @@ export function mockAlertRuleApi(server: SetupServer) {
  };
 }

+/**
+ * Used to mock the response from the /api/v1/status/buildinfo endpoint
+ */
+export function mockFeatureDiscoveryApi(server: SetupServer) {
+  return {
+    /**
+     *
+     * @param dsSettings Use `mockDataSource` to create a faks data source settings
+     * @param response Use `buildInfoResponse` to get a pre-defined response for Prometheus and Mimir
+     */
+    discoverDsFeatures: (dsSettings: DataSourceInstanceSettings, response: PromBuildInfoResponse) => {
+      server.use(
+        rest.get(`${dsSettings.url}/api/v1/status/buildinfo`, (_, res, ctx) => res(ctx.status(200), ctx.json(response)))
+      );
+    },
+  };
+}
+
 // Creates a MSW server and sets up beforeAll, afterAll and beforeEach handlers for it
 export function setupMswServer() {
  const server = setupServer();
@@ -1,12 +1,17 @@
 import { produce } from 'immer';
+import { Observable } from 'rxjs';

 import {
+  DataQuery,
+  DataQueryRequest,
+  DataQueryResponse,
  DataSourceApi,
  DataSourceInstanceSettings,
  DataSourceJsonData,
  DataSourcePluginMeta,
  DataSourceRef,
  ScopedVars,
+  TestDataSourceResponse,
 } from '@grafana/data';
 import { config, DataSourceSrv, GetDataSourceListFilters } from '@grafana/runtime';
 import { contextSrv } from 'app/core/services/context_srv';
@@ -56,12 +61,15 @@ export function mockDataSource<T extends DataSourceJsonData = DataSourceJsonData
 ): DataSourceInstanceSettings<T> {
  const id = partial.id ?? nextDataSourceId++;

+  const uid = partial.uid ?? `mock-ds-${nextDataSourceId}`;
+
  return {
    id,
-    uid: `mock-ds-${nextDataSourceId}`,
+    uid,
    type: 'prometheus',
    name: `Prometheus-${id}`,
    access: 'proxy',
+    url: `/api/datasources/proxy/uid/${uid}`,
    jsonData: {} as T,
    meta: {
      info: {
@@ -320,6 +328,20 @@ export const mockReceiversState = (partial: Partial<ReceiversState> = {}): Recei
  };
 };

+class MockDataSourceApi extends DataSourceApi {
+  constructor(instanceSettings: DataSourceInstanceSettings<DataSourceJsonData>) {
+    super(instanceSettings);
+  }
+
+  query(request: DataQueryRequest<DataQuery>): Promise<DataQueryResponse> | Observable<DataQueryResponse> {
+    throw new Error('Method not implemented.');
+  }
+  testDatasource(): Promise<TestDataSourceResponse> {
+    throw new Error('Method not implemented.');
+  }
+}
+
+// TODO This should be eventually moved to public/app/features/alerting/unified/testSetup/datasources.ts
 export class MockDataSourceSrv implements DataSourceSrv {
  datasources: Record<string, DataSourceApi> = {};
  // @ts-ignore
@@ -350,6 +372,7 @@ export class MockDataSourceSrv implements DataSourceSrv {
      if (dsSettings.isDefault) {
        this.defaultName = dsSettings.name;
      }
+      this.datasources[dsSettings.uid] = new MockDataSourceApi(dsSettings);
    }
  }

@@ -0,0 +1,16 @@
+import { keyBy } from 'lodash';
+
+import { DataSourceInstanceSettings } from '@grafana/data';
+import { config, setDataSourceSrv } from '@grafana/runtime';
+
+import { MockDataSourceSrv } from '../mocks';
+
+/**
+ * Sets up the data sources for the tests.
+ * Sets up both config object from grafana/runtime and the data source server
+ * @param configs data source instance settings. Use **mockDataSource** to create mock settings
+ */
+export function setupDataSources(...configs: DataSourceInstanceSettings[]) {
+  config.datasources = keyBy(configs, (c) => c.name);
+  setDataSourceSrv(new MockDataSourceSrv(config.datasources));
+}
@@ -0,0 +1,31 @@
+import { PromBuildInfoResponse } from 'app/types/unified-alerting-dto';
+
+export const buildInfoResponse: { prometheus: PromBuildInfoResponse; mimir: PromBuildInfoResponse } = {
+  prometheus: {
+    status: 'success',
+    data: {
+      version: '2.45.0',
+      revision: '8ef767e396bf8445f009f945b0162fd71827f445',
+      branch: 'HEAD',
+      buildUser: 'root@920118f645b7',
+      buildDate: '20230623-15:15:37',
+      goVersion: 'go1.20.5',
+    },
+  },
+  mimir: {
+    status: 'success',
+    data: {
+      application: 'Grafana Mimir',
+      version: 'r249-5bedc7a1-WIP',
+      revision: '5bedc7a1',
+      branch: 'weekly-r249',
+      goVersion: 'go1.20.5',
+      features: {
+        ruler_config_api: 'true',
+        alertmanager_config_api: 'true',
+        query_sharding: 'false',
+        federated_rules: 'false',
+      },
+    },
+  },
+};
@@ -79,6 +79,7 @@ export interface PromBuildInfoResponse {
      query_sharding?: 'true' | 'false';
      federated_rules?: 'true' | 'false';
    };
+    [key: string]: unknown;
  };
  status: 'success';
 }