test(cli): stop checkForUpdates from crashing test workers#1111
Merged
Conversation
`@oclif/core/lib/screen.js` reads `process.stdout.getWindowSize()` once, at module init, but only if `process.stdout.isTTY` is truthy. Vitest's stdout proxy doesn't implement `getWindowSize`, so when a test sets `isTTY = true` before oclif is loaded in that worker (the situation in `checkForUpdates.test.ts`), screen.js crashes the whole worker with `TypeError: stream.getWindowSize is not a function`. It's order-dependent: only the shard where checkForUpdates is the first oclif-touching file flakes, which is why we see different individual tests fail across shards in CI. Install a `getWindowSize` shim once per worker so screen.js can load safely no matter when isTTY gets flipped.
|
Claude finished @gu-stav's task in 1m 8s —— View job ReviewNo blocking issues. The change is small, test-only, and the rationale checks out. Verified:
Changeset: not required — this is a test-only fix with no runtime/public-API impact. Per the repo's changeset policy, that's the correct call (no Minor (optional, non-blocking) observations:
Nothing to fix. |
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs feat/workbench (fcf69a8) |
|---|---|---|
| Internal (raw) | 2.1 KB | -8 B, -0.4% |
| Internal (gzip) | 799 B | -2 B, -0.2% |
| Bundled (raw) | 10.97 MB | -8 B, -0.0% |
| Bundled (gzip) | 2.06 MB | -8 B, -0.0% |
| Import time | 861ms | +22ms, +2.6% |
bin:sanity
| Metric | Value | vs feat/workbench (fcf69a8) |
|---|---|---|
| Internal (raw) | 1023 B | - |
| Internal (gzip) | 486 B | - |
| Bundled (raw) | 9.84 MB | - |
| Bundled (gzip) | 1.77 MB | - |
| Import time | 2.32s | +26ms, +1.2% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @sanity/cli-core
Compared against feat/workbench (fcf69a8c)
| Metric | Value | vs feat/workbench (fcf69a8) |
|---|---|---|
| Internal (raw) | 98.7 KB | - |
| Internal (gzip) | 23.2 KB | - |
| Bundled (raw) | 21.64 MB | - |
| Bundled (gzip) | 3.43 MB | - |
| Import time | 801ms | +11ms, +1.5% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — create-sanity
Compared against feat/workbench (fcf69a8c)
| Metric | Value | vs feat/workbench (fcf69a8) |
|---|---|---|
| Internal (raw) | 908 B | - |
| Internal (gzip) | 483 B | - |
| Bundled (raw) | 931 B | - |
| Bundled (gzip) | 491 B | - |
| Import time | ❌ ChildProcess denied: node | - |
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
Contributor
Coverage Delta
Comparing 40 changed files against main @ Overall Coverage
|
gu-stav
force-pushed
the
fix/stabilize-checkforupdates-test
branch
from
5190c2d to
159d36a
Compare
gu-stav
force-pushed
the
fix/stabilize-checkforupdates-test
branch
from
159d36a to
5190c2d
Compare
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
2 participants
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.
Why
checkForUpdates.test.tsflipsprocess.stdout.isTTY = trueinbeforeEachto simulate an interactive terminal. The first time anything in that worker touches@oclif/coreafter that,@oclif/core/lib/screen.jsruns its module-init code, seesisTTY = true, and callsprocess.stdout.getWindowSize(). Vitest's stdout proxy doesn't implementgetWindowSize, so the whole worker crashes with:It's order-dependent — only the shard where
checkForUpdates.test.tshappens to be the first file to drag in@oclif/coreflakes, which is why different individual tests fail across shards (see the failing jobs on #1110: shard 4/6 on ubuntu, shard 3/4 on Windows).What
Install a
getWindowSizeshim once per worker intest/setup.ts. Now screen.js can load safely whetherisTTYis true or false at the time.Minimal, test-only, no production code touched.
Note
Medium Risk
Touches core
dev/buildorchestration, Vite config generation, and adds a new on-disk dev-server registry/lock mechanism; regressions could affect local development startup/port handling across platforms (notably Windows).Overview
Adds an experimental
federation.enabledCLI config option and wires it throughdevandbuildflows.When federation is enabled,
sanity devnow tries to start a shared workbench server (singleton lock + registry under~/.sanity{-staging}/dev-servers/), bumps the app/studio port, enables cross-server React refresh viareactRefreshHost, and registers running dev servers plus extracted manifests for workbench discovery.Builds gain a federation mode that skips runtime/static asset steps, applies user Vite config to federation builds, and injects the
@sanity/federation/viteplugin; auto-updates import-map generation is disabled for federation builds. Also adds uncached CLI config loading for long-lived processes, centralizes config/data dir resolution, and updates CI snapshot release tagging plus newfederated-studiofixture/test coverage.Reviewed by Cursor Bugbot for commit 159d36a. Bugbot is set up for automated code reviews on this repo. Configure here.