Repository navigation
test(cli): give each test file its own home so renders never wait on another file's lock - #7526
Merged
Merged
Conversation
…another file's lock
Contributor
Contributor
There was a problem hiding this comment.
Approving at ebc0dfc2.
- The root cause matches the evidence.
scripts/test-home.mjsruns once in the config process, so every file in the run shared oneHOMEand onelocks/render.lock. AsetupFilesentry runs before each test file's own imports, so lock and cache paths that are resolved when they are used (hyperframesCacheDir) land in that file's own home. The run-wide home and its XDG/config variable clearing stay in place, and files that share a lock on purpose do it within one file. - The "fails without the fix:" line holds; I confirmed it locally.
gives each test file its own home, so files never share the per-user render lockpasses on this head and fails with thesetupFilesentry removed.testHome.test.ts's home assertions still pass. Its spawn case fails only in my sandbox, which has nobunon PATH, and it is green in CI. - CI is green on this head, and the reachability hash for
vitest.config.tsis updated.
Non-blocking: afterAll removes the per-file home while any detached child a file spawned may still be writing to it. That's harmless with the force: true rm, so it's only worth knowing if a cleanup ENOTEMPTY shows up later.
Merged
3 of 4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Each CLI test file now gets a home folder of its own, so per-user state such as the render lock is never shared by two test files running at the same time.
Why
render command batch options > forwards gif loop and video frame format to batch row rendershas been timing out at 60 s on CI, on main and on unrelated PRs. Its own log says why:scripts/test-home.mjsis imported byvitest.config.tsbefore workers fork, so every test file in a run shares one home folder, and with it one per-user render lock. When astudioServer.test.tsrender holds that lock, the batch test, in another file, waits for it like a real second render would. Whether the two overlap depends on scheduling, which is why it fails on some runs and not others.Related work
Extends
scripts/test-home.mjs(the run-wide temp home, kept as is) with a per-file home set intest.setupFiles. The lock itself (src/utils/renderLock.ts) is unchanged; tests that share a lock on purpose, such asstudioServer.renderLock.test.ts, do it within one file and keep doing so.How
scripts/test-file-home.mjscreates a temp folder, pointsHOMEandUSERPROFILEat it, and removes it after the file's tests.vitest.config.tslists it intest.setupFiles, so the unit, chrome and spawn projects all get it.Test plan
src/utils/testFileHome.test.tsfails with the setup file removed fromvitest.config.tsand passes with it.Waiting for another render to finish (… project /tmp/holder …)and took 17.7 s; with the fix it took 1.9 s and did not wait. A longer hold reaches the 60 s timeout seen on CI.snapshotHdrFrame,diff-offset,media). The same 9 fail with the setup file removed, so they come from that machine's ffmpeg build, not this change; CI runs them on its own ffmpeg. The batch test passed in the full run.No visible change
Test setup only.
Size
A test-setup fix with no runtime code to ride along with: the open renderer PRs touch other packages and are red on this flake, so it goes first and alone.