test(workbench): consolidate unit-test mocking into cli-test#1556
test(workbench): consolidate unit-test mocking into cli-test#1556gu-stav wants to merge 1 commit into
Conversation
workbench-cli hand-rolled the same test scaffolding @sanity/cli-test already ships: a mock Output factory, a partial ux mock, and the getGlobalCliClient mock triple. Adopt the shared exports (createMockOutput, mocks/cli-core/ux, mocks/cli-core/apiClient) and wire cli-test into the package so there's a single source of truth. Refs SDK-1886
|
This change is part of the following stack: Change managed by git-spice. |
📦 Bundle Stats —
|
| Metric | Value | vs main (cc06484) |
|---|---|---|
| Internal (raw) | 2.2 KB | - |
| Internal (gzip) | 838 B | - |
| Bundled (raw) | 11.20 MB | - |
| Bundled (gzip) | 2.11 MB | - |
| Import time | 871ms | -1ms, -0.1% |
bin:sanity
| Metric | Value | vs main (cc06484) |
|---|---|---|
| Internal (raw) | 782 B | - |
| Internal (gzip) | 423 B | - |
| Bundled (raw) | 9.90 MB | - |
| Bundled (gzip) | 1.78 MB | - |
| Import time | 2.26s | -41ms, -1.8% |
🗺️ 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 main (cc064844)
| Metric | Value | vs main (cc06484) |
|---|---|---|
| Internal (raw) | 108.6 KB | - |
| Internal (gzip) | 27.1 KB | - |
| Bundled (raw) | 21.76 MB | - |
| Bundled (gzip) | 3.46 MB | - |
| Import time | 775ms | -13ms, -1.7% |
🗺️ 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-build
Compared against main (cc064844)
@sanity/cli-build/_internal/build
| Metric | Value | vs main (cc06484) |
|---|---|---|
| Internal (raw) | 113.0 KB | - |
| Internal (gzip) | 28.4 KB | - |
| Bundled (raw) | 18.10 MB | - |
| Bundled (gzip) | 3.63 MB | - |
| Import time | 1.37s | -4ms, -0.3% |
@sanity/cli-build/_internal/env
| Metric | Value | vs main (cc06484) |
|---|---|---|
| Internal (raw) | 1.8 KB | - |
| Internal (gzip) | 644 B | - |
| Bundled (raw) | 1.31 MB | - |
| Bundled (gzip) | 333.8 KB | - |
| Import time | 127ms | -1ms, -0.5% |
@sanity/cli-build/_internal/extract
| Metric | Value | vs main (cc06484) |
|---|---|---|
| Internal (raw) | 8.6 KB | - |
| Internal (gzip) | 2.7 KB | - |
| Bundled (raw) | 152.8 KB | - |
| Bundled (gzip) | 38.9 KB | - |
| Import time | 248ms | -0ms, -0.0% |
🗺️ ./_internal/env · ./_internal/extract · @sanity/cli-build:./_internal/build treemap too large to embed · 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 main (cc064844)
| Metric | Value | vs main (cc06484) |
|---|---|---|
| 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.
Coverage DeltaNo covered files changed in this PR. Overall Coverage
|
| const mockRequest = vi.fn() | ||
|
|
||
| vi.mock('@sanity/cli-core', async (importOriginal) => ({ | ||
| ...(await importOriginal<typeof import('@sanity/cli-core')>()), |
There was a problem hiding this comment.
Two things:
- Don't import the entirety of
@sanity/cli-corein the source-under-test (deployConfig.ts). If you do, then you are transitively importing JSDOM (a 42MB dependency) and that significantly slows down transpilation, which means tests will be impacted too. Instead, use the granular exports from@sanity/cli-coreto pull in only what the module needs. Looks likedeployConfig.tsonly needsimport {type Output, subdebug} from '@sanity/cli-core', so I would split that up into two import calls:import {type Output} from '@sanity/cli-core/types'andimport {subdebug} from '@sanity/cli-core/debug'. That a) significantly speeds up buildingdeployConfig.tsand b) also means that when testing the module, neither of those core imports need to be mocked since those particular exports and their import graphs are extremely lightweight. - When you do
await importOriginalinvi.mockcalls, you are essentially pulling in the entire module you are trying to mock - which, while convenient, also significantly impacts transpilation time. vitest transpiles tests on demand on every test invocation, meaning all ofsanity/cli-coreis pulled into the tranpilation graph (including 42MB of JSDOM) to run this test. Not good! If you look at the end of the CI runs for theunit tests, there will be an import time breakdown. In CI, a vast majority of time for running tests is spent just processing the import graph and transpiling - on average, only about 10% of time is spent running tests (see e.g. this PR's recent run: 290 seconds importing things, 30 seconds to transpile, and only 25 seconds to run the tests!). Wherever we can, we should use a) the 'thin'cli-coreexports and b) avoid these sorts ofimportOriginalcalls for mocking.
The mocks provided by cli-tests for the cli-core exports are there and easy to use. They are extremely lightweight and provide a mock for every single export in cli-core, so in theory you should never need to importOriginal for any cli-core mocks. See e.g. this test: it vi.mocks cli-core/ux, doesn't need to call importOriginal, and just imports the cli-test mock for all the cli-core/ux APIs. Easy and super fast.
Description
Follow-up from the undeploy stack (SDK-1886), scoped to
@sanity/workbench-cli. The package hand-rolled test scaffolding that@sanity/cli-testalready ships: a mockOutputfactory, a partialuxmock, and thegetGlobalCliClientmock triple duplicated across four files.This adopts the shared exports so there's one source of truth:
createMockOutput(promoted to@sanity/cli-test/test/util),mocks/cli-core/ux, andmocks/cli-core/apiClient. It also wirescli-testinto the package — devDependency, tsconfig paths, and vitesttsconfigPaths.What to review
createMockOutputlands in cli-test (test/util) and replaces theas unknown as Outputcasts.@sanity/cli-coreby spreading the sharedapiClientmock instead of hoisting their owngetGlobalCliClient;deployWorkbenchAppalso swaps its partialuxmock for the shared one.eslint-config-clinow lets__tests__/**import devDependencies — needed bydevTestHelpers.ts, which isn't a.test.tsfile.Testing
Covered by the existing workbench-cli unit suite; all tests pass. Typecheck, lint, format, and knip are clean (the pre-existing
BrettInterfacetype noise onmainis untouched and out of scope).