Incremental UI tests #2 (#81)

* App tests

* Add scaffolding for useIsMounted

* Ignore .js, .jsx, and other CRA files

* Action creator tests

* renderCommaSepList tests

* removeMiddleAuthorsTest

* shortenAuthors test

* shortenTableString

* getTagColor test

* isDOI test

* markdown test

* shhhh is okay

* self review feedback

* CR feebdack

client/package.json

@@ -108,5 +108,12 @@
       "last 1 firefox version",
       "last 1 safari version"
     ]
+  },
+  "jest": {
+    "collectCoverageFrom": [
+      "src/**/*.{ts,tsx}",
+      "!src/index.tsx",
+      "!src/store.ts"
+    ]
   }
 }

client/src/App.test.tsx

@@ -0,0 +1,9 @@
+import React from 'react';
+import App from './App';
+import { renderWithRouterRedux } from './testUtils/reduxRender';
+
+describe('<App />', () => {
+  it('renders without crashing', () => {
+    renderWithRouterRedux(<App />);
+  });
+});

client/src/actions/actions.test.ts

@@ -1,9 +1,17 @@
+/* eslint-disable @typescript-eslint/no-explicit-any */
 import { rest } from 'msw';
-import { fetchUser } from '.';
+import { fetchUser, setReview, updateDraftId, updateDrafts, updateReadingList, updateReviews } from '.';
 
 import { server } from '../mocks/server';
-import { blankUser } from '../templates';
-import { FETCH_USER } from './actionTypes';
+import { blankPaper, blankReview, blankUser } from '../templates';
+import {
+  FETCH_USER,
+  SET_REVIEW,
+  UPDATE_DRAFTS,
+  UPDATE_DRAFT_ID,
+  UPDATE_READING_LIST,
+  UPDATE_REVIEWS,
+} from './actionTypes';
 
 describe('redux actions', () => {
   describe('fetchUser', () => {
@@ -25,4 +33,25 @@ describe('redux actions', () => {
       expect(dispatchMock).toHaveBeenCalledWith({ payload: blankUser, type: FETCH_USER });
     });
   });
+
+  // These are all really simple, but we're chasing that sweet sweet test coverage
+  describe('other action creators', () => {
+    const scenarios: {
+      creator: any;
+      arg: any;
+      expectedType: any;
+    }[] = [
+      { creator: setReview, arg: blankReview, expectedType: SET_REVIEW },
+      { creator: updateDraftId, arg: 'Mongo ID', expectedType: UPDATE_DRAFT_ID },
+      { creator: updateDrafts, arg: [blankReview], expectedType: UPDATE_DRAFTS },
+      { creator: updateReadingList, arg: [blankPaper], expectedType: UPDATE_READING_LIST },
+      { creator: updateReviews, arg: [blankReview], expectedType: UPDATE_REVIEWS },
+    ];
+
+    scenarios.forEach(({ creator, arg, expectedType }) => {
+      it(`returns the right type for ${creator.name}`, () => {
+        expect(creator(arg).type).toBe(expectedType);
+      });
+    });
+  });
 });

client/src/components/DraftPage/DraftPage-redux.tsx

@@ -1,10 +1,11 @@
 import React from 'react';
