Skip to content

test(repositories): add unit tests for the notification outbox, delivery log and nip05 verification repositories - #918

Open
Priyanshubhartistm wants to merge 2 commits into
cameri:mainfrom
Priyanshubhartistm:test/repository-specs
Open

Priyanshubhartistm wants to merge 2 commits into
cameri:mainfrom
Priyanshubhartistm:test/repository-specs

Conversation

@Priyanshubhartistm

Copy link
Copy Markdown
Collaborator

Description

Adds unit specs for the three repositories that had none: notification-outbox, notification-delivery-log and nip05-verification. No production code changes.

They run the repositories' real knex queries on the postgres dialect through a small recording client (test/unit/repositories/recording-knex.ts) that never connects: each statement and its bindings are recorded and answered with scripted rows or counts, so the specs assert on the SQL that would run, not on stubbed repository methods.

  • Outbox: enqueue (pending, due now, unattempted), claimBatch (due pending plus processing rows whose 5 minute lease ran out, oldest first, for update skip locked, claimed rows marked processing), markDelivered, markFailed (back to pending with exponential backoff capped at a minute, dead once attempts are used up, error truncated to 2000 characters), deleteTerminalOlderThan (delivered and dead only).
  • Delivery log: append for success, failure and test notifications, findSuccessfulTargetIds (successes only, so a retry skips delivered targets), findRecent (default limit and each filter), deleteOlderThan.
  • NIP-05: findByPubkey, upsert (what is updated on conflict), findPendingVerifications, deleteByPubkey, and listForAdmin (each filter, stale and fresh, the filters combined, count and page filtered alike, pagination).

Related Issue

Closes #905

Motivation and Context

The notification outbox service spec stubs its repository, so the SQL of the repository was never run by a test, and listForAdmin already serves the admin API untested.

claimBatch is asserted on its query (locking and skipping locked rows); that two workers never receive the same message is guaranteed by PostgreSQL, so it is not something a unit test can show.

How Has This Been Tested?

  • The new specs pass (50 tests). Removing skipLocked, changing the backoff cap or the dead threshold, dropping the success filter, the null check of the stale filter or the clone of the count query each make a spec fail.
  • pnpm lint, pnpm check:deps, pnpm run build:check, pnpm run test:unit.

Screenshots (if appropriate):

Types of changes

  • Non-functional change (docs, style, minor refactor)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my code changes.
  • I added a changeset, or this is docs-only and I added an empty changeset.
  • All new and existing tests passed.

…ery log and nip05 verification repositories

Signed-off-by: Priyanshubhartistm <bhartipriyanshustm@gmail.com>
@changeset-bot

changeset-bot Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 229a475

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge, with all three previous findings addressed.

Summary

Adds unit tests for the notification outbox, delivery log, and NIP-05 repositories without changing production code.

  • Notification outbox queries now have tests from queueing through cleanup.
  • Notification delivery logs now have tests for recording and filtering entries.
  • NIP-05 verification queries now have tests for checks and admin lists.

Reviews (2) · Last reviewed commit: "test(repositories): assert the claim con..." · Reviewed by Greptile

Comment thread test/unit/repositories/notification-outbox-repository.spec.ts Outdated
Comment thread test/unit/repositories/recording-knex.ts Outdated
Comment thread test/unit/repositories/nip05-verification-repository.spec.ts Outdated
@coveralls

coveralls commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 39485282477

Coverage increased (+0.7%) to 74.451%

Details

  • Coverage increased (+0.7%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 11072
Covered Lines: 8645
Line Coverage: 78.08%
Relevant Branches: 5786
Covered Branches: 3906
Branch Coverage: 67.51%
Branches in Coverage %: Yes
Coverage Strength: 817.9 hits per line

💛 - Coveralls

…he last_checked_at fallback

Signed-off-by: Priyanshubhartistm <bhartipriyanshustm@gmail.com>

This branch has not been deployed

No deployments
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.

Three repositories have no unit tests (notification-outbox, notification-delivery-log, nip05-verification)

2 participants