You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The mockInterface migration (#483, then #487 / #488 / #491) is removing as unknown as jest.Mocked<IPort> casts from spec files, package by package. Those PRs deliberately leave class mocks alone, because they belong to a different utility — createMockInstance — and mixing the two would have made each diff much harder to review.
This issue tracks that second track so it doesn't get lost.
The anti-pattern
Identical in shape to the interface one, but the target is a class:
mockStandardService={getStandardById: jest.fn(),}asunknownasjest.Mocked<StandardService>;// 1 of N methods
The as unknown as double cast switches off the structural check that the spec typecheck pass (tsc --noEmit --project <pkg>/tsconfig.spec.json) relies on, so an object literal implementing one method still claims to be the whole class.
It walks the class prototype at runtime and backs every method with a jest.fn(), so the mock is complete by construction and needs no cast.
Inventory
190 non-interface as unknown as jest.Mocked<…> casts across 47 distinct targets (counted on main at a514337). Roughly 182 are genuine class targets; the other ~8 are TypeORM/DOM types (see "Out of scope" below).
Project
Casts
Spec typecheck enforced?
packages/deployments
65
✅
packages/accounts
42
✅
packages/standards
21
✅
packages/skills
19
✅
apps/cli
11
❌
apps/api
11
❌
packages/commands
8
✅
packages/spaces
7
✅
packages/node-utils
3
✅
packages/coding-agent
2
✅
packages/test-utils
1
✅
Most-mocked targets:
Target
Sites
PackmindEventEmitterService
29
UserService
18
PackageService
14
RenderModeConfigurationService
12
TargetService
9
SkillService
8
PackmindCliHexa
8
StandardService
7
OrganizationService
7
DeploymentsServices
6
The remaining 37 targets have 1–5 sites each.
createMockInstance is currently used in exactly one spec repo-wide (packages/skills/src/application/useCases/uploadSkill/UploadSkillUseCase.spec.ts), so in raw volume this is the larger untapped win of the two tracks.
How we should handle them
Convention
Same one settled in #487 and used since: construct, then stub.
Not the pre-seeded-literal form. Both are type-checked equally; post-construction stubbing is what the per-test overrides (mockResolvedValueOnce, mockRejectedValue) already use, and it keeps evaluation timing identical to the old jest.fn().mockResolvedValue(x).
Per package, the loop that worked for mockInterface
Worth knowing before starting; none of them block the current targets, all of which were checked:
It walks Object.getOwnPropertyNames(cls.prototype) — own properties only, so methods inherited from a base class are not mocked. PackmindEventEmitterService extends BaseService, but BaseService declares only abstract members and a constructor, so it contributes nothing at runtime and this is a non-issue today. It would bite if a base class ever gained a concrete method.
It only mocks members whose descriptor value is a function, so getters and arrow-function class fields (foo = () => {}) are skipped. Checked across the 13 most-mocked targets: all use plain prototype methods.
It takes the class itself as an argument, so the spec must import the real class — fine for these, but it does mean a spec can't mock a class it only knows by type.
If a future target trips one of the first two, the mock will be silently incomplete rather than failing loudly — a strictness option (like mockInterface's { strict: true }) would be the fix.
Out of scope
~8 of the 190 are not classes and should be left as hand-written mocks:
Repository<Distribution>, Repository<TestEntity>, SelectQueryBuilder<…> — TypeORM types. mockInterface's own docstring says a mostly-data type is better mocked by hand, and the same reasoning applies.
Response, and one bare T in a generic helper.
apps/cli and apps/api (22 casts) do not run the spec typecheck pass, so migrating them buys nothing until that's added to their project.json — worth doing, but as its own change.
Context
The
mockInterfacemigration (#483, then #487 / #488 / #491) is removingas unknown as jest.Mocked<IPort>casts from spec files, package by package. Those PRs deliberately leave class mocks alone, because they belong to a different utility —createMockInstance— and mixing the two would have made each diff much harder to review.This issue tracks that second track so it doesn't get lost.
The anti-pattern
Identical in shape to the interface one, but the target is a class:
The
as unknown asdouble cast switches off the structural check that the spec typecheck pass (tsc --noEmit --project <pkg>/tsconfig.spec.json) relies on, so an object literal implementing one method still claims to be the whole class.createMockInstance(packages/test-utils/src/createMockInstance.ts) replaces it:It walks the class prototype at runtime and backs every method with a
jest.fn(), so the mock is complete by construction and needs no cast.Inventory
190 non-interface
as unknown as jest.Mocked<…>casts across 47 distinct targets (counted onmainata514337). Roughly 182 are genuine class targets; the other ~8 are TypeORM/DOM types (see "Out of scope" below).packages/deploymentspackages/accountspackages/standardspackages/skillsapps/cliapps/apipackages/commandspackages/spacespackages/node-utilspackages/coding-agentpackages/test-utilsMost-mocked targets:
PackmindEventEmitterServiceUserServicePackageServiceRenderModeConfigurationServiceTargetServiceSkillServicePackmindCliHexaStandardServiceOrganizationServiceDeploymentsServicesThe remaining 37 targets have 1–5 sites each.
createMockInstanceis currently used in exactly one spec repo-wide (packages/skills/src/application/useCases/uploadSkill/UploadSkillUseCase.spec.ts), so in raw volume this is the larger untapped win of the two tracks.How we should handle them
Convention
Same one settled in #487 and used since: construct, then stub.
Not the pre-seeded-literal form. Both are type-checked equally; post-construction stubbing is what the per-test overrides (
mockResolvedValueOnce,mockRejectedValue) already use, and it keeps evaluation timing identical to the oldjest.fn().mockResolvedValue(x).Per package, the loop that worked for
mockInterfacenx typecheck <pkg>— this is where the value is. Every package migrated so far surfaced real fixture drift the cast had been hiding (♻️ refactor(deployments): migrate port mocks to mockInterface #488 found a stub forIAccountsPort.isMemberOf, a method that exists nowhere in the codebase; ♻️ refactor(standards): migrate port mocks to mockInterface #491 found everyfindMembershipstub instandardswas missing requiredUserSpaceMembershipfields). Expect the same here and budget review time for it.nx test <pkg>andnx lint <pkg>.Suggested order, largest payoff first:
deployments→accounts→standards→skills→commands/spaces/node-utils/coding-agent.Known constraints of
createMockInstanceWorth knowing before starting; none of them block the current targets, all of which were checked:
Object.getOwnPropertyNames(cls.prototype)— own properties only, so methods inherited from a base class are not mocked.PackmindEventEmitterServiceextendsBaseService, butBaseServicedeclares onlyabstractmembers and a constructor, so it contributes nothing at runtime and this is a non-issue today. It would bite if a base class ever gained a concrete method.valueis a function, so getters and arrow-function class fields (foo = () => {}) are skipped. Checked across the 13 most-mocked targets: all use plain prototype methods.If a future target trips one of the first two, the mock will be silently incomplete rather than failing loudly — a strictness option (like
mockInterface's{ strict: true }) would be the fix.Out of scope
~8 of the 190 are not classes and should be left as hand-written mocks:
Repository<Distribution>,Repository<TestEntity>,SelectQueryBuilder<…>— TypeORM types.mockInterface's own docstring says a mostly-data type is better mocked by hand, and the same reasoning applies.Response, and one bareTin a generic helper.apps/cliandapps/api(22 casts) do not run the spec typecheck pass, so migrating them buys nothing until that's added to theirproject.json— worth doing, but as its own change.