Sign up
View stargazersSign up

Fix Vitest 5 CI test job regressions#1349

closed5
main
copilot/fix-failing-github-actions-test
openedSep 9, 2026byCCopilotBot
6 files changed
View on GitHubTwo-way sync enabledPR last imported 10d ago

Summary

The failing test job came from a partial Vitest 5 upgrade: coverage still used @vitest/coverage-v8 3.x, and the project config/tests no longer matched Vitest 5's inline-project behavior. This aligns the coverage provider, restores the intended node/jsdom split, and hardens the affected tests so pnpm test:coverage runs cleanly again.

Changes

  • Dependency alignment

    • bump @vitest/coverage-v8 to ^5.0.0 in package.json and refresh pnpm-lock.yaml

  • Vitest project config

    • simplify shared alias setup in vitest.config.ts
    • make node/jsdom project includes and excludes explicit so Vitest 5 inheritance does not leak tests across environments

  • jsdom test harness

    • update tests/components/resizeObserverStub.ts to expose a constructable ResizeObserver stub compatible with Vitest 5

  • Test expectation fixes

    • make tests/extension/manualCaptureConcurrency.test.ts assert relative call growth instead of cross-test global totals
    • extend tests/main/ipcHandlers.test.ts to cover branches now counted by coverage and cast mocked services to satisfy test-project typing

Evidence-affecting

No

Verification

  • pnpm test:coverage
  • pnpm test tests/main/ipcHandlers.test.ts tests/extension/manualCaptureConcurrency.test.ts tests/components/mentionSuggestion.test.tsx tests/components/CapturesRoute.test.tsx
  • secret scan on changed files: clear

Pull request description generated by Claude Code

Checks

2 failing · 10 successful · 2 skipped
Some checks failed

Failing2

3699a44

Added 5 commits

  • test: restore Vitest project split2a390e1
  • test: cast mocked services for typecheck3699a44
  • chore: align Vitest coverage package4040d86
  • test: make extension capture assertion baseline-relative63f012c
  • test: fix ResizeObserver stub for Vitest 57e87e93
CcoderabbitaiBotSep 9, 2026
Synced from GitHub

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Free

Run ID: 65642deb-f799-413f-af46-a29373c46586

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

Cchatgpt-codex-connectorBotreviewed · 1 inline threadSep 9, 2026
Synced from GitHub

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3699a44f5c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

package.jsonline 186Open thread
……@@ -183,7 +183,7 @@
184184 "@types/uuid": "^10.0.0",
185185 "@vitejs/plugin-react": "^4.7.0",
186- "@vitest/coverage-v8": "^3.2.6",
186+ "@vitest/coverage-v8": "^5.0.0",
Cchatgpt-codex-connectorBoton line 186Sep 9, 2026

P1 Badge Keep the coverage stack compatible with Node 20

On the required Node 20 CI and development runtime, this selects @vitest/coverage-v8@5.0.0, whose newly added dependencies in pnpm-lock.yaml:2793-2799 declare Node >=22; the selected Vitest 5 runner likewise declares ^22.12.0 || ^24.0.0 || >=26.0.0 at pnpm-lock.yaml:5948-5950. Consequently, the advertised pnpm test:coverage stack is outside its supported runtime and can fail during installation or startup; align Vitest and its coverage provider on a Node-20-compatible major instead of completing the v5 upgrade.

AGENTS.md reference: AGENTS.md:L28-L28

Useful? React with 👍 / 👎.

MMatt DonovanSep 9, 2026

Defect: this branch still fails pnpm typecheck, so merging it leaves main red.

What: 11 TS2322 errors remain in six files the diff does not touch. Each is the same error this pull request already fixed in tests/main/ipcHandlers.test.ts: under Vitest 5 a bare vi.fn() widens to Mock<Procedure | Constructable>, which no longer satisfies a specific function type such as () => void or (ids: string[]) => void. The as unknown as <Type> cast used in ipcHandlers.test.ts is the same remedy at each site. Nothing on this head has been proven by continuous integration: run 34398560552 is action_required, so typecheck and test both report SKIPPED.

Where: tests/components/BatchTagPopover.test.tsx:44, tests/components/CaptureDetailsPanel.test.tsx:56, :57, :58, :183, :184, :185, tests/components/CaptureListContextMenu.test.tsx:66, tests/components/CaptureListSelection.test.tsx:72, tests/components/StorageConfig.test.tsx:96, tests/components/WaybackPanel.test.tsx:69.

Reproduce: at 3699a44f on Node 20.20.2, in a throwaway worktree:

1pnpm install --frozen-lockfile --ignore-scripts && pnpm typecheck

Exit code 2. Skipping lifecycle scripts does not affect typecheck. I did not run pnpm test, so the @vitest/coverage-v8 half of the fix is unverified. Tracked as #1382.

MMatt DonovanSep 9, 2026

Superseded by PR #1387, which completes this work and verifies green: pnpm typecheck exit 0, pnpm test exit 0 with 4054 passed and 10 skipped, and pnpm preflight exit 0 at head 96f14af0.

Credit where it is due: #1387 takes the tests/components/resizeObserverStub.ts fix from this branch directly, assigning the stub class rather than a vi.fn() wrapping it, and it reaches the same @vitest/coverage-v8 ^5.0.0 alignment you identified.

It diverges on the TS2322 cluster, and the reason is worth recording because this branch's approach would have papered over the real cause. The errors are not a vi.fn() call-site problem. They come from ReturnType<typeof vi.fn> in the declarations: ReturnType of a generic resolves the type parameter to its constraint, and Vitest 5 widened that constraint from Procedure to Procedure | Constructable. So #1387 types the declarations, with MockedObject<RecaptureService> and MockedObject<UpdaterService> in tests/main/ipcHandlers.test.ts and Mock<Sig> carrying each property's own signature at the eleven component sites, and uses no as unknown as cast anywhere. A cast at the assignment site fixes the symptom at one location at a time, which is why this branch cleared ipcHandlers.test.ts and left the eleven component sites failing.

#1387 also carries two Vitest 5 behaviour changes this branch does not: inline projects now inherit the declaring config, which merged the root include and dragged the main-process suites into jsdom, and clearMocks flipped to true, which breaks suites asserting cumulative call counts.

Leaving this open until #1387 merges, so there is still a fix on the board if that one fails review. No action needed here.

MMatt DonovanSep 10, 2026

Closing as superseded by PR #1387, which is now out of draft with all five required contexts green (lint, typecheck, test, build, e2e) and an approve pre-pass verdict at 96f14af0.

The reasoning is in the earlier comment on this pull request. In short, #1387 reaches the same @vitest/coverage-v8 alignment you identified and takes the resizeObserverStub.ts fix from this branch directly, but it types the mock declarations rather than casting at the assignment sites, which is why it clears all thirteen typecheck errors where this branch cleared two.

Nothing here is lost. Reopen if #1387 fails review.