-import { useProtected } from '../../hooks/useProtected';
+import { useProtected } from '../../hooks';
 
 import Drafts from '../Drafts';
 
 function DraftPageRedux(): JSX.Element {
   useProtected();
+
   return (
     <div className="width80">
       <Drafts />

client/src/components/Home/Home-redux.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { useProtected } from '../../hooks/useProtected';
+import { useProtected } from '../../hooks';
 import HomeContainer from './Home-container';
 
 export default function HomeRedux(): JSX.Element {

client/src/components/Login/Login.tsx

@@ -6,7 +6,7 @@ import { CalendarOutlined, TeamOutlined } from '@ant-design/icons';
 import { Redirect } from 'react-router-dom';
 import LazyHero from 'react-lazy-hero';
 import { Location } from 'history';
-import GoogleButton from 'react-google-button';
+import GoogleButton from 'react-google-button/dist/react-google-button';
 import './Login.scss';
 import { RootState } from '../../reducers';
 import { User } from '../../types';

client/src/components/utils.test.tsx

@@ -0,0 +1,214 @@
+/* eslint-disable no-console */
+/* eslint-disable no-return-assign */
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+import {
+  getTagColor,
+  HSLString,
+  isDOI,
+  removeMiddleAuthors,
+  renderCommaSepList,
+  shortenAuthors,
+  shortenTableString,
+  wrapMarkdownWithMath,
+} from './utils';
+
+describe('utils', () => {
+  describe('renderCommaSepList', () => {
+    it('renders nothing when nothing is passed in', () => {
+      expect(renderCommaSepList([])).toEqual([]);
+    });
+
+    it('renders one item without a comma separator', () => {
+      const output = renderCommaSepList(['Shrek']);
+      render(<div>{output}</div>);
+      expect(screen.getByText(/Shrek/)).toBeDefined();
+    });
+
+    it('renders two items without a comma separator and an "and"', () => {
+      const output = renderCommaSepList(['Shrek', 'Donkey']);
+      render(<div>{output}</div>);
+      expect(screen.getByText('Shrek')).toBeDefined();
+      expect(screen.getByText('and Donkey')).toBeDefined();
+    });
+
+    it('renders 3 or more items with comma separation', () => {
+      const output = renderCommaSepList(['Shrek', 'Donkey', 'Fiona', 'Father Time']);
+      render(<div>{output}</div>);
+      expect(screen.getByText(/Shrek,/)).toBeDefined();
+      expect(screen.getByText(/Donkey,/)).toBeDefined();
+      expect(screen.getByText(/Fiona/)).toBeDefined();
+      expect(screen.getByText('and Father Time')).toBeDefined();
+    });
+  });
+
+  describe('removeMiddleAuthors', () => {
+    const scenarios: {
+      description: string;
+      authorList: string[];
+      numKeepEitherEnd: number;
+      expectedResult: string[];
+    }[] = [
+      {
+        description: 'with zero authors',
+        authorList: [],
+        numKeepEitherEnd: 0,
+        expectedResult: [],
+      },
+      {
+        description: 'with few authors, keeping many on each end',
+        authorList: ['Piranesi', 'The Other'],
+        numKeepEitherEnd: 100,
+        expectedResult: ['Piranesi', 'The Other'],
+      },
+      {
+        description: 'with many authors, keep 0 on either end',
+        authorList: ['Piranesi', 'The Other'],
+        numKeepEitherEnd: 0,
+        expectedResult: ['Piranesi', 'The Other'],
+      },
+      {
+        description: 'with many authors, keep 2 on either end',
+        authorList: ['Piranesi', 'The Other', 'The Biscuit Box Man', 'Sixteen', "Sylvia D'Agostino"],
+        numKeepEitherEnd: 2,
+        expectedResult: ['Piranesi', 'The Other', '...', 'Sixteen', "Sylvia D'Agostino"],
+      },
+    ];
+
+    scenarios.forEach(({ description, authorList, numKeepEitherEnd, expectedResult }) => {
+      it(`returns the correct result ${description}`, () => {
+        expect(removeMiddleAuthors(authorList, numKeepEitherEnd)).toEqual(expectedResult);
+      });
+    });
+  });
+
+  describe('shortenAuthors', () => {
+    it('returns NAText when a single blank author is provided', () => {
+      const output = shortenAuthors(['']);
+      render(output as JSX.Element);
+      expect(screen.getByText('N/A')).toBeDefined();
+    });
+
+    it('returns NAText when no authors are provided', () => {
+      const output = shortenAuthors([]);
+      render(output as JSX.Element);
+      expect(screen.getByText('N/A')).toBeDefined();
+    });
+
+    const scenarios: {
+      description: string;
+      authors: string[];
+      expectedOutput: string;
+    }[] = [
+      {
+        description: 'with 2 authors',
+        authors: ['Stephen Curry', 'Klay Thompson'],
+        expectedOutput: 'Curry and Thompson',
+      },
+      {
+        description: 'with 1 author',
+        authors: ['Stephen Curry'],
+        expectedOutput: 'Stephen Curry',
+      },
+      {
+        description: 'with 3 authors',
+        authors: ['Stephen Curry', 'Klay Thompson', 'Billy the Shooter'],
+        expectedOutput: 'Curry et al.',
+      },
+    ];
+
+    scenarios.forEach(({ description, authors, expectedOutput }) => {
+      it(`returns the right string ${description}`, () => {
+        expect(shortenAuthors(authors)).toEqual(expectedOutput);
+      });
+    });
+  });
+
+  describe('shortenTableString', () => {
+    const scenarios: {
+      description: string;
+      inString: string;
+      cutoff: number;
+      expectedText: string;
+    }[] = [
+      {
+        description: 'with no string',
+        inString: '',
+        cutoff: -5,
+        expectedText: 'N/A',
+      },
+      {
+        // TODO EM: is this desired?
+        description: 'with a string and a negative cutoff',
+        inString: 'testing one two',
+        cutoff: -5,
+        expectedText: 'testing one two',
+      },
+      {
+        description: 'with a string and a small cutoff',
+        inString: 'testing one two',
+        cutoff: 5,
+        expectedText: 'testi...',
+      },
+      {
+        description: 'with a string and a huge cutoff',
+        inString: 'testing one two',
+        cutoff: 500,
+        expectedText: 'testing one two',
+      },
+    ];
+
+    scenarios.forEach(({ description, inString, cutoff, expectedText }) => {
+      it(`renders the correct element ${description}`, () => {
+        render(shortenTableString(inString, cutoff));
+        expect(screen.getByText(expectedText)).toBeDefined();
+      });
+    });
+  });
+
+  describe('getTagColor', () => {
+    it('returns the same color for the same tag', () => {
+      const tag = 'tag';
+      expect(getTagColor(tag)).toEqual(getTagColor(tag));
+    });
+  });
+
+  describe.skip('getReviewStats', () => {
+    // TODO: once review data deserialization is figured out, write this test
+  });
+
+  describe('isDOI', () => {
+    it('recognizes DOIs starting with 10', () => {
+      expect(isDOI('10.1.1')).toBeTruthy();
+    });
+
+    it('recognizes DOIs with doi.org', () => {
+      expect(isDOI('   doi.org   ')).toBeTruthy();
+    });
+
+    it('cannot be fooled by mere mortals', () => {
+      expect(isDOI('   doI.org   ')).toBeFalsy();
+      expect(isDOI('10 .1.1')).toBeFalsy();
+    });
+  });
+
+  describe('wrapMarkdownWithMath', () => {
+    // This is silly, but the react-katex library throws an ugly warning: https://github.com/talyssonoc/react-katex/issues/59
+    // so we'll just suppress it :)
+    const originalConsoleWarn = console.warn;
+    beforeAll(() => (console.warn = jest.fn()));
+    afterAll(() => (console.warn = originalConsoleWarn));
+
+    it('does not alter non-math string', () => {
+      const testString = 'The Year I Named the Constellations';
+      render(wrapMarkdownWithMath(testString));
+      expect(screen.getByText(testString)).toBeDefined();
+    });
+
+    it('renders math strings with markdown', () => {
+      render(wrapMarkdownWithMath('$\\sum_0^\\infty$'));
+      expect(screen.getAllByText(/∑/)).toBeDefined();
+      expect(screen.getAllByText(/∞/)).toBeDefined();
+    });
+  });
+});

client/src/components/utils.tsx

@@ -46,7 +46,7 @@ export const renderCommaSepList = (items: string[]): JSX.Element[] =>
 export const removeMiddleAuthors = (authorList: string[], numKeepEitherEnd: number): string[] => {
   const numAuthors = authorList.length;
   const numKeepTotal = numKeepEitherEnd * 2;
-  if (numAuthors <= numKeepTotal) {
+  if (numAuthors <= numKeepTotal || numKeepTotal === 0) {
     return authorList;
   }
 
@@ -60,6 +60,8 @@ export const removeMiddleAuthors = (authorList: string[], numKeepEitherEnd: numb
 };
 
 export const shortenAuthors = (authors: string[]): JSX.Element | string => {
+  if (!authors.length) return <NAText />;
+
   let authorString = '';
 
   if (authors.length === 2) {
@@ -78,6 +80,10 @@ export const shortenTableString = (str: string, cutoff: number): JSX.Element =>
     return <NAText />;
   }
 
+  if (cutoff <= 0) {
+    return <span>{str}</span>;
+  }
+
   return <span>{str.length >= cutoff ? `${str.substring(0, cutoff)}...` : str}</span>;
 };
 

client/src/declarations.d.ts

@@ -1,2 +1,3 @@
 declare module 'react-lazy-hero';
 declare module 'react-katex';
+declare module 'react-google-button/dist/react-google-button';

client/src/hooks/index.ts

@@ -0,0 +1,2 @@
+export * from './useIsMounted';
+export * from './useProtected';

client/src/hooks/useIsMounted/index.ts

@@ -0,0 +1 @@
+export * from './useIsMounted';

client/src/hooks/useIsMounted/useIsMounted.test.tsx

@@ -0,0 +1,16 @@
+import { render, screen } from '@testing-library/react';
+import React from 'react';
+import { useIsMounted } from '.';
+
+// Because you can't run a hook outside of a react component, we'll make a small test case:
+function TestComponent(): JSX.Element {
+  const isMounted = useIsMounted();
+  return <h1>{isMounted().toString()}</h1>;
+}
+
+xdescribe('useIsMounted', () => {
+  it('returns false before the component is mounted', async () => {
+    render(<TestComponent />);
+    expect(screen.getByText(/true/)).toBeDefined();
+  });
+});

client/src/hooks.ts → client/src/hooks/useIsMounted/useIsMounted.ts

@@ -1,4 +1,4 @@
-import { useCallback, useRef, useEffect } from 'react';
+import { useRef, useEffect, useCallback } from 'react';
 
 export const useIsMounted = (): (() => boolean) => {
   const mountedRef = useRef(false);