test: add unit tests for MaintenanceWorker and PaymentsService#440
test: add unit tests for MaintenanceWorker and PaymentsService#440Mahmoud-s-Khedr wants to merge 3 commits intocameri:mainfrom
Conversation
| sandbox.restore() | ||
| }) | ||
|
|
||
| // ─── constructor ────────────────────────────────────────────────────────── |
There was a problem hiding this comment.
ok removed the label comments form both files
There was a problem hiding this comment.
Pull request overview
Adds new Mocha/Chai/Sinon unit test suites for two core payment-flow modules to increase branch coverage and lock in expected behavior for invoice polling, confirmation, and notification.
Changes:
- Add unit tests for
PaymentsServicepublic methods and key branches (processor passthrough, transaction lifecycle, confirmation logic, notification pipeline). - Add unit tests for
MaintenanceWorkerscheduling, signal/error handling, invoice polling loop behavior, and per-invoice error isolation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/unit/services/payments-service.spec.ts | New unit tests for PaymentsService, including transaction and notification pipeline behavior |
| test/unit/app/maintenance-worker.spec.ts | New unit tests for MaintenanceWorker interval scheduling, signal handlers, and invoice processing loop |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Validation fires before transaction.begin(); rollback() on an unstarted | ||
| // transaction throws its own error, which is what ultimately propagates. | ||
| await expect( | ||
| service.confirmInvoice(makeCompletedInvoice({ confirmedAt: null })) | ||
| ).to.be.rejectedWith('Unable to get transaction: transaction not started.') | ||
| }) | ||
|
|
||
| it('throws when status is not COMPLETED', async () => { | ||
| await expect( | ||
| service.confirmInvoice(makeCompletedInvoice({ status: InvoiceStatus.PENDING })) | ||
| ).to.be.rejectedWith('Unable to get transaction: transaction not started.') | ||
| }) | ||
|
|
||
| it('throws when amountPaid is not a bigint', async () => { | ||
| await expect( | ||
| service.confirmInvoice(makeCompletedInvoice({ amountPaid: undefined })) | ||
| ).to.be.rejectedWith('Unable to get transaction: transaction not started.') |
There was a problem hiding this comment.
These validation-guard tests assert the method rejects with "Unable to get transaction: transaction not started.", which is coming from Transaction.rollback() being called even when begin() was never reached. This couples the test to an implementation bug and masks the intended validation errors (e.g., "Invoice confirmation date is not set"). Prefer updating PaymentsService.confirmInvoice() to only rollback if the transaction was started (or move begin() before validation), and then assert on the explicit validation error message(s) instead of the transaction error.
| // Validation fires before transaction.begin(); rollback() on an unstarted | |
| // transaction throws its own error, which is what ultimately propagates. | |
| await expect( | |
| service.confirmInvoice(makeCompletedInvoice({ confirmedAt: null })) | |
| ).to.be.rejectedWith('Unable to get transaction: transaction not started.') | |
| }) | |
| it('throws when status is not COMPLETED', async () => { | |
| await expect( | |
| service.confirmInvoice(makeCompletedInvoice({ status: InvoiceStatus.PENDING })) | |
| ).to.be.rejectedWith('Unable to get transaction: transaction not started.') | |
| }) | |
| it('throws when amountPaid is not a bigint', async () => { | |
| await expect( | |
| service.confirmInvoice(makeCompletedInvoice({ amountPaid: undefined })) | |
| ).to.be.rejectedWith('Unable to get transaction: transaction not started.') | |
| await expect( | |
| service.confirmInvoice(makeCompletedInvoice({ confirmedAt: null })) | |
| ).to.be.rejectedWith('Invoice confirmation date is not set') | |
| }) | |
| it('throws when status is not COMPLETED', async () => { | |
| await expect( | |
| service.confirmInvoice(makeCompletedInvoice({ status: InvoiceStatus.PENDING })) | |
| ).to.be.rejectedWith('Invoice status is not COMPLETED') | |
| }) | |
| it('throws when amountPaid is not a bigint', async () => { | |
| await expect( | |
| service.confirmInvoice(makeCompletedInvoice({ amountPaid: undefined })) | |
| ).to.be.rejectedWith('Invoice amount paid is not set') |
| it('throws when status is not COMPLETED', async () => { | ||
| await expect( | ||
| service.confirmInvoice(makeCompletedInvoice({ status: InvoiceStatus.PENDING })) | ||
| ).to.be.rejectedWith('Unable to get transaction: transaction not started.') |
There was a problem hiding this comment.
Same issue as the previous guard: this expectation currently relies on the rollback-before-begin behavior and will fail once confirmInvoice() correctly surfaces its own validation errors (e.g., "Invoice is not complete: ..."). After fixing the service logic, update this test to assert on the explicit validation error message rather than the transaction-not-started message.
| ).to.be.rejectedWith('Unable to get transaction: transaction not started.') | |
| ).to.be.rejectedWith('Invoice is not complete') |
| it('throws when amountPaid is not a bigint', async () => { | ||
| await expect( | ||
| service.confirmInvoice(makeCompletedInvoice({ amountPaid: undefined })) | ||
| ).to.be.rejectedWith('Unable to get transaction: transaction not started.') |
There was a problem hiding this comment.
This guard test is also asserting the transaction-not-started error that results from calling rollback() without a started transaction, instead of the intended validation error ("Invoice paid amount is not set: ..."). Once confirmInvoice() is fixed to not mask validation failures, update the assertion to match the explicit validation error message.
| ).to.be.rejectedWith('Unable to get transaction: transaction not started.') | |
| ).to.be.rejectedWith(/Invoice paid amount is not set:/) |
Description
Add unit tests for
MaintenanceWorkerandPaymentsServicecovering all public methods and key branches.MaintenanceWorker(test/unit/app/maintenance-worker.spec.ts):SIGINT,SIGHUP,SIGTERM,uncaughtException,unhandledRejection)run()— verifies the 60-second interval triggersonScheduleonSchedule()— payments disabled early-return, pending invoice fetching, skip on missing id/status, status update, COMPLETED +confirmedAtconfirm+notify flow, error isolation between invoicesonError()— re-throws uncaught exceptionsonExit()— callscloseand exits with code 0close()— invokes optional callback, does not throw without onePaymentsService(test/unit/services/payments-service.spec.ts):getPendingInvoices— correct offset/limit, error propagationgetInvoiceFromPaymentsProcessor— string ID passthrough, full-object passthrough whenverifyURLpresent, id-only whenverifyURLabsent, error propagationcreateInvoice— transaction lifecycle (upsert user → create invoice → persist → commit), rollback on processor failureupdateInvoice/updateInvoiceStatus— delegation to repository, error propagationconfirmInvoice— validation guards, SATS/BTC/MSATS unit conversion, admission fee logic (amount threshold, disabled schedules, whitelisted pubkeys), rollback on errorsendInvoiceUpdateNotification— missingamountPaidguard, MSATS→SATS conversion in content, event persist + broadcast, graceful error logging viaotherwise()Related Issue
#424
Motivation and Context
MaintenanceWorkerandPaymentsServiceare core to the payment flow but had no unit test coverage. These tests document expected behavior, guard against regressions, and make future refactoring safer.How Has This Been Tested?
Tests run with the project's existing Mocha + Chai + Sinon setup (
npm test). All dependencies are stubbed — no real database, network, or payment processor calls are made.Types of changes
Checklist: