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[ci]: fixed jest configuration not to skip too many devtools tests #26955

Merged

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Jun 15, 2023

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:

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

@hoxyq hoxyq requested review from acdlite and tyao1 June 15, 2023 11:46
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 15, 2023
@react-sizebot
Copy link

react-sizebot commented Jun 15, 2023

Comparing: 4ddc019...8921349

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 = 164.23 kB 164.23 kB = 51.73 kB 51.73 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 171.67 kB 171.67 kB = 53.97 kB 53.97 kB
facebook-www/ReactDOM-prod.classic.js = 570.12 kB 570.12 kB = 100.58 kB 100.58 kB
facebook-www/ReactDOM-prod.modern.js = 553.90 kB 553.90 kB = 97.75 kB 97.75 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 8921349

@hoxyq hoxyq force-pushed the devtools/fix-jest-skipping-devtools-tests branch from c0abd0d to 8921349 Compare June 15, 2023 11:56
@hoxyq hoxyq marked this pull request as ready for review June 15, 2023 12:05
Copy link
Contributor

@tyao1 tyao1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noice!

@hoxyq hoxyq merged commit c8deb5d into facebook:main Jun 22, 2023
@hoxyq hoxyq deleted the devtools/fix-jest-skipping-devtools-tests branch June 22, 2023 08:33
hoxyq added a commit that referenced this pull request Jun 26, 2023
…est (#27008)

- Correctly gate the test to `[18.0.0, 18.2.0]` versions of react, as it
was initially defined before
#26742
- I have recently fixed the gating logic in
#26955 and
#26997, should be stable now
- I have added a non-gated version of this test, which should run only
for the current version of react, the one we build from sources
- The test version for react `v[18.0.0, 18.2.0]` should expect
`"priorityLevel": "Immediate"`, the other `"priorityLevel": "Normal"`,
as it was changed in #26512
hoxyq added a commit that referenced this pull request Jul 4, 2023
List of changes:
* Devtools:-Removed unused CSS ([Biki-das](https://github.com/Biki-das)
in [#27032](#27032))
* fix[devtools/profilingCache-test]: specify correct version gate for
test ([hoxyq](https://github.com/hoxyq) in
[#27008](#27008))
* fix[devtools/ci]: fixed incorrect condition calculation for
@reactVersion annotation ([hoxyq](https://github.com/hoxyq) in
[#26997](#26997))
* fix[ci]: fixed jest configuration not to skip too many devtools tests
([hoxyq](https://github.com/hoxyq) in
[#26955](#26955))
* fix[devtools/standalone]: update webpack configurations
([hoxyq](https://github.com/hoxyq) in
[#26963](#26963))
* fix[devtools]: check if fiber is unmounted before trying to highlight
([hoxyq](https://github.com/hoxyq) in
[#26983](#26983))
* feat[devtools]: support x_google_ignoreList source maps extension
([hoxyq](https://github.com/hoxyq) in
[#26951](#26951))
* chore[devtools]: upgrade to webpack v5
([hoxyq](https://github.com/hoxyq) in
[#26887](#26887))
* fix[devtools]: display NaN as string in values
([hoxyq](https://github.com/hoxyq) in
[#26947](#26947))
* fix: devtools cannot be closed correctly
([Jack-Works](https://github.com/Jack-Works) in
[#25510](#25510))
* Fix:- Fixed dev tools inspect mode on Shadow dom
([Biki-das](https://github.com/Biki-das) in
[#26888](#26888))
* Updated copyright text to Copyright (c) Meta Platforms, Inc. and its …
([denmo530](https://github.com/denmo530) in
[#26830](#26830))
* Fix strict mode badge URL ([ibrahemid](https://github.com/ibrahemid)
in [#26825](#26825))
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://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">
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…est (facebook#27008)

- Correctly gate the test to `[18.0.0, 18.2.0]` versions of react, as it
was initially defined before
facebook#26742
- I have recently fixed the gating logic in
facebook#26955 and
facebook#26997, should be stable now
- I have added a non-gated version of this test, which should run only
for the current version of react, the one we build from sources
- The test version for react `v[18.0.0, 18.2.0]` should expect
`"priorityLevel": "Immediate"`, the other `"priorityLevel": "Normal"`,
as it was changed in facebook#26512
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
List of changes:
* Devtools:-Removed unused CSS ([Biki-das](https://github.com/Biki-das)
in [facebook#27032](facebook#27032))
* fix[devtools/profilingCache-test]: specify correct version gate for
test ([hoxyq](https://github.com/hoxyq) in
[facebook#27008](facebook#27008))
* fix[devtools/ci]: fixed incorrect condition calculation for
@reactVersion annotation ([hoxyq](https://github.com/hoxyq) in
[facebook#26997](facebook#26997))
* fix[ci]: fixed jest configuration not to skip too many devtools tests
([hoxyq](https://github.com/hoxyq) in
[facebook#26955](facebook#26955))
* fix[devtools/standalone]: update webpack configurations
([hoxyq](https://github.com/hoxyq) in
[facebook#26963](facebook#26963))
* fix[devtools]: check if fiber is unmounted before trying to highlight
([hoxyq](https://github.com/hoxyq) in
[facebook#26983](facebook#26983))
* feat[devtools]: support x_google_ignoreList source maps extension
([hoxyq](https://github.com/hoxyq) in
[facebook#26951](facebook#26951))
* chore[devtools]: upgrade to webpack v5
([hoxyq](https://github.com/hoxyq) in
[facebook#26887](facebook#26887))
* fix[devtools]: display NaN as string in values
([hoxyq](https://github.com/hoxyq) in
[facebook#26947](facebook#26947))
* fix: devtools cannot be closed correctly
([Jack-Works](https://github.com/Jack-Works) in
[facebook#25510](facebook#25510))
* Fix:- Fixed dev tools inspect mode on Shadow dom
([Biki-das](https://github.com/Biki-das) in
[facebook#26888](facebook#26888))
* Updated copyright text to Copyright (c) Meta Platforms, Inc. and its …
([denmo530](https://github.com/denmo530) in
[facebook#26830](facebook#26830))
* Fix strict mode badge URL ([ibrahemid](https://github.com/ibrahemid)
in [facebook#26825](facebook#26825))
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
…#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">

DiffTrain build for commit c8deb5d.
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