Skip to content

Commit

Permalink
fix[ci]: fixed jest configuration not to skip too many devtools tests (
Browse files Browse the repository at this point in the history
…facebook#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:
- facebook#26742
- facebook#25712
- facebook#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
<img width="1447" alt="Screenshot 2023-06-15 at 11 18 22"
src="https://github.com/facebook/react/assets/28902667/cad58994-2cb3-44b3-9eb2-1699c01a1eb3">

Running `profilingCache` test suite with specifying `reactVersion` to
`17` now runs this test case and skips others correctly
<img width="1447" alt="Screenshot 2023-06-15 at 11 20 11"
src="https://github.com/facebook/react/assets/28902667/d308960a-c172-4422-ba6f-9c0dbcd6f7d5">

Running `yarn test --project devtools ...` without specifying
`reactVersion` now runs all non-gated test cases
<img width="398" alt="Screenshot 2023-06-15 at 12 25 12"
src="https://github.com/facebook/react/assets/28902667/2b329634-0efd-4c4c-b460-889696bbc9e1">

Running `yarn test --project devtools ...` with specifying
`reactVersion` (to `17` in this example) now includes only gated tests
<img width="414" alt="Screenshot 2023-06-15 at 12 26 31"
src="https://github.com/facebook/react/assets/28902667/a702c27e-4c35-4b12-834c-e5bb06728997">
  • Loading branch information
hoxyq authored and AndyPengc12 committed Apr 15, 2024
1 parent 5040f3c commit 09d26f1
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 35 deletions.
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
},
],
},
]
`);
Expand Down
Expand Up @@ -10,7 +10,6 @@ const semver = require('semver');

let shouldPass;
let isFocused;
let shouldIgnore;
describe('transform-react-version-pragma', () => {
const originalTest = test;

Expand All @@ -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);
});
Expand All @@ -43,7 +41,6 @@ describe('transform-react-version-pragma', () => {
beforeEach(() => {
shouldPass = null;
isFocused = false;
shouldIgnore = false;
});

// @reactVersion >= 17.9
Expand Down Expand Up @@ -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);
});
});
5 changes: 3 additions & 2 deletions scripts/babel/transform-react-version-pragma.js
Expand Up @@ -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;
Expand Down Expand Up @@ -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';
}
}
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions 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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
12 changes: 3 additions & 9 deletions scripts/jest/jest-cli.js
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 1 addition & 6 deletions scripts/jest/preprocessor.js
Expand Up @@ -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});
Expand Down

0 comments on commit 09d26f1

Please sign in to comment.