Skip to content

finding(test-infra): nothing stops the 26th hand-listed vi.mock factory — and the recogniser has to be semantic, not a grep for importOriginal #6849

Description

@claude

Surfaced while implementing #6768 (PR #6847). That card converted the 25 frozen sites; this is the property of the tree that produced them, which the fix deliberately did not touch.

The gap

A vi.mock factory that hand-lists the exports it returns freezes the mock export surface. The next export any widely-imported module reads at module scope then kills that file at COLLECTION: zero failed assertions, the tests never run, and the suite reads like flake. PR #6847 demonstrates the failure and its removal with a two-leg ablation.

Nothing prevents the 26th one. A new test file written with a hand-listed factory reintroduces the defect the same day, and the class reopens silently — the cost is paid later, by whoever adds the next export, in a red suite that does not point at them.

Why a grep will not do it (measured on #6768)

The card was written from a grep for the literal importOriginal and counted 36 sites. The true frozen count is 25. Eleven files already inherit the real surface under a different spelling and need nothing:

  • 9 in plugin-dashboard spread await vi.importActual('@object-ui/react') from a zero-parameter factory.
  • EnvironmentListToolbar.test.tsx names the callback param importActual.
  • PageView.test.tsx names it orig.

So the recogniser has to be semantic — does the factory OBTAIN the real module (its callback param under any name, or vi.importActual of the same specifier) and SPREAD it — not a substring match. A name-based gate would demand edits to 11 already-correct files, and would be deleted by the first person it annoyed.

Shape of a fix (not a decision, for triage)

A gate in the family of the existing scripts/check-vi-mock-specifiers.mjs (same defect direction: a mock that is silently not doing what it looks like it does). It is green at rest today — PR #6847 leaves 0 frozen sites out of 106 — so it would start as a ratchet rather than a cleanup.

Open question for triage: whether to scope it to widely-imported workspace specifiers or to every vi.mock factory. The second is stricter but there are legitimate whole-module replacements (vi.mock('./ObjectCalendar', ...) in plugin-calendar/src/registration.test.tsx is a deliberate one, and its own comment explains why).

Generated by Claude Code


Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpriority:p2teststooling

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions