From d90a70b4d3a3fa8f1ac6847196bfac86c323cb2b Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 6 Sep 2023 14:30:10 -0700 Subject: [PATCH 1/9] Switch pre-commit hook to pre-push --- package.json | 4 ++-- yarn.lock | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index f2fee1a4734..1f6277adb91 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ "**/@types/react": "^18", "react-focus-lock": "^2.9.5" }, - "pre-commit": [ + "pre-push": [ "test-staged" ], "dependencies": { @@ -220,7 +220,7 @@ "postcss-inline-svg": "^4.1.0", "postcss-loader": "^7.0.1", "postcss-styled-syntax": "^0.4.0", - "pre-commit": "^1.2.2", + "pre-push": "^0.1.4", "prettier": "^2.8.8", "process": "^0.11.10", "prop-types": "^15.6.0", diff --git a/yarn.lock b/yarn.lock index 61387fcacf0..6d603d1194b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17532,10 +17532,10 @@ postcss@^8.4.24: picocolors "^1.0.0" source-map-js "^1.0.2" -pre-commit@^1.2.2: - version "1.2.2" - resolved "https://registry.yarnpkg.com/pre-commit/-/pre-commit-1.2.2.tgz#dbcee0ee9de7235e57f79c56d7ce94641a69eec6" - integrity sha1-287g7p3nI15X95xW186UZBpp7sY= +pre-push@^0.1.4: + version "0.1.4" + resolved "https://registry.yarnpkg.com/pre-push/-/pre-push-0.1.4.tgz#48e7a29e15d3d7a83b62d5a02e10ba5dc40ab3aa" + integrity sha512-bIdVuDQR3r5AWV7bM6OMHD3mCXA53Ql0LXmW5UfcSmJZq+J+TytqZ5YJcTmMLcojJysN65vcFIeCqRn6YidA+Q== dependencies: cross-spawn "^5.0.1" spawn-sync "^1.0.15" From c2785442cd98bb0932b93964d2d8233b5df7b544 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 6 Sep 2023 14:33:33 -0700 Subject: [PATCH 2/9] Update `test-staged` to look at all commits from main, not just local staged files --- scripts/test-staged.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/test-staged.js b/scripts/test-staged.js index 944d41a68f4..6d438062586 100644 --- a/scripts/test-staged.js +++ b/scripts/test-staged.js @@ -1,7 +1,9 @@ const { execSync } = require('child_process'); // find names of staged files -const stagedFiles = execSync('git diff --cached --name-only --diff-filter=ACMR') +const stagedFiles = execSync( + 'git diff main...HEAD --name-only --diff-filter=ACMR' +) .toString() .split(/[\r\n]+/g); From 4fb17b82daa1baa2f0f33af46bb7233c9483d89b Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 11 Sep 2023 14:59:10 -0700 Subject: [PATCH 3/9] Use simpler `--changedSince` jest flag --- scripts/test-staged.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/scripts/test-staged.js b/scripts/test-staged.js index 6d438062586..3c31d516c2f 100644 --- a/scripts/test-staged.js +++ b/scripts/test-staged.js @@ -1,13 +1,6 @@ const { execSync } = require('child_process'); -// find names of staged files -const stagedFiles = execSync( - 'git diff main...HEAD --name-only --diff-filter=ACMR' -) - .toString() - .split(/[\r\n]+/g); - -// execute tests related to the staged files -execSync(`yarn test-unit --findRelatedTests ${stagedFiles.join(' ')}`, { +// https://jestjs.io/docs/cli#--changedsince +execSync(`yarn test-unit --changedSince main`, { stdio: 'inherit', }); From a18b6e3577c43840f1cd937c25eded85025b84c9 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 11 Sep 2023 15:00:17 -0700 Subject: [PATCH 4/9] [EuiCard] Fix imports pointed directly at `src/components/` which messes up Jest's logic for finding related tests --- src/components/card/card.styles.ts | 3 ++- src/components/card/card.test.tsx | 8 +++++--- src/components/card/card.tsx | 10 ++++++---- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/components/card/card.styles.ts b/src/components/card/card.styles.ts index 841705d3885..681bb528fd7 100644 --- a/src/components/card/card.styles.ts +++ b/src/components/card/card.styles.ts @@ -7,7 +7,7 @@ */ import { css } from '@emotion/react'; -import { EuiCardProps } from '..'; + import { euiPaddingSize, euiSupportsHas, @@ -18,6 +18,7 @@ import { import { UseEuiTheme } from '../../services'; import { euiButtonColor } from '../../themes/amsterdam/global_styling/mixins'; +import { EuiCardProps } from './card'; const paddingKey = 'm'; const halfPaddingKey = 's'; diff --git a/src/components/card/card.test.tsx b/src/components/card/card.test.tsx index 3470f54e4f1..fe78a65de72 100644 --- a/src/components/card/card.test.tsx +++ b/src/components/card/card.test.tsx @@ -12,11 +12,13 @@ import { requiredProps } from '../../test'; import { shouldRenderCustomStyles } from '../../test/internal'; import { render } from '../../test/rtl'; -import { EuiCard, ALIGNMENTS } from './card'; - -import { EuiIcon, EuiAvatar, EuiI18n } from '../../components'; +import { EuiIcon } from '../icon'; +import { EuiAvatar } from '../avatar'; +import { EuiI18n } from '../i18n'; import { COLORS, SIZES } from '../panel/panel'; +import { EuiCard, ALIGNMENTS } from './card'; + describe('EuiCard', () => { test('is rendered', () => { const { container } = render( diff --git a/src/components/card/card.tsx b/src/components/card/card.tsx index f28d1245dcb..fd5c38071a6 100644 --- a/src/components/card/card.tsx +++ b/src/components/card/card.tsx @@ -15,21 +15,23 @@ import React, { } from 'react'; import classNames from 'classnames'; -import { CommonProps, ExclusiveUnion } from '../common'; import { getSecureRelForTarget, useEuiTheme, cloneElementWithCss, } from '../../services'; +import { useGeneratedHtmlId } from '../../services/accessibility'; +import { validateHref } from '../../services/security/href_validator'; + +import { CommonProps, ExclusiveUnion } from '../common'; import { EuiText } from '../text'; import { EuiTitle } from '../title'; import { EuiBetaBadge, EuiBetaBadgeProps } from '../badge/beta_badge'; import { EuiIconProps } from '../icon'; -import { EuiCardSelect, EuiCardSelectProps } from './card_select'; -import { useGeneratedHtmlId } from '../../services/accessibility'; -import { validateHref } from '../../services/security/href_validator'; import { EuiPanel, EuiPanelProps } from '../panel'; import { EuiSpacer } from '../spacer'; + +import { EuiCardSelect, EuiCardSelectProps } from './card_select'; import { euiCardBetaBadgeStyles, euiCardStyles, From 5d7a24d4b97ab16114f672bf1fe26ffd7f57b203 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 11 Sep 2023 15:00:59 -0700 Subject: [PATCH 5/9] [EuiPopover] Fix test import pointed at `src/components` which messes up Jest's logic for finding related tests --- src/components/popover/popover.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/popover/popover.test.tsx b/src/components/popover/popover.test.tsx index 1babda9f1bf..e0fd8854adc 100644 --- a/src/components/popover/popover.test.tsx +++ b/src/components/popover/popover.test.tsx @@ -12,7 +12,9 @@ import { act } from '@testing-library/react'; import { shouldRenderCustomStyles } from '../../test/internal'; import { requiredProps } from '../../test/required_props'; import { render } from '../../test/rtl'; -import { EuiFocusTrap } from '../'; + +import { keys } from '../../services'; +import { EuiFocusTrap } from '../focus_trap'; import { EuiPopover, @@ -21,8 +23,6 @@ import { PopoverAnchorPosition, } from './popover'; -import { keys } from '../../services'; - const actAdvanceTimersByTime = (time: number) => act(() => jest.advanceTimersByTime(time)); From 1063a245c4bafa09e1a0ca698176bc2e0659cafc Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 11 Sep 2023 15:01:14 -0700 Subject: [PATCH 6/9] [EuiProvider] Fix imports pointed at `src/components` --- src/global_styling/functions/typography.test.tsx | 2 +- src/services/theme/hooks.test.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/global_styling/functions/typography.test.tsx b/src/global_styling/functions/typography.test.tsx index 19aa28d553f..b9fda8d6fa4 100644 --- a/src/global_styling/functions/typography.test.tsx +++ b/src/global_styling/functions/typography.test.tsx @@ -9,7 +9,7 @@ import React, { FunctionComponent, PropsWithChildren } from 'react'; import { renderHook } from '@testing-library/react'; -import { EuiProvider } from '../../components'; +import { EuiProvider } from '../../components/provider'; import { useEuiTheme } from '../../services'; import { EuiThemeFontScales, EuiThemeFontUnits } from '../variables/typography'; diff --git a/src/services/theme/hooks.test.tsx b/src/services/theme/hooks.test.tsx index d3c5812ebd0..4e9014a290b 100644 --- a/src/services/theme/hooks.test.tsx +++ b/src/services/theme/hooks.test.tsx @@ -10,7 +10,7 @@ import React from 'react'; import { render, act } from '@testing-library/react'; import { renderHook } from '../../test/rtl'; -import { EuiProvider } from '../../components'; +import { EuiProvider } from '../../components/provider'; import { setEuiDevProviderWarning } from './warning'; import { From f80952b07e91a2c3dcf41b124d218790cce5fa1b Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 11 Sep 2023 15:02:30 -0700 Subject: [PATCH 7/9] [src-docs] Fix import string somehow triggering bizarre Jest find related tests bug?? - the `require`->`import` changes are syntactical sugar only, the main fix is line 44 (and no I don't understand why Jest is looking at strings lol) --- src-docs/src/components/guide_section/_utils.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src-docs/src/components/guide_section/_utils.test.js b/src-docs/src/components/guide_section/_utils.test.js index 64f5600e063..502fe6ea2b2 100644 --- a/src-docs/src/components/guide_section/_utils.test.js +++ b/src-docs/src/components/guide_section/_utils.test.js @@ -1,5 +1,5 @@ -const dedent = require('dedent'); -const { renderJsSourceCode } = require('./_utils.js'); +import dedent from 'dedent'; +import { renderJsSourceCode } from './_utils'; describe('renderJsSourceCode', () => { describe('EUI imports', () => { @@ -41,8 +41,8 @@ describe('renderJsSourceCode', () => { expect( renderJsSourceCode({ default: dedent(` - import { EuiButton, EuiFlexGroup, EuiFlexItem } from '../../../src/components'; - import { useGeneratedHtmlId } from '../../../src/services'; + import { EuiButton, EuiFlexGroup, EuiFlexItem } from '../../src/components'; + import { useGeneratedHtmlId } from '../../src/services'; export default () => {useGeneratedHtmlId()};`), }) From e4b32fe2474338f2552a47aa1d76ac8fd2a7d99f Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 11 Sep 2023 15:04:03 -0700 Subject: [PATCH 8/9] Fix `src/test` imports pointed at `src/components/` - which again, greatly messes up Jest's find related tests logic --- src/test/internal/render_custom_styles.tsx | 2 +- src/test/rtl/custom_render.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/internal/render_custom_styles.tsx b/src/test/internal/render_custom_styles.tsx index cd734df0660..3e54ac06839 100644 --- a/src/test/internal/render_custom_styles.tsx +++ b/src/test/internal/render_custom_styles.tsx @@ -12,7 +12,7 @@ import { get, set } from 'lodash'; import { render } from '../rtl'; import { cloneElementWithCss } from '../../services'; -import { keysOf } from '../../components'; +import { keysOf } from '../../components/common'; export const customStyles = { className: 'hello', diff --git a/src/test/rtl/custom_render.ts b/src/test/rtl/custom_render.ts index 98bd66773a1..c07927a5a97 100644 --- a/src/test/rtl/custom_render.ts +++ b/src/test/rtl/custom_render.ts @@ -16,7 +16,7 @@ import { within, } from '@testing-library/react'; -import { EuiProvider } from '../../components'; +import { EuiProvider } from '../../components/provider'; import * as dataTestSubjQueries from './data_test_subj_queries'; From 9b7f276b391dace245b4a09d46d877ad497d10b5 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 11 Sep 2023 15:29:03 -0700 Subject: [PATCH 9/9] Add regression test for Jest `--findRelatedTests` behavior - a lint rule might be better long-term, but this will at least warn us if our hook starts running extra tests again --- scripts/tests/test-staged.test.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 scripts/tests/test-staged.test.js diff --git a/scripts/tests/test-staged.test.js b/scripts/tests/test-staged.test.js new file mode 100644 index 00000000000..7f96d28f478 --- /dev/null +++ b/scripts/tests/test-staged.test.js @@ -0,0 +1,24 @@ +const { execSync } = require('child_process'); +const path = require('path'); +const euiRoot = path.resolve(__dirname, '../../'); // NOTE: CI lists the EUI project root as app/ instead of eui/ + +describe('test-staged.js', () => { + it('does not have --findRelatedTests regressions due to imports from src/components', () => { + const outputString = execSync( + 'yarn test-unit --findRelatedTests --listTests src/components/beacon/beacon.tsx', + { cwd: euiRoot } + ).toString(); + const output = outputString + .split('\n') + .filter((item) => item.endsWith('test.tsx')); + + expect( + output[0].endsWith('src/components/tour/tour_step.test.tsx') + ).toBeTruthy(); + expect( + output[1].endsWith('src/components/beacon/beacon.test.tsx') + ).toBeTruthy(); + + expect(output[2]).toBeUndefined(); + }); +});