Run persistent mode tests in CI #15029
Conversation
acdlite
force-pushed
the
run-persistent-tests-in-ci
branch
from
6489ddf
to
42c657c
Compare
Details of bundled changes.Comparing: 3f4852f...2cf0c14 react-native-renderer
react-noop-renderer
react-reconciler
Generated by 🚫 dangerJS |
acdlite
force-pushed
the
run-persistent-tests-in-ci
branch
from
8472cbd
to
b69a2d7
Compare
scripts/jest/setupPersistent.js
Outdated
| @@ -0,0 +1,7 @@ | |||
| 'use strict'; | |||
There was a problem hiding this comment.
Do we need both these identical files? setupPersistent and setupTests.persistent. What's the difference?
There was a problem hiding this comment.
Idk I copy-pasted the other test entry points
There was a problem hiding this comment.
Oh no wait, that's a rename mistake. I'll delete whichever one is superfluous.
package.json
Outdated
| @@ -80,11 +81,11 @@ | |||
| "rollup-plugin-replace": "^2.0.0", | |||
| "rollup-plugin-strip-banner": "^0.2.0", | |||
| "semver": "^5.5.0", | |||
| "shelljs": "^0.8.3", | |||
There was a problem hiding this comment.
#carmackno We need fewer deps. This is unnecessarily complicated and slow.
There was a problem hiding this comment.
Sure, I'll just delete that last commit then
There was a problem hiding this comment.
For context I would have just used exec or whatever but something something Windows contributors whatever I don't care enough it's CI anyway
There was a problem hiding this comment.
This was a bit of a last minute Friday drive-by review. The dependency is just one issue. The other issue is its reliance on file system information in general. We should leave that to jest since it's something that does a lot of complex work around invalidation, file watching, module resolution etc. It also fairly frequently changes.
So without this diff, it just runs more than it needs to on CI wasting cycles but should be equivalent otherwise?
So we can run it in persistent mode, too.
We can't mutate the stateNode in appendAllChildren because the children could be current. This is a bit weird because now the child that we append is different from the one on the fiber stateNode. I think this makes conceptual sense, but I suspect this likely breaks an assumption in Fabric. With this approach, we no longer need to clone to unhide the children, so I removed those host config methods. Fixes bug surfaced by fuzz tester. (The test case that failed was the one that's already hard coded.)
Refs behave differently in persistent mode. I added a TODO to write a persistent mode version of this test.
If a file doesn't reference react-noop-renderer, we shouldn't bother running it in persistent mode, since the results will be identical to the normal test run.
We don't need this now that we have the ability to run any test file in either mutation or persistent mode.
Seb objected to adding shelljs as a dep and I'm too lazy to worry about Windows support so whatever I'll just revert this.
|
I don't think this will affect CI times because we have four parallel instances and I put this task in the one that previously ran the shortest. I was more interested in the time it takes to run |
acdlite
force-pushed
the
run-persistent-tests-in-ci
branch
from
ded6104
to
2cf0c14
Compare
|
Persistent tests run in container 0, which is still not close to being the slowest one. |
Based on #15013
Adds a command to run React Noop tests in persistent mode.
This command will run in CI.
Includes a fix for a bug I found when running the tests.