IRL-26: Vitest unit tests + GitHub Actions CI#35
Conversation
Adds vitest as a devDependency with `npm test` / `npm run test:watch` scripts, and a CI workflow that runs lint + tests + build on every PR and push to main. Initial test coverage targets the highest-value pure-logic surfaces: - src/lib/dates.ts (15 tests) — UTC anchoring, DST transitions, round-trip with formatCalendarDateForInput, addCalendarDays/Months across DST, and the Math.round-vs-floor case in daysUntilCalendarDate that would otherwise collapse a 23-hour DST diff to 0 days. - src/lib/url.ts (7 tests) — http/https allowlist, javascript:/data:/file: rejection, bare-hostname https:// prepending. - src/lib/tasks.ts (8 tests) — assignee/priority/bucket/recurrence normalizers, including the legacy "WIFE" → "SHARED" fallback. CI uses a throwaway SQLite db (file:./prisma/ci.db) and runs `prisma migrate deploy` before the build — IRL-13 (build broken from unapplied migrations) would have been caught the moment it was pushed with this in place. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a Vitest-based unit test suite and a GitHub Actions CI workflow to continuously validate core “pure logic” helpers and prevent regressions (dates/DST, URL sanitization, and task metadata normalization).
Changes:
- Introduces Vitest (
npm test,npm run test:watch) and adds unit tests fordates,url, andtaskshelpers. - Adds a GitHub Actions CI workflow running migrations, lint, tests, and
next buildon PRs and pushes tomain. - Updates lockfile to include Vitest/Vite transitive dependencies.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/dates.test.ts |
Adds unit tests covering UTC-noon anchoring, DST behavior, and date math helpers. |
src/lib/url.test.ts |
Adds unit tests for http/https allowlisting and rejecting dangerous schemes. |
src/lib/tasks.test.ts |
Adds unit tests for normalizers (assignee/priority/bucket/recurrence). |
package.json |
Adds Vitest scripts and devDependency. |
package-lock.json |
Locks Vitest/Vite and new transitive dependencies. |
.github/workflows/ci.yml |
Adds CI pipeline for lint/test/build with SQLite + Prisma migrations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it("rounds across DST so a 1-day diff doesn't collapse to 0", () => { | ||
| // March 7 → March 8 spans the US DST transition. Naive floor-division of | ||
| // millisecond diffs would give 0 for the +23h gap. Math.round avoids this. | ||
| const before = new Date("2026-03-07T15:00:00"); | ||
| const after = parseCalendarDate("2026-03-08")!; | ||
| expect(daysUntilCalendarDate(after, before)).toBe(1); | ||
| }); |
There was a problem hiding this comment.
The DST-specific assertion here won’t actually validate the DST regression when the test suite runs in a non-DST timezone (e.g. GitHub Actions defaults to UTC). In UTC, a midnight→midnight diff is 24h so Math.floor(...) would still return 1, meaning this test can pass even if the DST bug reappears. Consider forcing the test runner timezone to a DST-observing zone (e.g. America/Los_Angeles) for this suite, or otherwise ensuring the environment makes the March 7→8 gap 23h so the test truly distinguishes round vs floor.
| # Use a throwaway SQLite file. The build prerenders / which queries the DB, | ||
| # so we need a real (empty) database — not just a generated Prisma client. | ||
| env: | ||
| DATABASE_URL: file:./prisma/ci.db |
There was a problem hiding this comment.
CI doesn’t set a timezone, and GitHub Actions runners typically run with TZ=UTC. Because daysUntilCalendarDate() intentionally compares local midnights, the DST regression test in dates.test.ts won’t exercise a DST boundary unless the job runs in a DST-observing timezone. Consider adding TZ (e.g. America/Los_Angeles) to the job env so the DST-focused unit test is meaningful and deterministic.
| DATABASE_URL: file:./prisma/ci.db | |
| DATABASE_URL: file:./prisma/ci.db | |
| TZ: America/Los_Angeles |
| "devDependencies": { | ||
| "@types/node": "^20", | ||
| "@types/react": "^19", | ||
| "@types/react-dom": "^19", | ||
| "eslint": "^9", | ||
| "eslint-config-next": "16.2.1", | ||
| "prisma": "5.22.0", | ||
| "typescript": "^5" | ||
| "typescript": "^5", | ||
| "vitest": "^4.1.4" |
There was a problem hiding this comment.
Adding Vitest pulls in Vite, which in this lockfile declares a Node engine requirement of ^20.19.0 || >=22.12.0. Since the repo currently has no engines field, developers on Node 20.9–20.18 (which still satisfies Next’s >=20.9.0) may hit runtime/tooling issues when running npm test. Consider adding an explicit engines.node constraint in package.json (or otherwise documenting/pinning the supported Node version) to match the stricter requirement introduced by the new test tooling.
ec6097a
into
alecvdpoel/irl-correctness-bug-bundle
Summary
Resolves IRL-26.
npm test/npm run test:watchscripts..github/workflows/ci.ymlrunning lint + tests + build on every PR and push to main.src/lib/dates.ts(15) — UTC anchoring, DST transitions (the IRL-22 regression case),addCalendarDays/Monthsround-trip, theMath.round-vs-floorcollapse that DST would otherwise trigger.src/lib/url.ts(7) — http/https allowlist,javascript:/data:/file:rejection, bare-hostnamehttps://prepending.src/lib/tasks.ts(8) — assignee/priority/bucket/recurrence normalizers, including the legacy"WIFE" → "SHARED"fallback.The CI job uses a throwaway SQLite db (
file:./prisma/ci.db) and runsprisma migrate deploybefore the build — IRL-13 (build broken from unapplied migrations) would have been caught the moment it was pushed with this in place.Note
Heads-up — out of scope
npm auditflags Next 16.2.1 → high-severity DoS (GHSA-q4gf-8mx6-v5v3), fix is bumping to 16.2.4. Not introduced by this PR but worth a separate ticket.Test plan
npm testlocally showsTests 31 passednpm run test:watchworks during development🤖 Generated with Claude Code