Skip to content

Commit c8deb5d

Browse files
authored
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: - #26742 - #25712 - #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">
1 parent 6c84b50 commit c8deb5d

File tree

6 files changed

+21
-35
lines changed

6 files changed

+21
-35
lines changed

packages/react-devtools-shared/src/__tests__/profilingCache-test.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -938,7 +938,8 @@ describe('ProfilingCache', () => {
938938
}
939939
});
940940

941-
// @reactVersion = 17.0
941+
// @reactVersion >= 17
942+
// @reactVersion < 18
942943
it('should handle unexpectedly shallow suspense trees', () => {
943944
// This test only runs in v17 because it's a regression test for legacy
944945
// Suspense behavior, and the implementation details changed in v18.
@@ -967,7 +968,15 @@ describe('ProfilingCache', () => {
967968
"passiveEffectDuration": null,
968969
"priorityLevel": "Normal",
969970
"timestamp": 0,
970-
"updaters": null,
971+
"updaters": [
972+
{
973+
"displayName": "render()",
974+
"hocDisplayNames": null,
975+
"id": 1,
976+
"key": null,
977+
"type": 11,
978+
},
979+
],
971980
},
972981
]
973982
`);

packages/react-devtools-shared/src/__tests__/transform-react-version-pragma-test.js

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ const semver = require('semver');
1010

1111
let shouldPass;
1212
let isFocused;
13-
let shouldIgnore;
1413
describe('transform-react-version-pragma', () => {
1514
const originalTest = test;
1615

@@ -34,7 +33,6 @@ describe('transform-react-version-pragma', () => {
3433
// eslint-disable-next-line no-unused-vars
3534
const _test_ignore_for_react_version = (testName, cb) => {
3635
originalTest(testName, (...args) => {
37-
shouldIgnore = true;
3836
shouldPass = false;
3937
return cb(...args);
4038
});
@@ -43,7 +41,6 @@ describe('transform-react-version-pragma', () => {
4341
beforeEach(() => {
4442
shouldPass = null;
4543
isFocused = false;
46-
shouldIgnore = false;
4744
});
4845

4946
// @reactVersion >= 17.9
@@ -137,14 +134,4 @@ describe('transform-react-version-pragma', () => {
137134
expect(shouldPass).toBe(true);
138135
expect(isFocused).toBe(true);
139136
});
140-
141-
test('ignore test if no reactVersion', () => {
142-
expect(shouldPass).toBe(false);
143-
expect(shouldIgnore).toBe(true);
144-
});
145-
146-
test.only('ignore focused test if no reactVersion', () => {
147-
expect(shouldPass).toBe(false);
148-
expect(shouldIgnore).toBe(true);
149-
});
150137
});

scripts/babel/transform-react-version-pragma.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
const getComments = require('./getComments');
66

77
const GATE_VERSION_STR = '@reactVersion ';
8+
const REACT_VERSION_ENV = process.env.REACT_VERSION;
89

910
function transform(babel) {
1011
const {types: t} = babel;
@@ -75,7 +76,7 @@ function transform(babel) {
7576
? '_test_react_version_focus'
7677
: '_test_react_version';
7778
expression.arguments = [condition, ...expression.arguments];
78-
} else {
79+
} else if (REACT_VERSION_ENV) {
7980
callee.name = '_test_ignore_for_react_version';
8081
}
8182
}
@@ -96,7 +97,7 @@ function transform(babel) {
9697
t.identifier('_test_react_version_focus'),
9798
[condition, ...expression.arguments]
9899
);
99-
} else {
100+
} else if (REACT_VERSION_ENV) {
100101
statement.expression = t.callExpression(
101102
t.identifier('_test_ignore_for_react_version'),
102103
expression.arguments

scripts/jest/devtools/setupEnv.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
22

33
const semver = require('semver');
4-
const ReactVersion = require('../../../packages/shared/ReactVersion');
4+
const {ReactVersion} = require('../../../ReactVersions');
55

66
const {
77
DARK_MODE_DIMMED_WARNING_COLOR,
@@ -32,7 +32,7 @@ global.process.env.LIGHT_MODE_DIMMED_ERROR_COLOR =
3232
global.process.env.LIGHT_MODE_DIMMED_LOG_COLOR = LIGHT_MODE_DIMMED_LOG_COLOR;
3333

3434
global._test_react_version = (range, testName, callback) => {
35-
const reactVersion = process.env.REACT_VERSION || ReactVersion.default;
35+
const reactVersion = process.env.REACT_VERSION || ReactVersion;
3636
const shouldPass = semver.satisfies(reactVersion, range);
3737

3838
if (shouldPass) {
@@ -43,7 +43,7 @@ global._test_react_version = (range, testName, callback) => {
4343
};
4444

4545
global._test_react_version_focus = (range, testName, callback) => {
46-
const reactVersion = process.env.REACT_VERSION || ReactVersion.default;
46+
const reactVersion = process.env.REACT_VERSION || ReactVersion;
4747
const shouldPass = semver.satisfies(reactVersion, range);
4848

4949
if (shouldPass) {

scripts/jest/jest-cli.js

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ const devToolsConfig = './scripts/jest/config.build-devtools.js';
1616
const persistentConfig = './scripts/jest/config.source-persistent.js';
1717
const buildConfig = './scripts/jest/config.build.js';
1818

19-
const {ReactVersion} = require('../../ReactVersions');
20-
2119
const argv = yargs
2220
.parserConfiguration({
2321
// Important: This option tells yargs to move all other options not
@@ -181,13 +179,9 @@ function validateOptions() {
181179
success = false;
182180
}
183181

184-
if (argv.reactVersion) {
185-
if (!semver.validRange(argv.reactVersion)) {
186-
success = false;
187-
logError('please specify a valid version range for --reactVersion');
188-
}
189-
} else {
190-
argv.reactVersion = ReactVersion;
182+
if (argv.reactVersion && !semver.validRange(argv.reactVersion)) {
183+
success = false;
184+
logError('please specify a valid version range for --reactVersion');
191185
}
192186
} else {
193187
if (argv.compactConsole) {

scripts/jest/preprocessor.js

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,7 @@ module.exports = {
8686
const plugins = (isTestFile ? testOnlyPlugins : sourceOnlyPlugins).concat(
8787
babelOptions.plugins
8888
);
89-
if (
90-
isTestFile &&
91-
isInDevToolsPackages &&
92-
(process.env.REACT_VERSION ||
93-
filePath.match(/\/transform-react-version-pragma-test/))
94-
) {
89+
if (isTestFile && isInDevToolsPackages) {
9590
plugins.push(pathToTransformReactVersionPragma);
9691
}
9792
let sourceAst = hermesParser.parse(src, {babel: true});

0 commit comments

Comments
 (0)