From 01f23ab60651202825a330db6ae50d7b6d6836b0 Mon Sep 17 00:00:00 2001 From: Trevor Pierce <1Copenut@users.noreply.github.com> Date: Fri, 29 Sep 2023 11:26:21 -0500 Subject: [PATCH] [Tech debt] Fix Jest unit tests on broken on React 17 (#7227) Co-authored-by: Cee Chen --- .buildkite/scripts/pipeline_test.sh | 4 +- package.json | 8 +- scripts/{cypress.js => test-cypress.js} | 3 + scripts/test-unit.js | 58 ++ .../collapsible_nav_beta.test.tsx.snap | 99 ---- .../collapsible_nav_beta.test.tsx | 19 +- .../keyboard_shortcuts.test.tsx.snap | 508 +++++++++++++++++- .../controls/keyboard_shortcuts.test.tsx | 35 +- .../text_truncate/text_truncate.tsx | 3 +- .../functions/typography.test.tsx | 2 +- src/services/theme/hooks.test.tsx | 6 +- .../testing/unit-testing.md | 6 +- 12 files changed, 620 insertions(+), 131 deletions(-) rename scripts/{cypress.js => test-cypress.js} (95%) create mode 100644 scripts/test-unit.js diff --git a/.buildkite/scripts/pipeline_test.sh b/.buildkite/scripts/pipeline_test.sh index 7fef13e2c04..bad0e5ce24a 100644 --- a/.buildkite/scripts/pipeline_test.sh +++ b/.buildkite/scripts/pipeline_test.sh @@ -13,4 +13,6 @@ docker run \ docker.elastic.co/eui/ci:5.3 \ bash -c "/opt/yarn*/bin/yarn \ && yarn cypress install \ - && NODE_OPTIONS=\"--max-old-space-size=2048\" npm run test-ci" + && yarn lint \ + && yarn test-unit --node-options=--max_old_space_size=2048 \ + && yarn test-cypress --node-options=--max_old_space_size=2048" diff --git a/package.json b/package.json index c69898046a7..45f68696438 100644 --- a/package.json +++ b/package.json @@ -31,12 +31,12 @@ "lint-sass": "yarn stylelint \"**/*.scss\" --quiet-deprecation-warnings", "test": "yarn lint && yarn test-unit", "test-ci": "yarn test && yarn test-cypress", - "test-unit": "cross-env NODE_ENV=test jest --config ./scripts/jest/config.js", + "test-unit": "node ./scripts/test-unit", "test-a11y": "node ./scripts/a11y-testing", "test-staged": "yarn lint && node scripts/test-staged.js", - "test-cypress": "node ./scripts/cypress", - "test-cypress-dev": "node ./scripts/cypress --dev", - "test-cypress-a11y": "node ./scripts/cypress --a11y", + "test-cypress": "node ./scripts/test-cypress", + "test-cypress-dev": "yarn test-cypress --dev", + "test-cypress-a11y": "yarn test-cypress --a11y", "combine-test-coverage": "sh ./scripts/combine-coverage.sh", "start-test-server": "BABEL_MODULES=false NODE_ENV=puppeteer NODE_OPTIONS=--max-old-space-size=4096 webpack-dev-server --config src-docs/webpack.config.js --port 9999", "yo-component": "yo ./generator-eui/app/component.js", diff --git a/scripts/cypress.js b/scripts/test-cypress.js similarity index 95% rename from scripts/cypress.js rename to scripts/test-cypress.js index 04ba81e7673..e0cbde1c7f0 100644 --- a/scripts/cypress.js +++ b/scripts/test-cypress.js @@ -17,6 +17,7 @@ const argv = yargs(hideBin(process.argv)) 'unknown-options-as-args': true, // collect any extra options to pass on to cypress }) .options({ + 'node-options': { type: 'string', default: '' }, 'skip-css': { type: 'boolean' }, dev: { type: 'boolean' }, theme: { type: 'string', default: 'light', choices: ['light', 'dark'] }, @@ -28,6 +29,7 @@ const argv = yargs(hideBin(process.argv)) }, }).argv; +const nodeOptions = argv['node-options']; const isDev = argv.hasOwnProperty('dev'); const isA11y = argv.hasOwnProperty('a11y'); const skipScss = argv.hasOwnProperty('skip-css'); @@ -59,6 +61,7 @@ const cypressCommandParts = [ 'cross-env', // windows support `THEME=${theme}`, // pass the theme 'BABEL_MODULES=false', // let webpack receive ES Module code + `NODE_OPTIONS="${nodeOptions}"`, 'NODE_ENV=cypress_test', // enable code coverage checks `REACT_VERSION=${reactVersion}`, // set react version to test `cypress ${testParams}`, diff --git a/scripts/test-unit.js b/scripts/test-unit.js new file mode 100644 index 00000000000..dd0f63dd979 --- /dev/null +++ b/scripts/test-unit.js @@ -0,0 +1,58 @@ +const { execSync } = require('child_process'); +const yargs = require('yargs/yargs'); +const { hideBin } = require('yargs/helpers'); +const chalk = require('chalk'); + +const { argv } = yargs(hideBin(process.argv)) + .parserConfiguration({ + // @see https://github.com/yargs/yargs-parser#configuration + 'camel-case-expansion': false, + 'unknown-options-as-args': true, + 'halt-at-non-option': true, + }) + .options({ + 'node-options': { type: 'string', default: '' }, + 'react-version': { + type: 'number', + default: 18, + choices: [16, 17, 18], + }, + testMatch: { + type: 'string', + description: + 'Pass in `react` to only test `.tsx` files, or `non-react` for the opposite. Also accepts standard Jest `--testMatch` globs', + coerce: (value) => { + if (value === 'react') { + value = '**/*.test.tsx'; + } else if (value === 'non-react') { + value = '**/*.test.{js,ts}'; + } + return value; + }, + }, + }); +const nodeOptions = argv['node-options']; +const reactVersion = argv['react-version']; +const testMatch = argv['testMatch']; + +const commandParts = [ + 'cross-env', // windows support + `NODE_OPTIONS="${nodeOptions}"`, + 'NODE_ENV=test', + `REACT_VERSION=${reactVersion}`, + `jest --config ./scripts/jest/config.js`, + ...argv._, // pass any extra options given to this script +]; +if (testMatch) { + commandParts.push(`--testMatch '${testMatch}'`); // has to come after any filename patterns +} + +const command = commandParts.join(' '); +console.log(chalk.white(command)); +try { + execSync(command, { stdio: 'inherit' }); +} catch { + // Jest already outputs sufficient error messaging, no need to emit a + // node child process stack trace that just unhelpfully points to this file + process.exit(1); +} diff --git a/src/components/collapsible_nav_beta/__snapshots__/collapsible_nav_beta.test.tsx.snap b/src/components/collapsible_nav_beta/__snapshots__/collapsible_nav_beta.test.tsx.snap index ca97480a43d..e5a804d20f6 100644 --- a/src/components/collapsible_nav_beta/__snapshots__/collapsible_nav_beta.test.tsx.snap +++ b/src/components/collapsible_nav_beta/__snapshots__/collapsible_nav_beta.test.tsx.snap @@ -105,102 +105,3 @@ exports[`EuiCollapsibleNavBeta renders initialIsCollapsed 1`] = ` `; - -exports[`EuiCollapsibleNavBeta responsive behavior collapses from a push flyout to an overlay flyout once the screen is smaller than 3x the flyout width 1`] = ` - -
-
- -
-
- -`; - -exports[`EuiCollapsibleNavBeta responsive behavior makes the overlay flyout full width once the screen is smaller than 1.5x the flyout width 1`] = ` - -
-
- -
-
-
-
-
- -
-
-
- -`; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_beta.test.tsx b/src/components/collapsible_nav_beta/collapsible_nav_beta.test.tsx index 4d594b35871..1f2c60f4cf6 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_beta.test.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_beta.test.tsx @@ -72,19 +72,28 @@ describe('EuiCollapsibleNavBeta', () => { it('collapses from a push flyout to an overlay flyout once the screen is smaller than 3x the flyout width', () => { mockWindowResize(600); - const { baseElement } = render( - Nav content + const { queryByTestSubject, getByTestSubject } = render( + + Nav content + ); - expect(baseElement).toMatchSnapshot(); + expect(queryByTestSubject('nav')).not.toBeInTheDocument(); + fireEvent.click(getByTestSubject('euiCollapsibleNavButton')); + + expect(getByTestSubject('nav').className).toContain('overlay'); + expect(getByTestSubject('nav').className).not.toContain('push'); }); it('makes the overlay flyout full width once the screen is smaller than 1.5x the flyout width', () => { mockWindowResize(320); const { baseElement, getByTestSubject } = render( - Nav content + + Nav content + ); fireEvent.click(getByTestSubject('euiCollapsibleNavButton')); - expect(baseElement).toMatchSnapshot(); + + expect(getByTestSubject('nav').className).toContain('isOverlayFullWidth'); // onClose testing expect( diff --git a/src/components/datagrid/controls/__snapshots__/keyboard_shortcuts.test.tsx.snap b/src/components/datagrid/controls/__snapshots__/keyboard_shortcuts.test.tsx.snap index 16d3188b9ab..31bd284158a 100644 --- a/src/components/datagrid/controls/__snapshots__/keyboard_shortcuts.test.tsx.snap +++ b/src/components/datagrid/controls/__snapshots__/keyboard_shortcuts.test.tsx.snap @@ -1,6 +1,512 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`useDataGridKeyboardShortcuts returns a popover containing a list of keyboard shortcuts 1`] = ` +exports[`useDataGridKeyboardShortcuts [React 16] returns a popover containing a list of keyboard shortcuts 1`] = ` + +
+
+
+
+ + + +
+
+
+
+
+
+
+ +
+
+ +`; + +exports[`useDataGridKeyboardShortcuts [React 17] returns a popover containing a list of keyboard shortcuts 1`] = ` + +
+
+
+
+ + + +
+
+
+
+
+
+
+ +
+
+ +`; + +exports[`useDataGridKeyboardShortcuts [React 18] returns a popover containing a list of keyboard shortcuts 1`] = `
diff --git a/src/components/datagrid/controls/keyboard_shortcuts.test.tsx b/src/components/datagrid/controls/keyboard_shortcuts.test.tsx index d707c5a4896..a6a4ad8b8ab 100644 --- a/src/components/datagrid/controls/keyboard_shortcuts.test.tsx +++ b/src/components/datagrid/controls/keyboard_shortcuts.test.tsx @@ -7,30 +7,35 @@ */ import React from 'react'; +import { fireEvent } from '@testing-library/react'; import { render, renderHook, renderHookAct, waitForEuiPopoverOpen, } from '../../../test/rtl'; +import { testByReactVersion } from '../../../test/internal'; + import { useDataGridKeyboardShortcuts } from './keyboard_shortcuts'; -import { fireEvent } from '@testing-library/react'; describe('useDataGridKeyboardShortcuts', () => { - it('returns a popover containing a list of keyboard shortcuts', async () => { - const { result } = renderHook(() => useDataGridKeyboardShortcuts()); - const { baseElement, getByTestSubject, rerender } = render( -
{result.current.keyboardShortcuts}
- ); + testByReactVersion( + 'returns a popover containing a list of keyboard shortcuts', + async () => { + const { result } = renderHook(() => useDataGridKeyboardShortcuts()); + const { baseElement, getByTestSubject, rerender } = render( +
{result.current.keyboardShortcuts}
+ ); - renderHookAct(() => { - fireEvent.click(getByTestSubject('dataGridKeyboardShortcutsButton')); - }); - rerender( -
{result.current.keyboardShortcuts}
- ); - await waitForEuiPopoverOpen(); + renderHookAct(() => { + fireEvent.click(getByTestSubject('dataGridKeyboardShortcutsButton')); + }); + rerender( +
{result.current.keyboardShortcuts}
+ ); + await waitForEuiPopoverOpen(); - expect(baseElement).toMatchSnapshot(); - }); + expect(baseElement).toMatchSnapshot(); + } + ); }); diff --git a/src/components/text_truncate/text_truncate.tsx b/src/components/text_truncate/text_truncate.tsx index d616bcb3660..4054b1dac1b 100644 --- a/src/components/text_truncate/text_truncate.tsx +++ b/src/components/text_truncate/text_truncate.tsx @@ -96,12 +96,13 @@ export type EuiTextTruncateProps = Omit< export const EuiTextTruncate: FunctionComponent = ({ width, + onResize, ...props }) => { return width != null ? ( ) : ( - + ); }; diff --git a/src/global_styling/functions/typography.test.tsx b/src/global_styling/functions/typography.test.tsx index b9fda8d6fa4..1b86b3bd811 100644 --- a/src/global_styling/functions/typography.test.tsx +++ b/src/global_styling/functions/typography.test.tsx @@ -7,7 +7,7 @@ */ import React, { FunctionComponent, PropsWithChildren } from 'react'; -import { renderHook } from '@testing-library/react'; +import { renderHook } from '../../test/rtl/render_hook'; import { EuiProvider } from '../../components/provider'; import { useEuiTheme } from '../../services'; diff --git a/src/services/theme/hooks.test.tsx b/src/services/theme/hooks.test.tsx index 4e9014a290b..d2cb442fe06 100644 --- a/src/services/theme/hooks.test.tsx +++ b/src/services/theme/hooks.test.tsx @@ -7,8 +7,8 @@ */ import React from 'react'; -import { render, act } from '@testing-library/react'; -import { renderHook } from '../../test/rtl'; +import { render } from '@testing-library/react'; +import { renderHook, renderHookAct } from '../../test/rtl'; import { EuiProvider } from '../../components/provider'; @@ -94,7 +94,7 @@ describe('useEuiThemeCSSVariables', () => { expect(result.current.globalCSSVariables).toBeUndefined(); expect(result.current.themeCSSVariables).toBeUndefined(); - act(() => { + renderHookAct(() => { result.current.setNearestThemeCSSVariables({ '--hello': 'world' }); }); diff --git a/wiki/contributing-to-eui/testing/unit-testing.md b/wiki/contributing-to-eui/testing/unit-testing.md index edd89ba3110..24d03349edf 100644 --- a/wiki/contributing-to-eui/testing/unit-testing.md +++ b/wiki/contributing-to-eui/testing/unit-testing.md @@ -25,7 +25,11 @@ contains `{component name}.tsx`. ## Targeting files to test -You can also add any string to the end of the command to run the tests only on directories that contain that string. For example, `yarn test-unit button` will only update the tests for directories that **contain** `button`. +You can also add any string to the end of the command to run the tests only on files or directories that contain that string. For example, `yarn test-unit button` will test `accordion/button.test.tsx` and `button/icon.test.tsx`, but not `card.test.tsx`. + +`yarn test-unit --testMatch=react / --testMatch=non-react` will test specifically only `.tsx` files vs. non-`.tsx` files. If not specified, both types of tests will run automatically. + +`yarn test-unit --react-version=18` will run tests against a specific React version that EUI supports (currently 16-18). ## Test helpers