Skip to content

test: add scheduler runner coverage - #107

Open
Erliandikasyahputraa wants to merge 1 commit into
Gsync:devfrom
Erliandikasyahputraa:contrib/test-scheduler-runner
Open

test: add scheduler runner coverage#107
Erliandikasyahputraa wants to merge 1 commit into
Gsync:devfrom
Erliandikasyahputraa:contrib/test-scheduler-runner

Conversation

@Erliandikasyahputraa

Copy link
Copy Markdown

Summary

  • add unit coverage for the scheduler runner
  • cover due, skipped, concurrent, successful, and failed automation runs
  • export runDueAutomations so the runner can be tested directly

Testing

  • npm run test
  • npm run lint
  • npx tsc --noEmit
  • npm run build

@Gsync

Gsync commented Aug 23, 2026

Copy link
Copy Markdown
Owner

The export change is fine, but the new tests need tightening before merge. I ran mutations against src/lib/scheduler/index.ts and the suite stayed green through all four of these:

  1. Replacing the AutomationAlreadyRunningError guard with if (false) — deleting the concurrent-run handling entirely. The test at line 86 asserts only expect(runAutomation).toHaveBeenCalled(), which is equally true in the generic-error path (line 108 asserts the same thing), so nothing distinguishes the branch. Seed two due automations and assert toHaveBeenCalledTimes(2) plus no failure row from automationRun.create.

  2. Adding throw error to the inner catch, so one failing automation aborts the whole batch. The test at line 100 is named "…and continuing", but the fixture has a single automation, so the continuing half never runs — the outer try/catch swallows the throw and the assertion still holds. Two automations with the first rejecting, then assert the second still ran.

  3. Setting userId: "wrong-user" and resumeId: undefined in the payload mapping (lines 64/70). The test at line 97 never checks what runAutomation was called with. This is the one place the scheduler hands a userId to the scraper, so a cross-user mix-up would ship silently. Add expect(runAutomation).toHaveBeenCalledWith(expect.objectContaining({ id, userId, resumeId, jobBoard, matchThreshold })) with those fields on the fixture.

  4. Deleting the where: { status: "active", nextRunAt: { lte: now } } clause, so every automation — paused ones included — runs on every tick. The first test is named "returns early if no automations are due" but never pins down what "due" means. scheduler-reaper.spec.ts:33-37 has the pattern for inspecting call args; assert where.status === "active" and that where.nextRunAt.lte is a Date.

Also worth dropping the // @ts-ignore on line 82 — it isn't needed (npx tsc --noEmit exits 0 without it) and would hide a future signature change to AutomationAlreadyRunningError or runAutomation.

Happy to merge once these are in.


Reviewed by Claude Code (Opus 5)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants