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

Gate DevTools test to fix CI #26742

Merged
merged 1 commit into from
Apr 29, 2023
Merged

Gate DevTools test to fix CI #26742

merged 1 commit into from
Apr 29, 2023

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Apr 28, 2023

This test started failing recently in older versions of React because the Scheduler priority inside a microtask is Normal instead of Immediate. This is expected because microtasks are not Scheduler tasks; it's an implementation detail.

I gated the test to only run in v17 because it's a regression test for legacy Suspense behavior, and the implementation details of the snapshot changed in v18.

Test plan

Using latest:

yarn test --build --project devtools --release-channel=experimental profilingcache

Using v17 (typically runs in a timed CI workflow):

/scripts/circleci/download_devtools_regression_build.js 17.0 --replaceBuild
yarn test --build --project devtools --release-channel=experimental --reactVersion 17.0 profilingcache

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 28, 2023
@acdlite acdlite force-pushed the fix-devtools-ci branch 4 times, most recently from ff31f63 to bc55d9b Compare April 28, 2023 21:33
@react-sizebot
Copy link

react-sizebot commented Apr 28, 2023

Comparing: 18282f8...79793dd

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.04 kB 164.04 kB = 51.72 kB 51.72 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 170.34 kB 170.34 kB = 53.63 kB 53.63 kB
facebook-www/ReactDOM-prod.classic.js = 567.04 kB 567.04 kB = 100.16 kB 100.16 kB
facebook-www/ReactDOM-prod.modern.js = 550.78 kB 550.78 kB = 97.36 kB 97.36 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 79793dd

@acdlite
Copy link
Collaborator Author

acdlite commented Apr 28, 2023

I'm realizing this test was written to cover a behavior that only exists in older versions of React, but even before this PR, it was gated to only run in newer versions of React.

So it was already effectively disabled.

A better fix would be to gate it to only run in older versions of React.

@acdlite acdlite marked this pull request as draft April 28, 2023 21:49
This test started failing recently in older versions of React because
the Scheduler priority inside a microtask is Normal instead of
Immediate. This is expected because microtasks are not Scheduler tasks;
it's an implementation detail.

I gated the test to only run in v17 because it's a regression test for
legacy Suspense behavior, and the implementation details of the snapshot
changed in v18.

Test plan
---------

Using latest:

```
yarn test --build --project devtools --release-channel=experimental profilingcache
```

Using v17 (typically runs in a timed CI workflow):

```
/scripts/circleci/download_devtools_regression_build.js 17.0 --replaceBuild
yarn test --build --project devtools --release-channel=experimental --reactVersion 17.0 profilingcache
```
@acdlite acdlite marked this pull request as ready for review April 29, 2023 14:59
@acdlite acdlite merged commit 86b0e91 into facebook:main Apr 29, 2023
acdlite added a commit to acdlite/next.js that referenced this pull request May 3, 2023
Includes the following upstream changes:

- [5dd90c562](https://github.com/facebook/react/commits/5dd90c562) Use content hash for react-native builds ([vercel#26734](facebook/react#26734)) (Samuel Susla)
- [559e83aeb](https://github.com/facebook/react/commits/559e83aeb) [Fizz] Allow an action provide a custom set of props to use for progressive enhancement ([vercel#26749](facebook/react#26749)) (Sebastian Markbåge)
- [67f4fb021](https://github.com/facebook/react/commits/67f4fb021) Allow forms to skip hydration of hidden inputs ([vercel#26735](facebook/react#26735)) (Sebastian Markbåge)
- [8ea96ef84](https://github.com/facebook/react/commits/8ea96ef84) [Fizz] Encode external fizz runtime into chunks eagerly ([vercel#26752](facebook/react#26752)) (Josh Story)
- [491aec5d6](https://github.com/facebook/react/commits/491aec5d6) Implement experimental_useOptimisticState ([vercel#26740](facebook/react#26740)) (Andrew Clark)
- [9545e4810](https://github.com/facebook/react/commits/9545e4810) Add nonce support to bootstrap scripts and external runtime ([vercel#26738](facebook/react#26738)) (Dan Ott)
- [86b0e9199](https://github.com/facebook/react/commits/86b0e9199) Gate DevTools test to fix CI ([#26742](facebook/react#26742)) (Andrew Clark)
- [b12bea62d](https://github.com/facebook/react/commits/b12bea62d) Preinits should support a nonce option ([#26744](facebook/react#26744)) (Josh Story)
- [efbd68511](https://github.com/facebook/react/commits/efbd68511) Remove unused `initialStatus` parameter from `useHostTransitionStatus` ([vercel#26743](facebook/react#26743)) (Sebastian Silbermann)
- [18282f881](https://github.com/facebook/react/commits/18282f881) Fix: Update while suspended fails to interrupt ([vercel#26739](facebook/react#26739)) (Andrew Clark)
- [540bab085](https://github.com/facebook/react/commits/540bab085) Implement experimental_useFormStatus ([#26722](facebook/react#26722)) (Andrew Clark)
acdlite added a commit to acdlite/next.js that referenced this pull request May 3, 2023
Includes the following upstream changes:

- [5dd90c562](https://github.com/facebook/react/commits/5dd90c562) Use content hash for react-native builds ([vercel#26734](facebook/react#26734)) (Samuel Susla)
- [559e83aeb](https://github.com/facebook/react/commits/559e83aeb) [Fizz] Allow an action provide a custom set of props to use for progressive enhancement ([vercel#26749](facebook/react#26749)) (Sebastian Markbåge)
- [67f4fb021](https://github.com/facebook/react/commits/67f4fb021) Allow forms to skip hydration of hidden inputs ([vercel#26735](facebook/react#26735)) (Sebastian Markbåge)
- [8ea96ef84](https://github.com/facebook/react/commits/8ea96ef84) [Fizz] Encode external fizz runtime into chunks eagerly ([vercel#26752](facebook/react#26752)) (Josh Story)
- [491aec5d6](https://github.com/facebook/react/commits/491aec5d6) Implement experimental_useOptimisticState ([vercel#26740](facebook/react#26740)) (Andrew Clark)
- [9545e4810](https://github.com/facebook/react/commits/9545e4810) Add nonce support to bootstrap scripts and external runtime ([vercel#26738](facebook/react#26738)) (Dan Ott)
- [86b0e9199](https://github.com/facebook/react/commits/86b0e9199) Gate DevTools test to fix CI ([#26742](facebook/react#26742)) (Andrew Clark)
- [b12bea62d](https://github.com/facebook/react/commits/b12bea62d) Preinits should support a nonce option ([#26744](facebook/react#26744)) (Josh Story)
- [efbd68511](https://github.com/facebook/react/commits/efbd68511) Remove unused `initialStatus` parameter from `useHostTransitionStatus` ([vercel#26743](facebook/react#26743)) (Sebastian Silbermann)
- [18282f881](https://github.com/facebook/react/commits/18282f881) Fix: Update while suspended fails to interrupt ([vercel#26739](facebook/react#26739)) (Andrew Clark)
- [540bab085](https://github.com/facebook/react/commits/540bab085) Implement experimental_useFormStatus ([#26722](facebook/react#26722)) (Andrew Clark)
ijjk pushed a commit to vercel/next.js that referenced this pull request May 3, 2023
Includes the following upstream changes:

- [b7972822b](https://github.com/facebook/react/commits/b7972822b)
useOptimisticState -> useOptimistic
([#26772](facebook/react#26772)) (Andrew Clark)
- [388686f29](https://github.com/facebook/react/commits/388686f29) Add
"canary" to list of allowed npm dist tags
([#26767](facebook/react#26767)) (Andrew Clark)
- [8a25302c6](https://github.com/facebook/react/commits/8a25302c6)
fix[dynamic-scripts-injection]: unregister content scripts before
registration ([#26765](facebook/react#26765))
(Ruslan Lesiutin)
- [2c2476834](https://github.com/facebook/react/commits/2c2476834)
Rename "next" prerelease channel to "canary"
([#26761](facebook/react#26761)) (Andrew Clark)
- [fa4314841](https://github.com/facebook/react/commits/fa4314841)
Remove deprecated workflow key from Circle config
([#26762](facebook/react#26762)) (Andrew Clark)
- [5dd90c562](https://github.com/facebook/react/commits/5dd90c562) Use
content hash for react-native builds
([#26734](facebook/react#26734)) (Samuel Susla)
- [559e83aeb](https://github.com/facebook/react/commits/559e83aeb)
[Fizz] Allow an action provide a custom set of props to use for
progressive enhancement
([#26749](facebook/react#26749)) (Sebastian
Markbåge)
- [67f4fb021](https://github.com/facebook/react/commits/67f4fb021) Allow
forms to skip hydration of hidden inputs
([#26735](facebook/react#26735)) (Sebastian
Markbåge)
- [8ea96ef84](https://github.com/facebook/react/commits/8ea96ef84)
[Fizz] Encode external fizz runtime into chunks eagerly
([#26752](facebook/react#26752)) (Josh Story)
- [491aec5d6](https://github.com/facebook/react/commits/491aec5d6)
Implement experimental_useOptimisticState
([#26740](facebook/react#26740)) (Andrew Clark)
- [9545e4810](https://github.com/facebook/react/commits/9545e4810) Add
nonce support to bootstrap scripts and external runtime
([#26738](facebook/react#26738)) (Dan Ott)
- [86b0e9199](https://github.com/facebook/react/commits/86b0e9199) Gate
DevTools test to fix CI
([#26742](facebook/react#26742)) (Andrew Clark)
- [b12bea62d](https://github.com/facebook/react/commits/b12bea62d)
Preinits should support a nonce option
([#26744](facebook/react#26744)) (Josh Story)
- [efbd68511](https://github.com/facebook/react/commits/efbd68511)
Remove unused `initialStatus` parameter from `useHostTransitionStatus`
([#26743](facebook/react#26743)) (Sebastian
Silbermann)
- [18282f881](https://github.com/facebook/react/commits/18282f881) Fix:
Update while suspended fails to interrupt
([#26739](facebook/react#26739)) (Andrew Clark)
- [540bab085](https://github.com/facebook/react/commits/540bab085)
Implement experimental_useFormStatus
([#26722](facebook/react#26722)) (Andrew Clark)

---------
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://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">
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
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
This test started failing recently in older versions of React because
the Scheduler priority inside a microtask is Normal instead of
Immediate. This is expected because microtasks are not Scheduler tasks;
it's an implementation detail.

I gated the test to only run in v17 because it's a regression test for
legacy Suspense behavior, and the implementation details of the snapshot
changed in v18.

Test plan
---------

Using latest:

```
yarn test --build --project devtools --release-channel=experimental profilingcache
```

Using v17 (typically runs in a timed CI workflow):

```
/scripts/circleci/download_devtools_regression_build.js 17.0 --replaceBuild
yarn test --build --project devtools --release-channel=experimental --reactVersion 17.0 profilingcache
```
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
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
This test started failing recently in older versions of React because
the Scheduler priority inside a microtask is Normal instead of
Immediate. This is expected because microtasks are not Scheduler tasks;
it's an implementation detail.

I gated the test to only run in v17 because it's a regression test for
legacy Suspense behavior, and the implementation details of the snapshot
changed in v18.

Test plan
---------

Using latest:

```
yarn test --build --project devtools --release-channel=experimental profilingcache
```

Using v17 (typically runs in a timed CI workflow):

```
/scripts/circleci/download_devtools_regression_build.js 17.0 --replaceBuild
yarn test --build --project devtools --release-channel=experimental --reactVersion 17.0 profilingcache
```

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

3 participants