Skip to content

test: add unit tests for MaintenanceWorker and PaymentsService#440

Open
Mahmoud-s-Khedr wants to merge 3 commits intocameri:mainfrom
Mahmoud-s-Khedr:add-tests-001
Open

test: add unit tests for MaintenanceWorker and PaymentsService#440
Mahmoud-s-Khedr wants to merge 3 commits intocameri:mainfrom
Mahmoud-s-Khedr:add-tests-001

Conversation

@Mahmoud-s-Khedr
Copy link
Copy Markdown
Contributor

Description

Add unit tests for MaintenanceWorker and PaymentsService covering all public methods and key branches.

MaintenanceWorker (test/unit/app/maintenance-worker.spec.ts):

  • Constructor signal handler registration (SIGINT, SIGHUP, SIGTERM, uncaughtException, unhandledRejection)
  • run() — verifies the 60-second interval triggers onSchedule
  • onSchedule() — payments disabled early-return, pending invoice fetching, skip on missing id/status, status update, COMPLETED + confirmedAt confirm+notify flow, error isolation between invoices
  • onError() — re-throws uncaught exceptions
  • onExit() — calls close and exits with code 0
  • close() — invokes optional callback, does not throw without one

PaymentsService (test/unit/services/payments-service.spec.ts):

  • getPendingInvoices — correct offset/limit, error propagation
  • getInvoiceFromPaymentsProcessor — string ID passthrough, full-object passthrough when verifyURL present, id-only when verifyURL absent, error propagation
  • createInvoice — transaction lifecycle (upsert user → create invoice → persist → commit), rollback on processor failure
  • updateInvoice / updateInvoiceStatus — delegation to repository, error propagation
  • confirmInvoice — validation guards, SATS/BTC/MSATS unit conversion, admission fee logic (amount threshold, disabled schedules, whitelisted pubkeys), rollback on error
  • sendInvoiceUpdateNotification — missing amountPaid guard, MSATS→SATS conversion in content, event persist + broadcast, graceful error logging via otherwise()

Related Issue

#424

Motivation and Context

MaintenanceWorker and PaymentsService are 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

  • Non-functional change (docs, style, minor refactor)

Checklist:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my code changes.
  • All new and existing tests passed.

sandbox.restore()
})

// ─── constructor ──────────────────────────────────────────────────────────
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove these comments

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok removed the label comments form both files

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 PaymentsService public methods and key branches (processor passthrough, transaction lifecycle, confirmation logic, notification pipeline).
  • Add unit tests for MaintenanceWorker scheduling, 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.

Comment on lines +285 to +301
// 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.')
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
// 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')

Copilot uses AI. Check for mistakes.
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.')
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
).to.be.rejectedWith('Unable to get transaction: transaction not started.')
).to.be.rejectedWith('Invoice is not complete')

Copilot uses AI. Check for mistakes.
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.')
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
).to.be.rejectedWith('Unable to get transaction: transaction not started.')
).to.be.rejectedWith(/Invoice paid amount is not set:/)

Copilot uses AI. Check for mistakes.
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.

3 participants