test: SanityCommand unit testing approach#1335
Conversation
📦 Bundle Stats —
|
| Metric | Value | vs main (944c6ef) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 1.0 KB | - |
| Bundled (raw) | 11.16 MB | - |
| Bundled (gzip) | 2.10 MB | - |
| Import time | 878ms | +11ms, +1.2% |
bin:sanity
| Metric | Value | vs main (944c6ef) |
|---|---|---|
| Internal (raw) | 782 B | - |
| Internal (gzip) | 423 B | - |
| Bundled (raw) | 9.87 MB | - |
| Bundled (gzip) | 1.78 MB | - |
| Import time | 2.02s | +44ms, +2.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 main (944c6ef4)
| Metric | Value | vs main (944c6ef) |
|---|---|---|
| Internal (raw) | 106.7 KB | - |
| Internal (gzip) | 26.7 KB | - |
| Bundled (raw) | 21.72 MB | - |
| Bundled (gzip) | 3.46 MB | - |
| Import time | 779ms | -4ms, -0.6% |
🗺️ 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 main (944c6ef4)
| Metric | Value | vs main (944c6ef) |
|---|---|---|
| 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 Delta
Comparing 2 changed files against main @ Overall Coverage
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1c9f7cc. Configure here.
| public async init(): Promise<void> { | ||
| const {args, flags} = await this.parse({ | ||
| args: this.ctor.args, | ||
| baseFlags: super.ctor.baseFlags, |
There was a problem hiding this comment.
Mock init wrong baseFlags
Low Severity
MockedSanityCommand.init passes super.ctor.baseFlags into parse, while SanityCommand.init uses (super.ctor as typeof SanityCommand).baseFlags. In-memory command tests can parse a different flag set than production once shared base flags exist on SanityCommand.
Reviewed by Cursor Bugbot for commit 1c9f7cc. Configure here.
There was a problem hiding this comment.
The difference pointed out here is purely type-specific, should not affect runtime.
…tor test to a unit test
…mplement forcing divergences between mock and real implementation to be raised as type error
2 participants
Description
Draft / Proof-of-concept on how to unit test
SanityCommandclasses. WIP. Addresses RUN-1420.What to review
This PR is meant as a conversation starting point. Unit testing OCLIF commands can be tricky given their object-oriented nature; testing class instance side effects can be hard, especially when using inheritance (as OCLIF recommends via its custom base class guide). OCLIF's recommended testing approach, IMO, encourages people down an integration-test heavy approach as its native test tooling shells out to a process and captures output streams in order to assert on behaviour.
The changes in this PR implement a mock-based approach: it exposes a
MockSanityCommandclass, along with a factory, that installs mocks forSanityCommandAPIs, and returns them along with the mocked class. Then it is up to each unit test implementor to install the mockedSanityCommandclass in place of the realSanityCommandclass. At this point, a unit test file can exercise aSanityCommandfully in memory using standardvitest(or any other test runner) mocking tools. This helps us keep tests fast and thus iteration cycles short.One benefit to this approach is that we can test things like flag combinations without needing to touch the filesystem. Many of our slowest tests require complicated test fixtures (studio projects) in order to validate superficial and simple logical branches like flag validation.
One downside to this approach, typical for any testing approach employing mocks, is that as the interfaces for a module evolve, so must the mocks that pair with them. However, I don't think this is a major problem in practice, as the
SanityCommandclass changes infrequently (the last change was 3 months ago).Testing
This PR attempts to use two commands to prove its viability: the
schema extractanddoctorcommands:Fixes RUN-1420.
Note
Low Risk
Test infrastructure and visibility tweaks on the CLI base class; doctor behavior should be equivalent aside from routing output through the same bound helpers.
Overview
Introduces a mock-based pattern for fast in-memory OCLIF command tests:
SanityCommandInterface, a public surface onSanityCommand(output,getCliConfig,getProjectId,getProjectRoot,isUnattended,resolveIsInteractive), andcreateMockSanityCommand()so tests can swap in aMockedSanityCommandviavi.mock('@sanity/cli-core').doctornow routes console I/O throughthis.outputinstead ofthis.log/this.error, with unit tests covering human output and--json.schema extractgets similar unit tests (default vs--watch, invalid flags). The integrationdoctorsuite drops the redundant--jsoncase now covered by units.Reviewed by Cursor Bugbot for commit 4a695f0. Bugbot is set up for automated code reviews on this repo. Configure here.