From 7d0ce6be716a7a42c669be6c2c2963cca095d029 Mon Sep 17 00:00:00 2001 From: Niral Patel Date: Tue, 30 Aug 2022 23:50:27 +0000 Subject: [PATCH 1/5] Isolating problem files --- .../src/scenes/DiffScene/DiffScene.spec.tsx | 162 ++++++++++++++++++ .../api-explorer/src/state/settings/slice.ts | 6 + .../api-explorer/src/test-utils/redux.tsx | 6 +- 3 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx diff --git a/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx b/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx new file mode 100644 index 000000000..d875d0363 --- /dev/null +++ b/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx @@ -0,0 +1,162 @@ +/* + + MIT License + + Copyright (c) 2021 Looker Data Sciences, Inc. + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. + + */ +import React from 'react' +import { screen, waitFor, render } from '@testing-library/react' +import userEvent from '@testing-library/user-event' + +import type { SpecItem, SpecList } from '@looker/sdk-codegen' +import { useLocation } from 'react-router-dom' +import { useDispatch } from 'react-redux' +import { getLoadedSpecs } from '../../test-data' +import { + createTestStore, + renderWithRouter, + renderWithRouterAndReduxProvider, +} from '../../test-utils' +import { getApixAdaptor, MockedProvider, mockHistory } from '../../utils' +import { DiffScene } from './DiffScene' + +// jest.mock('react-redux', () => { +// const reactRedux = jest.requireActual('react-redux') +// return { +// __esModule: true, +// ...reactRedux, +// useDispatch: jest.fn(reactRedux.useDispatch), +// } +// }) + +// jest.mock('react-router-dom', () => { +// const ReactRouter = jest.requireActual('react-router-dom') +// +// return { +// __esModule: true, +// ...ReactRouter, +// useHistory: jest.fn().mockReturnValue({ +// push: jest.fn(), +// location, +// useLocation: jest +// .fn() +// .mockReturnValue({ pathname: '/4.0/diff/3.1', search: '' }), +// }), +// } +// }) + +const specs = getLoadedSpecs() as SpecList +// class MockApixAdaptor { +// async fetchSpec(spec: SpecItem) { +// return new Promise(() => specs[spec.key]) +// } +// } + +// const mockApixAdaptor = new MockApixAdaptor() +// jest.mock('../../utils/apixAdaptor', () => { +// const apixAdaptor = jest.requireActual('../../utils/apixAdaptor') +// return { +// __esModule: true, +// ...apixAdaptor, +// getApixAdaptor: jest.fn(), +// } +// }) + +describe('DiffScene', () => { + // const mockDispatch = jest.fn() + + // afterEach(() => { + // jest.clearAllMocks() + // }) + // ;(getApixAdaptor as jest.Mock).mockReturnValue(mockApixAdaptor) + Element.prototype.scrollTo = jest.fn() + Element.prototype.scrollIntoView = jest.fn() + + const toggleNavigation = () => false + test.only('updating url dispatches store action', () => { + // ;(useDispatch as jest.Mock).mockReturnValue(mockDispatch) + // TODO issue: executing createTestStore here doesn't init default settings + const store = createTestStore({ + specs: { specs, currentSpecKey: '4.0' }, + settings: { initialized: true }, + }) + const history = mockHistory({ + initialEntries: ['/4.0/diff/3.1'], + }) + const MockScene = ( + + + + ) + const { rerender } = render(MockScene) + // history.push('/4.0/diff/3.1?opts=missing') + // rerender(MockScene) + // expect(mockDispatch).toHaveBeenLastCalledWith({ + // payload: { diffOptions: ['missing'] }, + // type: 'settings/setDiffOptionsAction', + // }) + // TODO: test URL change leads to store dispatch? - change mock history push implementation to change our location + // TODO: test that toggling another will push both options to store/url + }) + + // test.todo( + // 'rendering scene with opts param in url sets selected options in selector', + // async () => { + // jest.spyOn(routerLocation, 'useLocation').mockReturnValue({ + // pathname: `/`, + // search: `opts=missing%2Ctype%2Cresponse`, + // } as unknown as Location) + // const store = createTestStore({ + // specs: { specs, currentSpecKey: '3.1' }, + // settings: { initialized: true, diffOptions: [] }, + // }) + // jest.spyOn(reactRedux, 'useDispatch').mockReturnValue(mockDispatch) + // renderWithRouterAndReduxProvider( + // , + // ['/diff/3.1'], + // store + // ) + // expect(mockDispatch).toHaveBeenLastCalledWith({ + // payload: { diffOptions: ['missing', 'type', 'response'] }, + // type: 'settings/setDiffOptionsAction', + // }) + // expect( + // screen.getByRole('option', { + // name: 'Missing', + // }) + // ).toBeInTheDocument() + // expect( + // screen.getByRole('option', { + // name: 'Type', + // }) + // ).toBeInTheDocument() + // expect( + // screen.getByRole('option', { + // name: 'Response', + // }) + // ).toBeInTheDocument() + // } + // ) + test.todo('toggling comparison option pushes value to url opts param') + test.todo('unselecting comparison option will remove it from url opts param') + test.todo('selecting clear option will remove all params from url opts param') +}) diff --git a/packages/api-explorer/src/state/settings/slice.ts b/packages/api-explorer/src/state/settings/slice.ts index a5799e4c0..c0f93b1cb 100644 --- a/packages/api-explorer/src/state/settings/slice.ts +++ b/packages/api-explorer/src/state/settings/slice.ts @@ -36,6 +36,7 @@ export interface UserDefinedSettings { } export interface SettingState extends UserDefinedSettings { + diffOptions: string[] searchPattern: string searchCriteria: SearchCriterionTerm[] tagFilter: string @@ -44,6 +45,7 @@ export interface SettingState extends UserDefinedSettings { } export const defaultSettings = { + diffOptions: ['missing', 'params', 'type', 'body', 'response'], sdkLanguage: 'Python', searchPattern: '', searchCriteria: setToCriteria(SearchAll) as SearchCriterionTerm[], @@ -55,6 +57,7 @@ export const defaultSettingsState: SettingState = { initialized: false, } +type SetDiffOptionsAction = Pick type SetSearchPatternAction = Pick type SetSdkLanguageAction = Pick type SetTagFilterAction = Pick @@ -78,6 +81,9 @@ export const settingsSlice = createSlice({ state.error = action.payload state.initialized = false }, + setDiffOptionsAction(state, action: PayloadAction) { + state.diffOptions = action.payload.diffOptions + }, setSdkLanguageAction(state, action: PayloadAction) { state.sdkLanguage = action.payload.sdkLanguage }, diff --git a/packages/api-explorer/src/test-utils/redux.tsx b/packages/api-explorer/src/test-utils/redux.tsx index f0f6023a8..f558681bd 100644 --- a/packages/api-explorer/src/test-utils/redux.tsx +++ b/packages/api-explorer/src/test-utils/redux.tsx @@ -78,8 +78,9 @@ type DeepPartial = { [P in keyof T]?: DeepPartial } -export const createTestStore = (overrides?: DeepPartial) => - createStore({ +export const createTestStore = (overrides?: DeepPartial) => { + // TODO: revert back to implicit return after fixing default initialization issue + return createStore({ preloadedState: { settings: { ...preloadedState.settings, @@ -100,3 +101,4 @@ export const createTestStore = (overrides?: DeepPartial) => specs: specsSlice.reducer, }, }) +} From 4c498d4c358dd5e23055f22b78eda71077576fba Mon Sep 17 00:00:00 2001 From: Niral Patel Date: Wed, 31 Aug 2022 01:16:05 +0000 Subject: [PATCH 2/5] Clean diffscene.spec to get investigated --- .../src/scenes/DiffScene/DiffScene.spec.tsx | 130 +----------------- 1 file changed, 2 insertions(+), 128 deletions(-) diff --git a/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx b/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx index d875d0363..3ca5b46e4 100644 --- a/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx +++ b/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx @@ -23,140 +23,14 @@ SOFTWARE. */ -import React from 'react' -import { screen, waitFor, render } from '@testing-library/react' -import userEvent from '@testing-library/user-event' -import type { SpecItem, SpecList } from '@looker/sdk-codegen' -import { useLocation } from 'react-router-dom' -import { useDispatch } from 'react-redux' -import { getLoadedSpecs } from '../../test-data' -import { - createTestStore, - renderWithRouter, - renderWithRouterAndReduxProvider, -} from '../../test-utils' -import { getApixAdaptor, MockedProvider, mockHistory } from '../../utils' -import { DiffScene } from './DiffScene' - -// jest.mock('react-redux', () => { -// const reactRedux = jest.requireActual('react-redux') -// return { -// __esModule: true, -// ...reactRedux, -// useDispatch: jest.fn(reactRedux.useDispatch), -// } -// }) - -// jest.mock('react-router-dom', () => { -// const ReactRouter = jest.requireActual('react-router-dom') -// -// return { -// __esModule: true, -// ...ReactRouter, -// useHistory: jest.fn().mockReturnValue({ -// push: jest.fn(), -// location, -// useLocation: jest -// .fn() -// .mockReturnValue({ pathname: '/4.0/diff/3.1', search: '' }), -// }), -// } -// }) - -const specs = getLoadedSpecs() as SpecList -// class MockApixAdaptor { -// async fetchSpec(spec: SpecItem) { -// return new Promise(() => specs[spec.key]) -// } -// } - -// const mockApixAdaptor = new MockApixAdaptor() -// jest.mock('../../utils/apixAdaptor', () => { -// const apixAdaptor = jest.requireActual('../../utils/apixAdaptor') -// return { -// __esModule: true, -// ...apixAdaptor, -// getApixAdaptor: jest.fn(), -// } -// }) +import { preloadedState } from '../../test-utils' describe('DiffScene', () => { - // const mockDispatch = jest.fn() - - // afterEach(() => { - // jest.clearAllMocks() - // }) - // ;(getApixAdaptor as jest.Mock).mockReturnValue(mockApixAdaptor) Element.prototype.scrollTo = jest.fn() Element.prototype.scrollIntoView = jest.fn() - const toggleNavigation = () => false test.only('updating url dispatches store action', () => { - // ;(useDispatch as jest.Mock).mockReturnValue(mockDispatch) - // TODO issue: executing createTestStore here doesn't init default settings - const store = createTestStore({ - specs: { specs, currentSpecKey: '4.0' }, - settings: { initialized: true }, - }) - const history = mockHistory({ - initialEntries: ['/4.0/diff/3.1'], - }) - const MockScene = ( - - - - ) - const { rerender } = render(MockScene) - // history.push('/4.0/diff/3.1?opts=missing') - // rerender(MockScene) - // expect(mockDispatch).toHaveBeenLastCalledWith({ - // payload: { diffOptions: ['missing'] }, - // type: 'settings/setDiffOptionsAction', - // }) - // TODO: test URL change leads to store dispatch? - change mock history push implementation to change our location - // TODO: test that toggling another will push both options to store/url + expect(preloadedState).toEqual({}) }) - - // test.todo( - // 'rendering scene with opts param in url sets selected options in selector', - // async () => { - // jest.spyOn(routerLocation, 'useLocation').mockReturnValue({ - // pathname: `/`, - // search: `opts=missing%2Ctype%2Cresponse`, - // } as unknown as Location) - // const store = createTestStore({ - // specs: { specs, currentSpecKey: '3.1' }, - // settings: { initialized: true, diffOptions: [] }, - // }) - // jest.spyOn(reactRedux, 'useDispatch').mockReturnValue(mockDispatch) - // renderWithRouterAndReduxProvider( - // , - // ['/diff/3.1'], - // store - // ) - // expect(mockDispatch).toHaveBeenLastCalledWith({ - // payload: { diffOptions: ['missing', 'type', 'response'] }, - // type: 'settings/setDiffOptionsAction', - // }) - // expect( - // screen.getByRole('option', { - // name: 'Missing', - // }) - // ).toBeInTheDocument() - // expect( - // screen.getByRole('option', { - // name: 'Type', - // }) - // ).toBeInTheDocument() - // expect( - // screen.getByRole('option', { - // name: 'Response', - // }) - // ).toBeInTheDocument() - // } - // ) - test.todo('toggling comparison option pushes value to url opts param') - test.todo('unselecting comparison option will remove it from url opts param') - test.todo('selecting clear option will remove all params from url opts param') }) From 61bc584840b91015a009034691607709f679b252 Mon Sep 17 00:00:00 2001 From: Niral Patel Date: Wed, 31 Aug 2022 02:08:05 +0000 Subject: [PATCH 3/5] Refactor --- .../api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx b/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx index 3ca5b46e4..0a753a580 100644 --- a/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx +++ b/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx @@ -25,12 +25,13 @@ */ import { preloadedState } from '../../test-utils' +import { getLoadedSpecs } from '../../test-data' describe('DiffScene', () => { Element.prototype.scrollTo = jest.fn() Element.prototype.scrollIntoView = jest.fn() - - test.only('updating url dispatches store action', () => { + const specs = getLoadedSpecs() + test('calling getLoadedSpecs should not affect preloadedState', () => { expect(preloadedState).toEqual({}) }) }) From 52e28074324a46dc0bdc1385b9b32a88584940fc Mon Sep 17 00:00:00 2001 From: Niral Patel Date: Wed, 31 Aug 2022 02:56:47 +0000 Subject: [PATCH 4/5] getLoadedSpecs affecting preloadedState --- packages/api-explorer/package.json | 2 + .../src/scenes/DiffScene/DiffScene.spec.tsx | 12 ++- packages/api-explorer/src/utils/index.ts | 1 + .../api-explorer/src/utils/testReduxUtils.tsx | 75 +++++++++++++++++++ 4 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 packages/api-explorer/src/utils/testReduxUtils.tsx diff --git a/packages/api-explorer/package.json b/packages/api-explorer/package.json index 5b91b8c1c..d0449f768 100644 --- a/packages/api-explorer/package.json +++ b/packages/api-explorer/package.json @@ -87,7 +87,9 @@ "react": "^16.13.1", "react-diff-viewer": "^3.1.1", "react-dom": "^16.13.1", + "react-helmet-async": "^1.3.0", "react-is": "^16.13.1", + "react-query": "^3.39.2", "react-redux": "^7.2.3", "react-router": "^5.1.2", "react-router-dom": "^5.1.2", diff --git a/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx b/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx index 0a753a580..90529e806 100644 --- a/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx +++ b/packages/api-explorer/src/scenes/DiffScene/DiffScene.spec.tsx @@ -24,14 +24,22 @@ */ -import { preloadedState } from '../../test-utils' +import type { SpecList } from '@looker/sdk-codegen' import { getLoadedSpecs } from '../../test-data' +import { createTestStore, preloadedState } from '../../test-utils' describe('DiffScene', () => { Element.prototype.scrollTo = jest.fn() Element.prototype.scrollIntoView = jest.fn() - const specs = getLoadedSpecs() + + // NOTE: if call getLoadedSpecs, preloadedState = undefined + // if comment out getLoadedSpecs, preloadedState = expected + const specs = getLoadedSpecs() as SpecList test('calling getLoadedSpecs should not affect preloadedState', () => { expect(preloadedState).toEqual({}) + const store = createTestStore({ + specs: { specs, currentSpecKey: '4.0' }, + settings: { initialized: true }, + }) }) }) diff --git a/packages/api-explorer/src/utils/index.ts b/packages/api-explorer/src/utils/index.ts index 1eaca523a..a0959f5ab 100644 --- a/packages/api-explorer/src/utils/index.ts +++ b/packages/api-explorer/src/utils/index.ts @@ -30,4 +30,5 @@ export { getLoded } from './lodeUtils' export { useWindowSize } from './useWindowSize' export * from './apixAdaptor' export * from './adaptorUtils' +export { MockedProvider, mockHistory } from './testReduxUtils' export { useNavigation, useGlobalStoreSync, useQuery } from './hooks' diff --git a/packages/api-explorer/src/utils/testReduxUtils.tsx b/packages/api-explorer/src/utils/testReduxUtils.tsx new file mode 100644 index 000000000..bc54819f0 --- /dev/null +++ b/packages/api-explorer/src/utils/testReduxUtils.tsx @@ -0,0 +1,75 @@ +/* + + MIT License + + Copyright (c) 2022 Looker Data Sciences, Inc. + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. + + */ +import { ComponentsProvider } from '@looker/components' +import type { Store } from '@looker/redux' +import React from 'react' +import { QueryClient, QueryClientProvider } from 'react-query' +import { Provider } from 'react-redux' +import { Router } from 'react-router' +import { HelmetProvider } from 'react-helmet-async' +import type { MemoryHistoryBuildOptions, History } from 'history' +import { createMemoryHistory } from 'history' +import type { RootState } from '../state' +import { createTestStore } from '../test-utils' + +export interface MockedProviderProps { + history?: History + store?: Store +} + +/** + * Mocks all providers needed to render any component or scene + */ +export const MockedProvider: React.FC = ({ + children, + history = mockHistory(), + store = createTestStore(), +}) => { + return ( + + + + + + {children} + + + + + + ) +} + +export const mockHistory = ( + /** + * Set the current route by passing in a string or to mock the entire + * history stack pass in MemoryHistoryBuildOptions + */ + route?: string | MemoryHistoryBuildOptions +) => + createMemoryHistory( + typeof route === 'string' ? { initialEntries: [route] } : route + ) From 213b9569f3fd277a1630ccb0937ee7728cfebbdb Mon Sep 17 00:00:00 2001 From: Niral Patel Date: Wed, 31 Aug 2022 03:01:05 +0000 Subject: [PATCH 5/5] slice test to verify preloadedState --- .../src/state/settings/slice.spec.ts | 46 +++++++++++++++++++ .../api-explorer/src/test-utils/redux.tsx | 9 ++-- 2 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 packages/api-explorer/src/state/settings/slice.spec.ts diff --git a/packages/api-explorer/src/state/settings/slice.spec.ts b/packages/api-explorer/src/state/settings/slice.spec.ts new file mode 100644 index 000000000..096d43553 --- /dev/null +++ b/packages/api-explorer/src/state/settings/slice.spec.ts @@ -0,0 +1,46 @@ +/* + + MIT License + + Copyright (c) 2021 Looker Data Sciences, Inc. + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. + + */ +import type { SearchCriterionTerm } from '@looker/sdk-codegen' +import { SearchAll, setToCriteria } from '@looker/sdk-codegen' +import { preloadedState } from '../../test-utils' +import { defaultSettingsState } from './slice' + +describe('Settings Slice', () => { + test('defaultSettingsState contains the default initial settings state', () => { + expect(defaultSettingsState).toEqual({ + diffOptions: ['missing', 'params', 'type', 'body', 'response'], + sdkLanguage: 'Python', + searchPattern: '', + searchCriteria: setToCriteria(SearchAll) as SearchCriterionTerm[], + tagFilter: 'ALL', + initialized: false, + }) + }) + + test('preloadedState settings initialized with default settings', () => { + expect(preloadedState.settings).toEqual(defaultSettingsState) + }) +}) diff --git a/packages/api-explorer/src/test-utils/redux.tsx b/packages/api-explorer/src/test-utils/redux.tsx index f558681bd..fbea3c8be 100644 --- a/packages/api-explorer/src/test-utils/redux.tsx +++ b/packages/api-explorer/src/test-utils/redux.tsx @@ -80,12 +80,13 @@ type DeepPartial = { export const createTestStore = (overrides?: DeepPartial) => { // TODO: revert back to implicit return after fixing default initialization issue + const settings = { + ...preloadedState.settings, + ...overrides?.settings, + } as SettingState return createStore({ preloadedState: { - settings: { - ...preloadedState.settings, - ...overrides?.settings, - } as SettingState, + settings, lodes: { ...defaultLodesState, ...overrides?.lodes,