From 1a683e53c51419a931a19c8d9869024e14d28b76 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Tue, 2 Aug 2022 14:47:41 +0100 Subject: [PATCH] Typed variables pt2: Use type-accurate mock variables in tests (#52987) * wip * make diff easier to read * Update template_srv getVariables to return new TypedVariableModel * update VariableType to use the type from TypedVariableModel * tidy things up * Chore: Use type-accurate mock variables in tests * make createBaseVariableModel take the variable type --- .../features/variables/adhoc/actions.test.ts | 5 +- .../variables/state/__tests__/fixtures.ts | 83 ++++++++ .../app/features/variables/state/helpers.ts | 38 ++-- .../features/variables/state/reducers.test.ts | 77 ++----- .../variables/state/sharedReducer.test.ts | 192 +++--------------- .../state/templateVarsChangedInUrl.test.ts | 8 +- .../state/upgradeLegacyQueries.test.ts | 6 +- 7 files changed, 149 insertions(+), 260 deletions(-) create mode 100644 public/app/features/variables/state/__tests__/fixtures.ts diff --git a/public/app/features/variables/adhoc/actions.test.ts b/public/app/features/variables/adhoc/actions.test.ts index 22c3d49b1e0..b070c470385 100644 --- a/public/app/features/variables/adhoc/actions.test.ts +++ b/public/app/features/variables/adhoc/actions.test.ts @@ -1,6 +1,5 @@ -import { DataSourceInstanceSettings, DataSourcePluginMeta } from '@grafana/data'; +import { DataSourceInstanceSettings, DataSourcePluginMeta, TypedVariableModel } from '@grafana/data'; import { locationService } from '@grafana/runtime'; -import { VariableModel } from 'app/features/variables/types'; import { reduxTester } from '../../../../test/core/redux/reduxTester'; import { variableAdapters } from '../adapters'; @@ -499,7 +498,7 @@ describe('adhoc actions', () => { }); }); -function createAddVariableAction(variable: VariableModel, index = 0) { +function createAddVariableAction(variable: TypedVariableModel, index = 0) { const identifier = toKeyedVariableIdentifier(variable); const global = false; const data = { global, index, model: { ...variable, index: -1, global } }; diff --git a/public/app/features/variables/state/__tests__/fixtures.ts b/public/app/features/variables/state/__tests__/fixtures.ts new file mode 100644 index 00000000000..e8e7e6178fc --- /dev/null +++ b/public/app/features/variables/state/__tests__/fixtures.ts @@ -0,0 +1,83 @@ +import { + BaseVariableModel, + ConstantVariableModel, + CustomVariableModel, + LoadingState, + QueryVariableModel, + VariableHide, + VariableOption, + VariableRefresh, + VariableSort, + VariableType, +} from '@grafana/data'; + +function createBaseVariableModel(type: T): BaseVariableModel & { type: T } { + return { + name: 'myVariableName', + id: '0', + rootStateKey: 'key', + global: false, + hide: VariableHide.dontHide, + skipUrlSync: false, + index: 0, + state: LoadingState.NotStarted, + error: null, + description: null, + type, + }; +} + +export function createVariableOption(value: string, rest: Partial> = {}): VariableOption { + return { + value, + text: rest.text ?? value, + selected: rest.selected ?? false, + }; +} + +export function createQueryVariable(input: Partial = {}): QueryVariableModel { + return { + ...createBaseVariableModel('query'), + label: 'DefaultLabel', + datasource: { + uid: 'abc-123', + type: 'prometheus', + }, + definition: 'def', + sort: VariableSort.alphabeticalAsc, + query: 'label_values(job)', + regex: '', + refresh: VariableRefresh.onDashboardLoad, + multi: false, + includeAll: false, + current: createVariableOption('prom-prod', { text: 'Prometheus (main)', selected: true }), + options: [ + createVariableOption('prom-prod', { text: 'Prometheus (main)', selected: true }), + createVariableOption('prom-dev'), + ], + ...input, + }; +} + +export function createConstantVariable(input: Partial): ConstantVariableModel { + return { + ...createBaseVariableModel('constant'), + query: '', + current: createVariableOption('database'), + options: [], + hide: VariableHide.hideVariable, + ...input, + }; +} + +export function createCustomVariable(input: Partial): CustomVariableModel { + return { + ...createBaseVariableModel('custom'), + multi: false, + includeAll: false, + current: createVariableOption('prom-prod', { text: 'Prometheus (main)', selected: true }), + options: [], + query: '', + ...input, + }; +} diff --git a/public/app/features/variables/state/helpers.ts b/public/app/features/variables/state/helpers.ts index 70db7abdeb3..f1d296117bf 100644 --- a/public/app/features/variables/state/helpers.ts +++ b/public/app/features/variables/state/helpers.ts @@ -1,6 +1,6 @@ import { combineReducers } from '@reduxjs/toolkit'; -import { LoadingState } from '@grafana/data'; +import { TypedVariableModel } from '@grafana/data'; import { dashboardReducer } from 'app/features/dashboard/state/reducers'; import { DashboardState, StoreState } from '../../../types'; @@ -15,6 +15,7 @@ import { VariableModel, } from '../types'; +import { createQueryVariable } from './__tests__/fixtures'; import { keyedVariablesReducer, KeyedVariablesState } from './keyedVariablesReducer'; import { getInitialTemplatingState, TemplatingState } from './reducers'; import { VariablesState } from './types'; @@ -24,8 +25,8 @@ export const getVariableState = ( inEditorIndex = -1, includeEmpty = false, includeSystem = false -): Record => { - const variables: Record = {}; +): VariablesState => { + const variables: Record = {}; if (includeSystem) { const dashboardModel: DashboardVariableModel = { @@ -86,43 +87,27 @@ export const getVariableState = ( } for (let index = 0; index < noOfVariables; index++) { - variables[index] = { + variables[index] = createQueryVariable({ id: index.toString(), - rootStateKey: 'key', - type: 'query', name: `Name-${index}`, - hide: VariableHide.dontHide, - index, label: `Label-${index}`, - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, - }; + index, + }); } if (includeEmpty) { - variables[NEW_VARIABLE_ID] = { + variables[NEW_VARIABLE_ID] = createQueryVariable({ id: NEW_VARIABLE_ID, - rootStateKey: 'key', - type: 'query', name: `Name-${NEW_VARIABLE_ID}`, - hide: VariableHide.dontHide, - index: noOfVariables, label: `Label-${NEW_VARIABLE_ID}`, - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, - }; + index: noOfVariables, + }); } return variables; }; -export const getVariableTestContext = ( +export const getVariableTestContext = ( adapter: VariableAdapter, variableOverrides: Partial = {} ) => { @@ -132,6 +117,7 @@ export const getVariableTestContext = ( index: 0, name: '0', }; + const defaultVariable = { ...adapter.initialState, ...defaults, diff --git a/public/app/features/variables/state/reducers.test.ts b/public/app/features/variables/state/reducers.test.ts index fdf61d8cbab..2ab57e974e3 100644 --- a/public/app/features/variables/state/reducers.test.ts +++ b/public/app/features/variables/state/reducers.test.ts @@ -4,9 +4,10 @@ import { VariableType } from '@grafana/data'; import { reducerTester } from '../../../../test/core/redux/reducerTester'; import { VariableAdapter, variableAdapters } from '../adapters'; -import { initialVariableModelState, QueryVariableModel } from '../types'; +import { QueryVariableModel } from '../types'; import { toVariablePayload } from '../utils'; +import { createQueryVariable } from './__tests__/fixtures'; import { VariablePayload, VariablesState } from './types'; import { cleanVariables, variablesReducer } from './variablesReducer'; @@ -32,64 +33,43 @@ describe('variablesReducer', () => { describe('when cleanUpDashboard is dispatched', () => { it('then all variables except global variables should be removed', () => { const initialState: VariablesState = { - '0': { - ...initialVariableModelState, + '0': createQueryVariable({ id: '0', index: 0, - type: 'query', name: 'Name-0', label: 'Label-0', - }, - '1': { - ...initialVariableModelState, + }), + + '1': createQueryVariable({ id: '1', index: 1, - type: 'query', name: 'Name-1', label: 'Label-1', global: true, - }, - '2': { - ...initialVariableModelState, + }), + + '2': createQueryVariable({ id: '2', index: 2, - type: 'query', name: 'Name-2', label: 'Label-2', - }, - '3': { - ...initialVariableModelState, + }), + + '3': createQueryVariable({ id: '3', index: 3, - type: 'query', name: 'Name-3', label: 'Label-3', global: true, - }, + }), }; reducerTester() .givenReducer(variablesReducer, initialState) .whenActionIsDispatched(cleanVariables()) .thenStateShouldEqual({ - '1': { - ...initialVariableModelState, - id: '1', - index: 1, - type: 'query', - name: 'Name-1', - label: 'Label-1', - global: true, - }, - '3': { - ...initialVariableModelState, - id: '3', - index: 3, - type: 'query', - name: 'Name-3', - label: 'Label-3', - global: true, - }, + '1': initialState['1'], + '3': initialState['3'], }); }); }); @@ -97,14 +77,7 @@ describe('variablesReducer', () => { describe('when any action is dispatched with a type prop that is registered in variableAdapters', () => { it('then the reducer for that variableAdapter should be invoked', () => { const initialState: VariablesState = { - '0': { - ...initialVariableModelState, - id: '0', - index: 0, - type: 'query', - name: 'Name-0', - label: 'Label-0', - }, + '0': createQueryVariable({ id: '0' }), }; variableAdapters.get('mock').reducer = jest.fn().mockReturnValue(initialState); const mockAction = createAction('mockAction'); @@ -123,14 +96,7 @@ describe('variablesReducer', () => { describe('when any action is dispatched with a type prop that is not registered in variableAdapters', () => { it('then the reducer for that variableAdapter should be invoked', () => { const initialState: VariablesState = { - '0': { - ...initialVariableModelState, - id: '0', - index: 0, - type: 'query', - name: 'Name-0', - label: 'Label-0', - }, + '0': createQueryVariable({ id: '0' }), }; variableAdapters.get('mock').reducer = jest.fn().mockReturnValue(initialState); const mockAction = createAction('mockAction'); @@ -145,14 +111,7 @@ describe('variablesReducer', () => { describe('when any action is dispatched missing type prop', () => { it('then the reducer for that variableAdapter should be invoked', () => { const initialState: VariablesState = { - '0': { - ...initialVariableModelState, - id: '0', - index: 0, - type: 'query', - name: 'Name-0', - label: 'Label-0', - }, + '0': createQueryVariable({ id: '0' }), }; variableAdapters.get('mock').reducer = jest.fn().mockReturnValue(initialState); const mockAction = createAction('mockAction'); diff --git a/public/app/features/variables/state/sharedReducer.test.ts b/public/app/features/variables/state/sharedReducer.test.ts index fd795ed798a..5b7b3cbdca8 100644 --- a/public/app/features/variables/state/sharedReducer.test.ts +++ b/public/app/features/variables/state/sharedReducer.test.ts @@ -10,9 +10,10 @@ import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE } from '../constants'; import { changeVariableNameSucceeded } from '../editor/reducer'; import { createQueryVariableAdapter } from '../query/adapter'; import { initialQueryVariableModelState } from '../query/reducer'; -import { ConstantVariableModel, QueryVariableModel, VariableHide, VariableOption } from '../types'; +import { ConstantVariableModel, QueryVariableModel, VariableOption } from '../types'; import { toVariablePayload } from '../utils'; +import { createConstantVariable } from './__tests__/fixtures'; import { getVariableState, getVariableTestContext } from './helpers'; import { addVariable, @@ -97,39 +98,20 @@ describe('sharedReducer', () => { describe('when removeVariable is dispatched and reIndex is true', () => { it('then state should be correct', () => { - const initialState: VariablesState = getVariableState(3); + const initialState = getVariableState(3); + const payload = toVariablePayload({ id: '1', type: 'query' }, { reIndex: true }); reducerTester() .givenReducer(sharedReducer, initialState) .whenActionIsDispatched(removeVariable(payload)) .thenStateShouldEqual({ '0': { - id: '0', - rootStateKey: 'key', - type: 'query', - name: 'Name-0', - hide: VariableHide.dontHide, + ...initialState['0'], index: 0, - label: 'Label-0', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, }, '2': { - id: '2', - rootStateKey: 'key', - type: 'query', - name: 'Name-2', - hide: VariableHide.dontHide, + ...initialState['2'], index: 1, - label: 'Label-2', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, }, }); }); @@ -139,38 +121,13 @@ describe('sharedReducer', () => { it('then state should be correct', () => { const initialState: VariablesState = getVariableState(3); const payload = toVariablePayload({ id: '1', type: 'query' }, { reIndex: false }); + reducerTester() .givenReducer(sharedReducer, initialState) .whenActionIsDispatched(removeVariable(payload)) .thenStateShouldEqual({ - '0': { - id: '0', - rootStateKey: 'key', - type: 'query', - name: 'Name-0', - hide: VariableHide.dontHide, - index: 0, - label: 'Label-0', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, - }, - '2': { - id: '2', - rootStateKey: 'key', - type: 'query', - name: 'Name-2', - hide: VariableHide.dontHide, - index: 2, - label: 'Label-2', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, - }, + '0': initialState['0'], + '2': initialState['2'], }); }); }); @@ -184,55 +141,12 @@ describe('sharedReducer', () => { .whenActionIsDispatched(duplicateVariable(payload)) .thenStateShouldEqual({ ...initialState, - '0': { - id: '0', - rootStateKey: 'key', - type: 'query', - name: 'Name-0', - hide: VariableHide.dontHide, - index: 0, - label: 'Label-0', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, - }, - '1': { - id: '1', - rootStateKey: 'key', - type: 'query', - name: 'Name-1', - hide: VariableHide.dontHide, - index: 1, - label: 'Label-1', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, - }, - '2': { - id: '2', - rootStateKey: 'key', - type: 'query', - name: 'Name-2', - hide: VariableHide.dontHide, - index: 2, - label: 'Label-2', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, - }, '11': { ...initialQueryVariableModelState, + ...initialState['1'], id: '11', - rootStateKey: 'key', name: 'copy_of_Name-1', index: 3, - label: 'Label-1', }, }); }); @@ -247,46 +161,16 @@ describe('sharedReducer', () => { .whenActionIsDispatched(changeVariableOrder(payload)) .thenStateShouldEqual({ '0': { - id: '0', - rootStateKey: 'key', - type: 'query', - name: 'Name-0', - hide: VariableHide.dontHide, + ...initialState['0'], index: 1, - label: 'Label-0', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, }, '1': { - id: '1', - rootStateKey: 'key', - type: 'query', - name: 'Name-1', - hide: VariableHide.dontHide, + ...initialState['1'], index: 2, - label: 'Label-1', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, }, '2': { - id: '2', - rootStateKey: 'key', - type: 'query', - name: 'Name-2', - hide: VariableHide.dontHide, + ...initialState['2'], index: 0, - label: 'Label-2', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, }, }); }); @@ -299,46 +183,16 @@ describe('sharedReducer', () => { .whenActionIsDispatched(changeVariableOrder(payload)) .thenStateShouldEqual({ '0': { - id: '0', - rootStateKey: 'key', - type: 'query', - name: 'Name-0', - hide: VariableHide.dontHide, + ...initialState['0'], index: 2, - label: 'Label-0', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, }, '1': { - id: '1', - rootStateKey: 'key', - type: 'query', - name: 'Name-1', - hide: VariableHide.dontHide, + ...initialState['1'], index: 0, - label: 'Label-1', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, }, '2': { - id: '2', - rootStateKey: 'key', - type: 'query', - name: 'Name-2', - hide: VariableHide.dontHide, + ...initialState['2'], index: 1, - label: 'Label-2', - skipUrlSync: false, - global: false, - state: LoadingState.NotStarted, - error: null, - description: null, }, }); }); @@ -567,6 +421,7 @@ describe('sharedReducer', () => { const newType = 'constant' as VariableType; const identifier: KeyedVariableIdentifier = { id: '0', type: 'query', rootStateKey: 'key' }; const payload = toVariablePayload(identifier, { newType }); + reducerTester() .givenReducer(sharedReducer, cloneDeep(queryAdapterState)) .whenActionIsDispatched(changeVariableNameSucceeded(toVariablePayload(identifier, { newName: 'test' }))) @@ -581,11 +436,14 @@ describe('sharedReducer', () => { ...constantAdapterState, '0': { ...constantAdapterState[0], - rootStateKey: 'key', - name: 'test', - description: 'new description', - label: 'new label', - type: 'constant', + ...createConstantVariable({ + type: 'constant', + rootStateKey: 'key', + name: 'test', + description: 'new description', + label: 'new label', + current: {} as VariableOption, + }), }, }); }); diff --git a/public/app/features/variables/state/templateVarsChangedInUrl.test.ts b/public/app/features/variables/state/templateVarsChangedInUrl.test.ts index a98eee8f0d8..31c47aa710b 100644 --- a/public/app/features/variables/state/templateVarsChangedInUrl.test.ts +++ b/public/app/features/variables/state/templateVarsChangedInUrl.test.ts @@ -1,3 +1,5 @@ +import { TypedVariableModel } from '@grafana/data'; + import { DashboardState, StoreState } from '../../../types'; import { DashboardModel, PanelModel } from '../../dashboard/state'; import { initialState } from '../../dashboard/state/reducers'; @@ -5,7 +7,6 @@ import { variableAdapters } from '../adapters'; import { createConstantVariableAdapter } from '../constant/adapter'; import { createCustomVariableAdapter } from '../custom/adapter'; import { constantBuilder, customBuilder } from '../shared/testing/builders'; -import { VariableModel } from '../types'; import { ExtendedUrlQueryMap } from '../utils'; import { templateVarsChangedInUrl } from './actions'; @@ -16,7 +17,10 @@ const dashboardModel = new DashboardModel({}); variableAdapters.setInit(() => [createCustomVariableAdapter(), createConstantVariableAdapter()]); -async function getTestContext(urlQueryMap: ExtendedUrlQueryMap = {}, variable: VariableModel | undefined = undefined) { +async function getTestContext( + urlQueryMap: ExtendedUrlQueryMap = {}, + variable: TypedVariableModel | undefined = undefined +) { jest.clearAllMocks(); const key = 'key'; diff --git a/public/app/features/variables/state/upgradeLegacyQueries.test.ts b/public/app/features/variables/state/upgradeLegacyQueries.test.ts index bc4281ca926..65d7c4b6cf9 100644 --- a/public/app/features/variables/state/upgradeLegacyQueries.test.ts +++ b/public/app/features/variables/state/upgradeLegacyQueries.test.ts @@ -1,8 +1,8 @@ -import { VariableSupportType } from '@grafana/data'; +import { TypedVariableModel, VariableSupportType } from '@grafana/data'; import { thunkTester } from '../../../../test/core/thunk/thunkTester'; import { customBuilder, queryBuilder } from '../shared/testing/builders'; -import { TransactionStatus, VariableModel } from '../types'; +import { TransactionStatus } from '../types'; import { toKeyedVariableIdentifier } from '../utils'; import { upgradeLegacyQueries } from './actions'; @@ -12,7 +12,7 @@ import { changeVariableProp } from './sharedReducer'; interface Args { query?: any; - variable?: VariableModel; + variable?: TypedVariableModel; datasource?: any; transactionStatus?: TransactionStatus; }