From 09d26f1f177c5035c98cff5f72b7f3dd23faec5d Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 22 Jun 2023 09:33:05 +0100 Subject: [PATCH] fix[ci]: fixed jest configuration not to skip too many devtools tests (#26955) ## Summary Running `yarn test --project devtools --build` currently skips all non-gated (without `@reactVersion` directives) devtools tests. This is not expected behaviour, these changes are fixing it. There were multiple related PRs to it: - https://github.com/facebook/react/pull/26742 - https://github.com/facebook/react/pull/25712 - https://github.com/facebook/react/pull/24555 With these changes, the resulting behaviour will be: - If `REACT_VERSION` env variable is specified: - jest will not include all non-gated test cases in the test run - jest will run only a specific test case, when specified `REACT_VERSION` value satisfies the range defined by `@reactVersion` directives for this test case - If `REACT_VERSION` env variable is not specified, jest will run all non-gated tests: - jest will include all non-gated test cases in the test run - jest will run all non-gated test cases, the only skipped test cases will be those, which specified the range that does not include the next stable version of react, which will be imported from `ReactVersions.js` ## How did you test this change? Running `profilingCache` test suite without specifying `reactVersion` now skips gated (>= 17 & < 18) test Screenshot 2023-06-15 at 11 18 22 Running `profilingCache` test suite with specifying `reactVersion` to `17` now runs this test case and skips others correctly Screenshot 2023-06-15 at 11 20 11 Running `yarn test --project devtools ...` without specifying `reactVersion` now runs all non-gated test cases Screenshot 2023-06-15 at 12 25 12 Running `yarn test --project devtools ...` with specifying `reactVersion` (to `17` in this example) now includes only gated tests Screenshot 2023-06-15 at 12 26 31 --- .../src/__tests__/profilingCache-test.js | 13 +++++++++++-- .../transform-react-version-pragma-test.js | 13 ------------- scripts/babel/transform-react-version-pragma.js | 5 +++-- scripts/jest/devtools/setupEnv.js | 6 +++--- scripts/jest/jest-cli.js | 12 +++--------- scripts/jest/preprocessor.js | 7 +------ 6 files changed, 21 insertions(+), 35 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js index 892a5ca14c40..f03bf2391525 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js @@ -938,7 +938,8 @@ describe('ProfilingCache', () => { } }); - // @reactVersion = 17.0 + // @reactVersion >= 17 + // @reactVersion < 18 it('should handle unexpectedly shallow suspense trees', () => { // This test only runs in v17 because it's a regression test for legacy // Suspense behavior, and the implementation details changed in v18. @@ -967,7 +968,15 @@ describe('ProfilingCache', () => { "passiveEffectDuration": null, "priorityLevel": "Normal", "timestamp": 0, - "updaters": null, + "updaters": [ + { + "displayName": "render()", + "hocDisplayNames": null, + "id": 1, + "key": null, + "type": 11, + }, + ], }, ] `); diff --git a/packages/react-devtools-shared/src/__tests__/transform-react-version-pragma-test.js b/packages/react-devtools-shared/src/__tests__/transform-react-version-pragma-test.js index 2e3e3d752d24..49a9f96117cc 100644 --- a/packages/react-devtools-shared/src/__tests__/transform-react-version-pragma-test.js +++ b/packages/react-devtools-shared/src/__tests__/transform-react-version-pragma-test.js @@ -10,7 +10,6 @@ const semver = require('semver'); let shouldPass; let isFocused; -let shouldIgnore; describe('transform-react-version-pragma', () => { const originalTest = test; @@ -34,7 +33,6 @@ describe('transform-react-version-pragma', () => { // eslint-disable-next-line no-unused-vars const _test_ignore_for_react_version = (testName, cb) => { originalTest(testName, (...args) => { - shouldIgnore = true; shouldPass = false; return cb(...args); }); @@ -43,7 +41,6 @@ describe('transform-react-version-pragma', () => { beforeEach(() => { shouldPass = null; isFocused = false; - shouldIgnore = false; }); // @reactVersion >= 17.9 @@ -137,14 +134,4 @@ describe('transform-react-version-pragma', () => { expect(shouldPass).toBe(true); expect(isFocused).toBe(true); }); - - test('ignore test if no reactVersion', () => { - expect(shouldPass).toBe(false); - expect(shouldIgnore).toBe(true); - }); - - test.only('ignore focused test if no reactVersion', () => { - expect(shouldPass).toBe(false); - expect(shouldIgnore).toBe(true); - }); }); diff --git a/scripts/babel/transform-react-version-pragma.js b/scripts/babel/transform-react-version-pragma.js index 1414852d2cf5..d7d42f865cf8 100644 --- a/scripts/babel/transform-react-version-pragma.js +++ b/scripts/babel/transform-react-version-pragma.js @@ -5,6 +5,7 @@ const getComments = require('./getComments'); const GATE_VERSION_STR = '@reactVersion '; +const REACT_VERSION_ENV = process.env.REACT_VERSION; function transform(babel) { const {types: t} = babel; @@ -75,7 +76,7 @@ function transform(babel) { ? '_test_react_version_focus' : '_test_react_version'; expression.arguments = [condition, ...expression.arguments]; - } else { + } else if (REACT_VERSION_ENV) { callee.name = '_test_ignore_for_react_version'; } } @@ -96,7 +97,7 @@ function transform(babel) { t.identifier('_test_react_version_focus'), [condition, ...expression.arguments] ); - } else { + } else if (REACT_VERSION_ENV) { statement.expression = t.callExpression( t.identifier('_test_ignore_for_react_version'), expression.arguments diff --git a/scripts/jest/devtools/setupEnv.js b/scripts/jest/devtools/setupEnv.js index b53ad4e89360..b3ef1b9387dc 100644 --- a/scripts/jest/devtools/setupEnv.js +++ b/scripts/jest/devtools/setupEnv.js @@ -1,7 +1,7 @@ 'use strict'; const semver = require('semver'); -const ReactVersion = require('../../../packages/shared/ReactVersion'); +const {ReactVersion} = require('../../../ReactVersions'); const { DARK_MODE_DIMMED_WARNING_COLOR, @@ -32,7 +32,7 @@ global.process.env.LIGHT_MODE_DIMMED_ERROR_COLOR = global.process.env.LIGHT_MODE_DIMMED_LOG_COLOR = LIGHT_MODE_DIMMED_LOG_COLOR; global._test_react_version = (range, testName, callback) => { - const reactVersion = process.env.REACT_VERSION || ReactVersion.default; + const reactVersion = process.env.REACT_VERSION || ReactVersion; const shouldPass = semver.satisfies(reactVersion, range); if (shouldPass) { @@ -43,7 +43,7 @@ global._test_react_version = (range, testName, callback) => { }; global._test_react_version_focus = (range, testName, callback) => { - const reactVersion = process.env.REACT_VERSION || ReactVersion.default; + const reactVersion = process.env.REACT_VERSION || ReactVersion; const shouldPass = semver.satisfies(reactVersion, range); if (shouldPass) { diff --git a/scripts/jest/jest-cli.js b/scripts/jest/jest-cli.js index dd36597e2909..af7bc3be7707 100644 --- a/scripts/jest/jest-cli.js +++ b/scripts/jest/jest-cli.js @@ -16,8 +16,6 @@ const devToolsConfig = './scripts/jest/config.build-devtools.js'; const persistentConfig = './scripts/jest/config.source-persistent.js'; const buildConfig = './scripts/jest/config.build.js'; -const {ReactVersion} = require('../../ReactVersions'); - const argv = yargs .parserConfiguration({ // Important: This option tells yargs to move all other options not @@ -181,13 +179,9 @@ function validateOptions() { success = false; } - if (argv.reactVersion) { - if (!semver.validRange(argv.reactVersion)) { - success = false; - logError('please specify a valid version range for --reactVersion'); - } - } else { - argv.reactVersion = ReactVersion; + if (argv.reactVersion && !semver.validRange(argv.reactVersion)) { + success = false; + logError('please specify a valid version range for --reactVersion'); } } else { if (argv.compactConsole) { diff --git a/scripts/jest/preprocessor.js b/scripts/jest/preprocessor.js index 6826930c7025..91f629c85f92 100644 --- a/scripts/jest/preprocessor.js +++ b/scripts/jest/preprocessor.js @@ -86,12 +86,7 @@ module.exports = { const plugins = (isTestFile ? testOnlyPlugins : sourceOnlyPlugins).concat( babelOptions.plugins ); - if ( - isTestFile && - isInDevToolsPackages && - (process.env.REACT_VERSION || - filePath.match(/\/transform-react-version-pragma-test/)) - ) { + if (isTestFile && isInDevToolsPackages) { plugins.push(pathToTransformReactVersionPragma); } let sourceAst = hermesParser.parse(src, {babel: true});