test: add unit tests for invoice and user repositories#526
test: add unit tests for invoice and user repositories#526CKodidela wants to merge 3 commits intocameri:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves unit test coverage for the data-access layer by adding repository specs for InvoiceRepository and UserRepository, and updates the unit test runner configuration to be Windows-compatible by moving the spec glob into Mocha’s config.
Changes:
- Add unit tests for
InvoiceRepositoryandUserRepositoryusing stubbed DB clients (no real DB connections). - Move the Mocha spec glob to
.mocharc.jsand simplifynpm run test:unittomochafor cross-platform compatibility.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
test/unit/repositories/invoice-repository.spec.ts |
Adds unit tests covering InvoiceRepository public methods and mapping behavior. |
test/unit/repositories/user-repository.spec.ts |
Adds unit tests covering UserRepository public methods and vanish/balance/admission paths. |
package.json |
Simplifies test:unit script to rely on Mocha config for spec discovery (Windows fix). |
.mocharc.js |
Adds spec glob so Mocha handles test file expansion consistently across shells/OSes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('generates a UUID when invoice has no id', () => { | ||
| const invoice = makeInvoice() | ||
| delete (invoice as any).id | ||
|
|
||
| const sql = repository.upsert(invoice).toString() |
There was a problem hiding this comment.
The test deletes invoice.id via delete (invoice as any).id, which bypasses the Invoice type (where id is required) to reach a code path. If UUID generation is intended public behavior, consider making id optional in the input type/signature; otherwise, prefer a dedicated input type or explicit cast at the call site rather than mutating a typed object.
| import { IEventRepository, IUserRepository } from '../../../src/@types/repositories' | ||
| import { UserRepository } from '../../../src/repositories/user-repository' |
There was a problem hiding this comment.
IUserRepository is imported but never used. This will typically fail lint/CI due to unused imports; remove it or use it for the repository variable type if that was the intent.
| unit: InvoiceUnit.MSATS, | ||
| status: InvoiceStatus.PENDING, | ||
| description: 'test invoice', | ||
| confirmed_at: null as any, | ||
| expires_at: null as any, |
There was a problem hiding this comment.
makeDBInvoice uses null as any for nullable-looking DB fields (amount_paid, confirmed_at, expires_at). This weakens type-safety and likely means DBInvoice should model these as nullable (e.g., bigint | null, Date | null) so the test data can avoid any casts.
61560b4 to
57e1aa5
Compare
Description
Adds unit test specs for
InvoiceRepositoryandUserRepository, covering all public methods with a stubbed DB client (no real database connections). Also fixes a pre-existing Windows incompatibility in thetest:unitnpm script where single-quoted globs were passed literally to cmd.exe.New files:
test/unit/repositories/invoice-repository.spec.ts(15 tests)test/unit/repositories/user-repository.spec.ts(16 tests)Modified files:
.mocharc.jsmovedspecglob here so mocha handles expansion cross-platformpackage.jsonsimplifiedtest:unitscript to justmochaRelated Issue
Closes #491
Motivation and Context
These data-access layer components had virtually no test coverage. Poor branch coverage meant conditional paths (found/not-found, error propagation, vanish-state fallback) were completely untested.
How Has This Been Tested?
All tests use sinon sandboxes and a stubbed
DatabaseClient— no real Postgres connection required.Coverage after this PR (measured with
nyc):invoice-repository.tsuser-repository.tsFull suite: 875 passing, 0 failing (
npm run test:unit).Types of changes
Checklist