Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Jest cache for transform-react-version-pragma #25712

Merged
merged 1 commit into from
Nov 28, 2022

Conversation

tyao1
Copy link
Contributor

@tyao1 tyao1 commented Nov 19, 2022

Summary

Jest caching wasn't working correctly for transform-react-version-pragma. One condition for including transform-react-version-pragma is that process.env.REACT_VERSION is set, but it wasn't included in the cache key computation. Thus local test runs would only run without transform-react-version-pragma, if jest runs weren't using the -reactVersion flag and then added it.

Inlined the scripts/jest/devtools/preprocessor.js file, because it makes it more obvious that process.env.REACT_VERSION is used in scripts/jest/preprocessor.js

How did you test this change?

Repro step:

  • Clear jest cache
  • node ./scripts/jest/jest-cli.js --build --project devtools --release-channel=experimental --reactVersion 18.0
  • node ./scripts/jest/jest-cli.js --build --project devtools --release-channel=experimental

Before:
Jest cached the first run with REACT_VERSION set, so in the second run transform-react-version-pragma is still there and runs only the regression tests on old react versions.

After:

  • The second run runs all tests and ignore // @reactVersion as expected.

@tyao1 tyao1 requested a review from lunaruan November 19, 2022 02:35
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Nov 19, 2022
@sizebot
Copy link

sizebot commented Nov 19, 2022

Comparing: c08d8b8...8b4a6ba

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 154.33 kB 154.33 kB = 48.98 kB 48.98 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 156.25 kB 156.25 kB = 49.60 kB 49.60 kB
facebook-www/ReactDOM-prod.classic.js = 533.14 kB 533.14 kB = 94.42 kB 94.42 kB
facebook-www/ReactDOM-prod.modern.js = 518.24 kB 518.24 kB = 92.24 kB 92.24 kB
facebook-www/ReactDOMForked-prod.classic.js = 533.14 kB 533.14 kB = 94.42 kB 94.42 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 8b4a6ba

@tyao1 tyao1 merged commit edbfc63 into main Nov 28, 2022
@tyao1 tyao1 deleted the ty-fix-jest-cache-for-react-version-pragma branch December 13, 2022 02:27
hoxyq added a commit that referenced this pull request Jun 22, 2023
…#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:/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:/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:/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:/facebook/react/assets/28902667/a702c27e-4c35-4b12-834c-e5bb06728997">
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…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:/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:/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:/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:/facebook/react/assets/28902667/a702c27e-4c35-4b12-834c-e5bb06728997">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants