From a587174e0a6f91528aa10c379cea3a8d953e2b83 Mon Sep 17 00:00:00 2001 From: Julien Wajsberg Date: Thu, 16 Nov 2023 14:04:07 +0100 Subject: [PATCH 1/2] Simplify how we pass the props to CompareWithBase --- src/__tests__/Search/CompareWithBase.test.tsx | 35 ++++------ src/components/CompareResults/ResultsView.tsx | 33 ++++------ src/components/Search/CompareWithBase.tsx | 64 ++++++------------- src/components/Search/SearchContainer.tsx | 34 ++++------ 4 files changed, 56 insertions(+), 110 deletions(-) diff --git a/src/__tests__/Search/CompareWithBase.test.tsx b/src/__tests__/Search/CompareWithBase.test.tsx index dc1efff2b..727c92cd8 100644 --- a/src/__tests__/Search/CompareWithBase.test.tsx +++ b/src/__tests__/Search/CompareWithBase.test.tsx @@ -7,7 +7,6 @@ import CompareWithBase from '../../components/Search/CompareWithBase'; import SearchView from '../../components/Search/SearchView'; import { Strings } from '../../resources/Strings'; import useProtocolTheme from '../../theme/protocolTheme'; -import type { Repository } from '../../types/state'; import getTestData from '../utils/fixtures'; import { renderWithRouter, store } from '../utils/setupTests'; import { screen } from '../utils/test-utils'; @@ -18,34 +17,24 @@ const protocolTheme = renderHook(() => useProtocolTheme()).result.current const themeMode = protocolTheme.palette.mode; function renderComponent(isEditable: boolean) { const { testData } = getTestData(); - const selectedRevsBase = testData.slice(0, 1); - const selectedRevsNew = testData.slice(1, 3); + const baseRevs = testData.slice(0, 1); + const newRevs = testData.slice(1, 3); - const displayedCheckedRevisions = { - baseRevs: selectedRevsBase, - newRevs: selectedRevsNew, - }; - - const baseRepos = selectedRevsBase.map((item) => { - const selectedRep = repoMap[item.repository_id]; - return selectedRep; - }); - - const newRepos = selectedRevsNew.map((item) => { - const selectedRep = repoMap[item.repository_id]; - return selectedRep; - }); + // The "??" operations below are so that Typescript doesn't wonder about the + // undefined value later. + const baseRepos = baseRevs.map( + (item) => repoMap[item.repository_id] ?? 'try', + ); + const newRepos = newRevs.map((item) => repoMap[item.repository_id] ?? 'try'); - const displayedRepositories = { - baseRepos: baseRepos as Repository['name'][], - newRepos: newRepos as Repository['name'][], - }; renderWithRouter( , ); } diff --git a/src/components/CompareResults/ResultsView.tsx b/src/components/CompareResults/ResultsView.tsx index f346eced2..3da03179f 100644 --- a/src/components/CompareResults/ResultsView.tsx +++ b/src/components/CompareResults/ResultsView.tsx @@ -35,25 +35,14 @@ function ResultsView(props: ResultsViewProps) { (state) => state.selectedRevisions.new, ); - const displayedSelectedRevisions = { - baseRevs: selectedRevisionsListBase, - newRevs: selectedRevisionsListNew, - }; - - const selectedBaseRepositories = selectedRevisionsListBase.map((item) => { - const selectedRep = repoMap[item.repository_id]; - return selectedRep; - }); - - const selectedNewRepositories = selectedRevisionsListNew.map((item) => { - const selectedRep = repoMap[item.repository_id]; - return selectedRep; - }); - - const displayedRepositories = { - baseRepos: selectedBaseRepositories as Repository['name'][], - newRepos: selectedNewRepositories as Repository['name'][], - }; + // The "??" operations below are so that Typescript doesn't wonder about the + // undefined value later. + const selectedBaseRepositories = selectedRevisionsListBase.map( + (item) => repoMap[item.repository_id] ?? 'try', + ); + const selectedNewRepositories = selectedRevisionsListNew.map( + (item) => repoMap[item.repository_id] ?? 'try', + ); const { dispatchFetchCompareResults, dispatchFakeCompareResults } = useFetchCompareResults(); @@ -150,8 +139,10 @@ function ResultsView(props: ResultsViewProps) { diff --git a/src/components/Search/CompareWithBase.tsx b/src/components/Search/CompareWithBase.tsx index 6ce11f6b1..0eb90dedd 100644 --- a/src/components/Search/CompareWithBase.tsx +++ b/src/components/Search/CompareWithBase.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo } from 'react'; +import { useState } from 'react'; import Divider from '@mui/material/Divider'; import Grid from '@mui/material/Grid'; @@ -8,12 +8,7 @@ import { useAppSelector } from '../../hooks/app'; import { Strings } from '../../resources/Strings'; import { CompareCardsStyles } from '../../styles'; import { SearchStyles } from '../../styles'; -import type { - ThemeMode, - RevisionsList, - Repository, - InputType, -} from '../../types/state'; +import type { ThemeMode, RevisionsList, Repository } from '../../types/state'; import CompareButton from './CompareButton'; import FrameworkDropdown from './FrameworkDropdown'; import SearchComponent from './SearchComponent'; @@ -25,21 +20,19 @@ const stringsNew = Strings.components.searchDefault.base.collapsed.revision; interface CompareWithBaseProps { mode: ThemeMode; isEditable: boolean; - displayedRevisions: { - baseRevs: RevisionsList[]; - newRevs: RevisionsList[]; - }; - displayedRepositories: { - baseRepos: Repository['name'][]; - newRepos: Repository['name'][]; - }; + baseRevs: RevisionsList[]; + newRevs: RevisionsList[]; + baseRepos: Repository['name'][]; + newRepos: Repository['name'][]; } function CompareWithBase({ mode, isEditable, - displayedRevisions, - displayedRepositories, + baseRevs, + newRevs, + baseRepos, + newRepos, }: CompareWithBaseProps) { const [expanded, setExpanded] = useState(true); @@ -48,31 +41,6 @@ function CompareWithBase({ const search = useAppSelector((state) => state.search); const baseRepository = search.base.repository; const newRepository = search.new.repository; - const revisionsAndReposBase = useMemo(() => { - const revsAndRepos = { - revisions: displayedRevisions.baseRevs, - repositories: displayedRepositories.baseRepos, - }; - return revsAndRepos; - }, [displayedRevisions.baseRevs, displayedRepositories.baseRepos]); - - const revisionsAndReposNew = useMemo(() => { - const revsAndRepos = { - revisions: displayedRevisions.newRevs, - repositories: displayedRepositories.newRepos, - }; - return revsAndRepos; - }, [displayedRevisions.baseRevs, displayedRepositories.baseRepos]); - const baseSearchProps = { - ...stringsBase, - ...revisionsAndReposBase, - searchType: 'base' as InputType, - }; - const newSearchProps = { - ...stringsNew, - ...revisionsAndReposNew, - searchType: 'new' as InputType, - }; const isWarning = (baseRepository === 'try' && newRepository !== 'try') || (baseRepository !== 'try' && newRepository === 'try'); @@ -116,16 +84,22 @@ function CompareWithBase({
state.search.base.checkedRevisions, ); - const displayedCheckedRevisions = { - baseRevs: checkedRevisionsListBase, - newRevs: checkedRevisionsListNew, - }; - const checkedNewRepos = checkedRevisionsListNew.map((item) => { - const selectedRep = repoMap[item.repository_id]; - return selectedRep; - }); - - const checkedBaseRepos = checkedRevisionsListBase.map((item) => { - const selectedRep = repoMap[item.repository_id]; - return selectedRep; - }); - - const displayedRepositories = { - baseRepos: checkedBaseRepos as Repository['name'][], - newRepos: checkedNewRepos as Repository['name'][], - }; + // The "??" operations below are so that Typescript doesn't wonder about the + // undefined value later. + const checkedNewRepos = checkedRevisionsListNew.map( + (item) => repoMap[item.repository_id] ?? 'try', + ); + const checkedBaseRepos = checkedRevisionsListBase.map( + (item) => repoMap[item.repository_id] ?? 'try', + ); return (
{/* hidden until post-mvp release */} From 11c192d3d0f005ddfd3a69b95534b3ac4a3a4199 Mon Sep 17 00:00:00 2001 From: Julien Wajsberg Date: Wed, 22 Nov 2023 16:32:34 +0100 Subject: [PATCH 2/2] Increase test coverage for the Search view --- src/__tests__/Search/SearchView.test.tsx | 55 +++++++++++++++--------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/src/__tests__/Search/SearchView.test.tsx b/src/__tests__/Search/SearchView.test.tsx index d6d8f29e9..9dd21b5c4 100644 --- a/src/__tests__/Search/SearchView.test.tsx +++ b/src/__tests__/Search/SearchView.test.tsx @@ -3,7 +3,6 @@ import userEvent from '@testing-library/user-event'; import { act } from 'react-dom/test-utils'; import SearchView from '../../components/Search/SearchView'; -import { setSelectedRevisions } from '../../reducers/SelectedRevisionsSlice'; import { Strings } from '../../resources/Strings'; import useProtocolTheme from '../../theme/protocolTheme'; import { RevisionsList, InputType } from '../../types/state'; @@ -17,7 +16,7 @@ const protocolTheme = renderHook(() => useProtocolTheme()).result.current const toggleColorMode = renderHook(() => useProtocolTheme()).result.current .toggleColorMode; function renderComponent() { - renderWithRouter( + return renderWithRouter( { const searchInput = screen.getAllByRole('textbox')[0]; await user.click(searchInput); - await act(async () => { - expect(store.getState().search[searchType].searchResults).toStrictEqual( - testData, - ); - }); - const comment = await screen.findAllByText("you've got no arms left!"); expect(comment[0]).toBeInTheDocument(); + expect(store.getState().search[searchType].searchResults).toStrictEqual( + testData, + ); + // Press Escape key to hide search results. await user.keyboard('{Escape}'); @@ -279,23 +276,41 @@ describe('Base Search', () => { it('should have compare button and once clicked should redirect to results page with the right query params', async () => { const { testData } = getTestData(); + fetchTestData(testData); - const { history } = renderWithRouter( - , - ); + const { history } = renderComponent(); expect(history.location.pathname).toEqual('/'); const user = userEvent.setup({ delay: null }); - act(() => { - store.dispatch( - setSelectedRevisions({ selectedRevisions: testData.slice(0, 2) }), - ); - }); + // focus first input to show results + const inputs = screen.getAllByRole('textbox'); + await user.click(inputs[0]); + + // Select a base rev + let items = await screen.findAllByText("you've got no arms left!"); + await user.click(items[0]); + + // Press Escape key to hide search results. + await user.keyboard('{Escape}'); + expect(items[0]).not.toBeInTheDocument(); + + // Check that the item has been added + await screen.findByText(/no arms left/); + + // Now focus the second input + await user.click(inputs[1]); + // Select a new rev + items = await screen.findAllByText("it's just a flesh wound"); + await user.click(items[0]); + + // Press Escape key to hide search results. + await user.keyboard('{Escape}'); + + // Check that the item has been added + await screen.findByText(/flesh wound/); + + // Press the compare button const compareButton = document.querySelector('.compare-button'); await user.click(compareButton as HTMLElement);