test: cover branches vitest 5 reports as uncovered (unblocks #91)

[vpetersson-bot]

Unblocks #91 (vitest / @vitest/coverage-v8 3.x → 5.x), which fails CI with:

ERROR: Coverage for branches (72.97%) does not meet global threshold (80%)

Why it fails

Not a regression in the bump — vitest 5 ships a stricter v8 coverage remapper. It counts 37 branches where vitest 3 counted 53, and stops crediting branches that were never actually exercised. The threshold in vitest.config.ts is unchanged; the measurement got honest.

So the uncovered code is genuinely untested, not a reporting artifact. Every path below had no test at all:

Area	Untested path
assign-screen-to-playlist	perform() was never invoked — 0% functions
schedule-playlist-item	missing-API-key guard; the is_new_asset branch that uploads first
complete-workflow	missing-API-key guard; the existing-Zapier-label branch; the duration || 10 fallback
cleanup-zapier-content	the paths where a delete is rejected and the counter must not increment
utils	empty-response guards in createAsset / createPlaylistItem; the 409 already-assigned and non-409 error branches in assignPlaylistToScreen; a failed deleteAsset

What this PR does

Adds 15 tests (one new file, test/assign-screen-to-playlist.test.ts). Tests only — no source or dependency changes.

Verification

Run on Node 22.23.2 against both versions:

	vitest 3.2.7 (current)	vitest 5.0.1 (#91)
before	42 tests… branches 86.79%	branches 72.97% ❌
after	42 passed, 100% all metrics	42 passed, 100% all metrics

npm run lint:check, npm run format:check and npm run build all pass.

Because it is clean on the currently pinned vitest 3 as well, it is safe to merge ahead of the bump — master stays green either way. Once this lands, #91 needs a @dependabot rebase (it is also currently conflicted on package-lock.json after #92 merged) and should go green.

Note, not addressed here

vitest 5 emits a forward-compat warning, harmless today:

ESM syntax in a file loaded as CommonJS (vitest.config.ts:1:1)

Left alone since it is unrelated to the coverage gate — worth a follow-up when configLoader: 'native' becomes Vite's default.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Address the workflow response mock issue and strengthen the requested interceptor assertions.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds branch-focused tests to restore Vitest 5 coverage thresholds without changing production or dependency code.

Changes:

• Covers utility edge cases and failures.
• Adds scheduling, workflow, cleanup, and assignment scenarios.
• Adds tests for previously uncovered branches.

File summaries

File	Summary
test/utils.test.ts	Tests utility edge cases and failures.
test/schedule-playlist-item.test.ts	Covers API-key and new-asset branches.
test/complete-workflow.test.ts	Covers label reuse and duration fallback.
test/cleanup.test.ts	Verifies rejected deletions are not counted.
test/assign-screen-to-playlist.test.ts	Adds assignment action coverage.

Review details

Suppressed comments (2)

test/cleanup.test.ts:189

• The count assertions alone do not prove that the rejected delete requests were attempted: skipping the second playlist deletion and both asset deletion calls would still produce 1 and 0. Capture the delete interceptors and assert each isDone() after the operation so this test actually exercises the failure-return paths it claims to cover.

    expect(response.playlists_removed).toBe(1);
    expect(response.assets_removed).toBe(0);
    expect(response.message).toBe(
      'Successfully removed 1 playlists and 0 assets'
    );

test/complete-workflow.test.ts:300

• This mock returns an object, but createPlaylistItem expects the API response to be an array and reads items[0]. Because the workflow does not inspect the returned item, this test can pass while the helper returns undefined; use the same array-shaped response as the helper's contract.

      .reply(201, { id: 'item-123' });

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Three unresolved review findings identify gaps in proving the intended mocked API requests were executed.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

test/cleanup.test.ts:186

• These count assertions do not prove that the rejected delete requests ran: skipping the asset loop entirely would still produce assets_removed === 0, and stopping after the first playlist would still produce playlists_removed === 1. The test can therefore pass without exercising the failure paths it is intended to cover; assert that all configured Nock interceptors were consumed (or keep handles and assert isDone()).

test/assign-screen-to-playlist.test.ts:46

• This assertion can pass even if the action returns a success object without issuing the assignment request, because the response fields are derived from the input. Assert that the configured interceptor was consumed so this test verifies that the API call actually occurred.

    expect(response.message).toBe('Successfully assigned playlist to screen');

test/assign-screen-to-playlist.test.ts:70

• This test does not prove that the 409 response was consumed. If the action returned the success object without making a request, it would still pass, leaving the already-assigned branch untested. Assert that the interceptor was consumed.

    expect(response.message).toBe('Successfully assigned playlist to screen');

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite