Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore(deps-dev): bump jest from 27.5.1 to 28.0.0 (#5421)
* chore(deps-dev): bump jest from 27.5.1 to 28.0.0 Bumps [jest](https://github.com/facebook/jest/tree/HEAD/packages/jest) from 27.5.1 to 28.0.0. - [Release notes](https://github.com/facebook/jest/releases) - [Changelog](https://github.com/facebook/jest/blob/main/CHANGELOG.md) - [Commits](https://github.com/facebook/jest/commits/v28.0.0/packages/jest) --- updated-dependencies: - dependency-name: jest dependency-type: direct:development update-type: version-update:semver-major ... --- This required some manual updates. The easy ones were: * Jest 28 no longer includes `jest-environment-jsdom` as an implicit dependency. Our unit and e2e tests both use this, so I've added an explicit dependency for it. * Jest 28 adds a new `github-actions` reporter available by default. This seems like an obvious win, so I went ahead and enabled it. * Jest 28 adds native support for a `--shard` argument; I've replaced the janky manual version our GitHub CI build was using with the new native support. I fixed the off-by-one issue with our sharded job naming while I was here. I'll update the `main` branch policy accordingly once this PR merges. The other three were more interesting; I've given them separate comments for ease of linking them. --- #### Jest 28 tries to use an ESM version of `uuid` by default Jest 28 attempts to respect `package.json` `exports` fields based on the test environment you're using. This means that for packages that export separate entry points for node vs browser environments, Jest will attempt to use the browser entry point when you're using `jest-environment-jsdom` and the node entry point when you're using `jest-environment-node`. This is a good change that we want to use in most cases; it keeps our test environment closer to a real browser environment. Unfortunately, one of our dependencies (`uuid`) only provides an ESM implementation (not CommonJS) for browsers, though it provides both ESM and CommonJS versions for node. [This comment on uuid#620 summarizes their exports matrix](uuidjs/uuid#620 (comment)), and [uuid#616's discussion](uuidjs/uuid#616) has some context on the issue. `uuid`'s `package.json` indicates that its ESM+browser entry point is what *all* browser cases should use, so Jest ends up trying to use that even though Jest tries to use a CommonJS+browser entry point where available. This produces hundreds of errors that look something like this: <details> <summary>Full example of an error</summary> ``` ● Test suite failed to run Jest encountered an unexpected token Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax. Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration. By default "node_modules" folder is ignored by transformers. Here's what you can do: • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it. • If you are trying to use TypeScript, see https://jestjs.io/docs/getting-started#using-typescript • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config. • If you need a custom transformation specify a "transform" option in your config. • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option. You'll find more details and examples of these config options in the docs: https://jestjs.io/docs/configuration For information about custom transformations, see: https://jestjs.io/docs/code-transformation Details: C:\repos\accessibility-insights-web\node_modules\uuid\dist\esm-browser\index.js:1 ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){export { default as v1 } from './v1.js'; ^^^^^^ SyntaxError: Unexpected token 'export' 1 | // Copyright (c) Microsoft Corporation. All rights reserved. 2 | // Licensed under the MIT License. > 3 | import { v4 } from 'uuid'; | ^ 4 | 5 | export type UUIDGenerator = () => string; 6 | at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1773:14) at Object.<anonymous> (src/common/uid-generator.ts:3:26) ``` </details> I considered a few options for addressing this: * We could wait and hope that `uuid` starts including a CommonJS + browser entry point, like uuidjs/uuid#616 proposes. Jest would start picking this up without any changes, but this would fail because such a `uuid` entry point would assume the presence of a browser `crypto.getRandomValues` API. We would then have to either wait for JSDOM to start providing this (probably via jsdom/jsdom#3352) or provide a polyfill as part of our test environment/test setup. This is probably not a good solution because it seems unlikely that `uuid` will add a CommonJS browser build; the most recent word I saw from `uuid`'s maintainers was that they were ["convinced that adding a CommonJS browser build would be fundamentally wrong at this point in time"](uuidjs/uuid#620 (comment)) * We could try to make ESM+Jest work (this would fix the immediate issue but would still require a `crypto` implementation). [Jest's native support for this is currently blocked on node/v8 issues](jestjs/jest#9430), but it's possible we could work around that with something like https://github.com/nicolo-ribaudo/jest-light-runner. * We could override the `exports` conditions that `jest-environment-jsdom` passes, per [the suggestion in the Jest v27 to v28 upgrade guide](https://jestjs.io/docs/upgrading-to-jest28#packagejson-exports). As I understand it, this would amount to creating our own custom Jest Test Environment wrapping `jest-environment-jsdom` and overriding its [`exportConditions`](https://github.com/facebook/jest/blob/v28.0.0/packages/jest-environment-jsdom/src/index.ts#L160) property to look for use `node` instead of `browser`. This is something we could do ourselves immediately and much less work than moving to ESM, but I didn't love this solution because I wanted to keep the proper export conditions in the cases where they aren't broken * We could use a Jest `moduleNameMapper` entry like `'^uuid$': require.resolve('uuid')` to force the use of a Node+CommonJS version of `uuid`. This works and is more scoped than overriding `exports` conditions, but has the downside that it is essentially a silent `yarn resolution`; it forces *every* dependency chain through `uuid` to use whatever version of `uuid` happens to be hoisted, even if some chains want older versions. This would make future `uuid` upgrades very dangerous. See uuidjs/uuid#616 (comment) for context. * The option I chose: create a [custom Jest resolver](9ad4e61) which overrides only the specific behavior of how Jest resolves the `uuid` dependency, forcing it use the CommonJS+node version despite `jest-environment-jsdom`'s export conditions. --- #### `jsdom` v17 assumes the availability of a global `TextEncoder`, which `jest-environment-jsdom` does not provide Because we updated how we pull in `jest-environment-jsdom`, we ended up updating the version of `jsdom` that we use in practice, so we started hitting this not-entirely-new issue. This one is described by jsdom/jsdom#2524 (comment) (the original issue is for a separate-but-related feature request, which was unfortunately hijacked). The symptom is an error of form `ReferenceError: TextEncoder is not defined` on a line importing `jsdom`. <details> <summary>Full example of an error</summary> ``` FAIL unit tests src/tests/unit/tests/injected/visualization/drawer.test.ts ● Test suite failed to run ReferenceError: TextEncoder is not defined 1 | // Copyright (c) Microsoft Corporation. All rights reserved. 2 | // Licensed under the MIT License. > 3 | import { JSDOM } from 'jsdom'; | ^ 4 | 5 | export class TestDocumentCreator { 6 | public static createTestDocument(html: string = ''): Document { at Object.<anonymous> (node_modules/jsdom/node_modules/whatwg-url/lib/encoding.js:2:21) at Object.<anonymous> (node_modules/jsdom/node_modules/whatwg-url/lib/url-state-machine.js:5:34) at Object.<anonymous> (node_modules/jsdom/node_modules/whatwg-url/lib/URL-impl.js:2:13) at Object.<anonymous> (node_modules/jsdom/node_modules/whatwg-url/lib/URL.js:442:14) at Object.<anonymous> (node_modules/jsdom/node_modules/whatwg-url/webidl2js-wrapper.js:3:13) at Object.<anonymous> (node_modules/jsdom/node_modules/whatwg-url/index.js:3:34) at Object.<anonymous> (node_modules/jsdom/lib/api.js:7:19) at Object.<anonymous> (src/tests/unit/common/test-document-creator.ts:3:30) at Object.<anonymous> (src/tests/unit/tests/injected/visualization/drawer.test.ts:15:78) ``` </details> The issue is that we're trying to import `jsdom` from within a `jest-environment-jsdom` environment. `jsdom` uses a global `TextEncoder` as part of its implementation, but doesn't export one globally. The suggested workaround from the Jest maintainer is to not import `jsdom` reentrantly, and to instead use `jest-environment-node` for tests that import `jsdom` themselves. We don't want to do that; we have some tests that want to import `jsdom` to create a fresh DOM environment to test in isolation with, but some of our transitive dependencies assume the availability of a global `document`/`window` for reasons unimportant to those specific tests. Instead, I worked around this by just re-exporting Node `util`'s `TextEncoder` as `window.TextEncoder` in our unit tests' `jest-setup`. --- #### Jest 28 forbids `describe(SomeThing, ...)` for `SomeThing`s without `name`s In Jest 27, specifying `describe(SomeThing, ...)` when `SomeThing` does not have a name (eg, an arrow function) was not an error; it just produced tests where the "SomeThing" that was intended to be filled in at the beginning of a test name was actually filled in as an empty string. Jest 28 has become more strict about this. Passing a named function or a named class is now explicitly supported, but passing an unnamed thing now produces an error of the form `Invalid first argument, <stringified version of Thing>. It must be a named class, named function, number, or string.` In practice, this error message is very challenging to parse because <stringified version of Thing> is the entire definition of a function, complete with code coverage markers: <details> <summary>Example of one of these errors</summary> ``` FAIL unit tests src/tests/unit/tests/DetailsView/details-view-content.test.tsx ● Test suite failed to run Invalid first argument, function (props) { /* istanbul ignore next */ cov_7cparukpf().f[1]++; var selectedDetailsViewSwitcherNavConfiguration = /* istanbul ignore next */ (cov_7cparukpf().s[20]++, props.deps.getDetailsSwitcherNavConfiguration({ selectedDetailsViewPivot: props.storeState.visualizationStoreData.selectedDetailsViewPivot })); /* istanbul ignore next */ cov_7cparukpf().s[21]++; var renderHeader = function () { /* istanbul ignore next */ cov_7cparukpf().f[2]++; var storeState = /* istanbul ignore next */ (cov_7cparukpf().s[22]++, props.storeState); var visualizationStoreData = /* istanbul ignore next */ (cov_7cparukpf().s[23]++, storeState.visualizationStoreData); /* istanbul ignore next */ cov_7cparukpf().s[24]++; return /*#__PURE__*/React.createElement(_interactiveHeader.InteractiveHeader, { deps: props.deps, selectedPivot: visualizationStoreData.selectedDetailsViewPivot, featureFlagStoreData: storeState.featureFlagStoreData, tabClosed: props.storeState.tabStoreData.isClosed, navMenu: selectedDetailsViewSwitcherNavConfiguration.leftNavHamburgerButton, isSideNavOpen: props.isSideNavOpen, setSideNavOpen: props.setSideNavOpen, narrowModeStatus: props.narrowModeStatus }); }; /* istanbul ignore next */ cov_7cparukpf().s[25]++; var renderOverlay = function () { /* istanbul ignore next */ cov_7cparukpf().f[3]++; var deps = /* istanbul ignore next */ (cov_7cparukpf().s[26]++, props.deps), storeState = /* istanbul ignore next */ (cov_7cparukpf().s[27]++, props.storeState); /* istanbul ignore next */ cov_7cparukpf().s[28]++; return /*#__PURE__*/React.createElement(_detailsViewOverlay.DetailsViewOverlay, { deps: deps, previewFeatureFlagsHandler: props.deps.previewFeatureFlagsHandler, scopingActionMessageCreator: props.deps.scopingActionMessageCreator, inspectActionMessageCreator: props.deps.inspectActionMessageCreator, detailsViewStoreData: storeState.detailsViewStoreData, scopingStoreData: storeState.scopingPanelStateStoreData, featureFlagStoreData: storeState.featureFlagStoreData, userConfigurationStoreData: storeState.userConfigurationStoreData }); }; /* istanbul ignore next */ cov_7cparukpf().s[29]++; var renderDetailsView = function () { /* istanbul ignore next */ cov_7cparukpf().f[4]++; var deps = /* istanbul ignore next */ (cov_7cparukpf().s[30]++, props.deps), storeState = /* istanbul ignore next */ (cov_7cparukpf().s[31]++, props.storeState); var selectedDetailsRightPanelConfiguration = /* istanbul ignore next */ (cov_7cparukpf().s[32]++, props.deps.getDetailsRightPanelConfiguration({ selectedDetailsViewPivot: storeState.visualizationStoreData.selectedDetailsViewPivot, detailsViewRightContentPanel: storeState.detailsViewStoreData.detailsViewRightContentPanel })); var selectedTest = /* istanbul ignore next */ (cov_7cparukpf().s[33]++, selectedDetailsViewSwitcherNavConfiguration.getSelectedDetailsView(storeState)); var automatedChecksCardsViewData = /* istanbul ignore next */ (cov_7cparukpf().s[34]++, props.deps.getCardViewData(props.storeState.unifiedScanResultStoreData.rules, props.storeState.unifiedScanResultStoreData.results, props.deps.getCardSelectionViewData(props.storeState.cardSelectionStoreData, props.storeState.unifiedScanResultStoreData, props.deps.isResultHighlightUnavailable))); var tabStopRequirementData = /* istanbul ignore next */ (cov_7cparukpf().s[35]++, props.storeState.visualizationScanResultStoreData.tabStops.requirements); var needsReviewCardsViewData = /* istanbul ignore next */ (cov_7cparukpf().s[[36](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:36)]++, props.deps.getCardViewData(props.storeState.needsReviewScanResultStoreData.rules, props.storeState.needsReviewScanResultStoreData.results, props.deps.getCardSelectionViewData(props.storeState.needsReviewCardSelectionStoreData, props.storeState.needsReviewScanResultStoreData, props.deps.isResultHighlightUnavailable))); var targetAppInfo = /* istanbul ignore next */ (cov_7cparukpf().s[[37](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:37)]++, { name: props.storeState.tabStoreData.title, url: props.storeState.tabStoreData.url }); var scanDate = /* istanbul ignore next */ (cov_7cparukpf().s[[38](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:38)]++, props.deps.getDateFromTimestamp(props.storeState.unifiedScanResultStoreData.timestamp)); var scanMetadata = /* istanbul ignore next */ (cov_7cparukpf().s[[39](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:39)]++, { timespan: { scanComplete: scanDate }, targetAppInfo: targetAppInfo, toolData: props.storeState.unifiedScanResultStoreData.toolInfo }); /* istanbul ignore next */ cov_7cparukpf().s[[40](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:40)]++; return /*#__PURE__*/React.createElement(_detailsViewBody.DetailsViewBody, { deps: deps, tabStoreData: storeState.tabStoreData, tabStopsViewStoreData: storeState.tabStopsViewStoreData, assessmentStoreData: storeState.assessmentStoreData, pathSnippetStoreData: storeState.pathSnippetStoreData, featureFlagStoreData: storeState.featureFlagStoreData, selectedTest: selectedTest, detailsViewStoreData: storeState.detailsViewStoreData, visualizationStoreData: storeState.visualizationStoreData, visualizationScanResultData: storeState.visualizationScanResultStoreData, visualizationConfigurationFactory: props.deps.visualizationConfigurationFactory, assessmentsProvider: props.deps.assessmentsProvider, dropdownClickHandler: props.deps.dropdownClickHandler, clickHandlerFactory: props.deps.clickHandlerFactory, assessmentInstanceTableHandler: props.deps.assessmentInstanceTableHandler, issuesTableHandler: props.deps.issuesTableHandler, rightPanelConfiguration: selectedDetailsRightPanelConfiguration, switcherNavConfiguration: selectedDetailsViewSwitcherNavConfiguration, userConfigurationStoreData: storeState.userConfigurationStoreData, automatedChecksCardsViewData: automatedChecksCardsViewData, needsReviewCardsViewData: needsReviewCardsViewData, scanIncompleteWarnings: storeState.unifiedScanResultStoreData.scanIncompleteWarnings, scanMetadata: scanMetadata, isSideNavOpen: props.isSideNavOpen, setSideNavOpen: props.setSideNavOpen, narrowModeStatus: props.narrowModeStatus, tabStopRequirementData: tabStopRequirementData }); }; /* istanbul ignore next */ cov_7cparukpf().s[[41](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:41)]++; return /*#__PURE__*/React.createElement(React.Fragment, null, renderHeader(), renderDetailsView(), renderOverlay()); }. It must be a named class, named function, number, or string. [49](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:49) | import { StoreMocks } from './store-mocks'; [50](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:50) | > [51](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:51) | describe(DetailsViewContent, () => { | ^ [52](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:52) | const pageTitle = 'DetailsViewContainerTest title'; [53](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:53) | const pageUrl = 'http://detailsViewContainerTest/url/'; [54](https://github.com/microsoft/accessibility-insights-web/runs/6167262184?check_suite_focus=true#step:7:54) | let detailsViewActionMessageCreator: IMock<DetailsViewActionMessageCreator>; at Object.describe (src/tests/unit/tests/DetailsView/details-view-content.test.tsx:51:1) ``` </details> 18 of our test files trigger this issue. In every case, the unnamed component in question is a `NamedFC` React component. The `displayName` that `NamedFC` gives these components is just for the benefit of React debugging messages, and doesn't help Jest - these really are 18 test files with test names like ` renders normally` instead of `ThingUnderTest renders normally`, with an empty string where the component name belongs. I think the ideal way to resolve this would be to completely remove our `NamedFC` wrapper and replace it with `eslint-plugin-react`'s [display-name](https://github.com/jsx-eslint/eslint-plugin-react/blob/HEAD/docs/rules/display-name.md) and/or [function-component-definition](https://github.com/jsx-eslint/eslint-plugin-react/blob/HEAD/docs/rules/function-component-definition.md) rules (with similar settings to [what AirBnB uses as of this issue's resolution](airbnb/javascript#2505)). That would be a huge change, though; we have hundreds of components that use `NamedFC`. I'd want to do a separate PR for that. Instead, this PR just changes the 18 tests in question to use `TheNamedFC.displayName`; this has the advantage that it will break obviously if we ever do the bigger change. --- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Dan Bjorge <danielbj@microsoft.com>
- Loading branch information
1 parent
0d6ff85
commit f267f8d