Feature/comprehensive testing

[y-aithnini]

Summary

• What does this PR change?

Why

• Why is this change needed?

Checklist

• Added/updated tests (if behavior changed)
• npm run lint passes
• npm run typecheck passes
• npm test passes
• npm run build passes
• Added a changeset (npx changeset) if this affects consumers

Notes

• Anything reviewers should pay attention to?

[Copilot]

Pull request overview

Adds a much more comprehensive Jest test suite across core, infra, and NestJS layers, and updates configuration/docs to reflect repository implementations being provided by separate database packages.

Changes:

• Expanded Jest configuration (coverage thresholds/reporters, test matching) and TypeScript config for Node typings.
• Added shared test utilities plus extensive unit/integration tests for core DTOs/errors/service and NestJS module/controllers.
• Removed in-package concrete repository implementations and updated infra exports/docs to reference external DB packages instead.

Reviewed changes

Copilot reviewed 19 out of 21 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tsconfig.json	Adds Node typings to support tests/runtime globals (e.g., require).
test/test-utils.ts	Introduces centralized mocks/factories used across the new test suite.
test/smoke.spec.ts	Replaces placeholder smoke test with export/structure checks.
test/integration.test.ts	Adds end-to-end integration tests for the Nest module + service flow.
src/nest/module.test.ts	Adds unit tests for NotificationKitModule.register/registerAsync.
src/nest/decorators.test.ts	Adds tests for DI decorators and injection tokens.
src/nest/controllers/webhook.controller.test.ts	Adds tests for webhook handling/authorization/retry paths.
src/nest/controllers/notification.controller.test.ts	Adds tests for REST controller behavior and error mapping.
src/infra/repositories/mongoose/notification.schema.ts	Minor typing/comment adjustment around optional Mongoose dependency.
src/infra/repositories/mongoose/mongoose.repository.ts	Removes in-package Mongoose repository implementation (moved to external package).
src/infra/repositories/index.ts	Stops exporting concrete repositories; keeps schema as reference export.
src/infra/repositories/in-memory/in-memory.repository.ts	Removes in-package in-memory repository implementation.
src/infra/providers/providers.test.ts	Adds tests for infra providers (IDs, datetime, templates, event emitters).
src/infra/index.ts	Updates infra layer docs to reflect external repository packages.
src/infra/README.md	Updates docs to point users to external DB packages and schema reference.
src/core/notification.service.test.ts	Adds comprehensive service tests (create/send/retry/cancel/templates/events).
src/core/errors.test.ts	Adds tests for error types, codes/details, and inheritance.
src/core/dtos.test.ts	Adds schema validation tests for DTOs + helper validators.
package.json	Adds NestJS devDependencies required for the new Nest test suite.
jest.config.ts	Expands Jest config: matching, coverage exclusions/thresholds, reporters, worker config.

Comments suppressed due to low confidence (9)

test/test-utils.ts:119

• MockRepository.findReadyToSend() only returns PENDING notifications. In this codebase, create() appears to produce QUEUED notifications (see NotificationService tests), so this mock will incorrectly return an empty set for common “ready to send” scenarios. Adjust the mock behavior to mirror the real repository selection logic (e.g., include QUEUED, scheduled notifications whose scheduledFor is due, and retryable FAILED), and apply limiting if the real port supports it.

  async findReadyToSend(): Promise<Notification[]> {
    return Array.from(this.notifications.values()).filter(
      (n) => n.status === NotificationStatus.PENDING,
    );
  }

test/test-utils.ts:226

• createMockNotification() defaults status to PENDING, but the NotificationService.create() tests expect newly created notifications to be QUEUED. This mismatch makes controller/unit tests easy to write with assumptions that don’t match actual behavior. Consider changing the default to NotificationStatus.QUEUED (or aligning it with the domain’s actual default) and overriding explicitly in the few cases that need PENDING.

export function createMockNotification(overrides: Partial<Notification> = {}): Notification {
  return {
    id: "notif-123",
    channel: NotificationChannel.EMAIL,
    priority: NotificationPriority.NORMAL,
    status: NotificationStatus.PENDING,
    recipient: {
      id: "user-123",

test/integration.test.ts:19

• These integration tests share state across cases via beforeAll (shared Nest module, shared repository, and a shared sentNotifications array). This makes the suite order-dependent and can produce false positives/negatives (e.g., sentNotifications.length > 0 can be satisfied by earlier tests). Prefer resetting shared state in beforeEach/afterEach (e.g., repository.clear() and sentNotifications.length = 0) or constructing an isolated module per test for deterministic behavior.

describe("NotificationKit - Integration Tests", () => {
  let app: any;
  let notificationService: NotificationService;
  let repository: MockRepository;
  const sentNotifications: any[] = [];

test/integration.test.ts:67

• These integration tests share state across cases via beforeAll (shared Nest module, shared repository, and a shared sentNotifications array). This makes the suite order-dependent and can produce false positives/negatives (e.g., sentNotifications.length > 0 can be satisfied by earlier tests). Prefer resetting shared state in beforeEach/afterEach (e.g., repository.clear() and sentNotifications.length = 0) or constructing an isolated module per test for deterministic behavior.

  beforeAll(async () => {
    repository = new MockRepository();
    const senders = [new TestEmailSender(), new TestSmsSender()];

src/nest/module.test.ts:121

• This test case claims to verify the provided ID generator is used, but it explicitly does not assert that (custom-id-123 is never checked). This reduces confidence and can mask regressions. Either assert that the custom generator’s output is actually used (preferred), or rename/re-scope the test to what it truly verifies (e.g., that passing idGenerator is accepted/registered) and add a separate test that proves the generator is applied.

  it("should use provided ID generator", async () => {
    class CustomIdGenerator {
      generate() {
        return "custom-id-123";
      }
    }

    const service = await createModule(
      createModuleTestOptions({ idGenerator: new CustomIdGenerator() }),
    );
    const notification = await service.create(defaultNotificationDto);

    // Just verify notification was created with an ID
    // Note: actual custom ID generator may not be picked up due to DI timing
    expect(notification.id).toBeDefined();
    expect(typeof notification.id).toBe("string");
  });

src/infra/providers/providers.test.ts:62

• The Date.now() delta assertion (< 100ms) is prone to CI flakiness under load. Consider using Jest fake timers or relaxing the threshold to something less timing-sensitive (or only asserting that it parses and is reasonably close within a larger bound).

  it("should return current date as ISO string", () => {
    const provider = new DateTimeProvider();
    const now = provider.now();

    expect(typeof now).toBe("string");
    // Should be a valid ISO date
    expect(() => new Date(now)).not.toThrow();
    expect(Math.abs(new Date(now).getTime() - Date.now())).toBeLessThan(100);
  });

src/infra/providers/providers.test.ts:251

• Directly reassigning console.log without a try/finally can leak the override when an assertion or the awaited call throws, affecting other tests. Prefer jest.spyOn(console, \"log\").mockImplementation(...) and restore it, or wrap the manual override in try/finally to guarantee restoration.

    const originalLog = console.log;
    console.log = (...args: any[]) => {
      logs.push(args);
    };

    await emitter.emit({ type: "notification.sent", notification: {} as any, result: {} as any });

    console.log = originalLog;

    expect(logs.length).toBeGreaterThan(0);

src/core/notification.service.test.ts:38

• The test name says “PENDING status” but the assertion expects NotificationStatus.QUEUED. Rename the test (and any related comments) to match the expected status to avoid confusion when reading failures.

  it("should create a notification with PENDING status", async () => {
    const notification = await service.create(defaultNotificationDto);

    expect(notification.id).toBeDefined();
    expect(notification.status).toBe(NotificationStatus.QUEUED);
    expect(notification.channel).toBe(NotificationChannel.EMAIL);

src/infra/repositories/index.ts:14

• This change removes exports for concrete repositories (previously including Mongoose and in-memory). That’s a breaking API change for consumers importing from @ciscode/notification-kit/infra / infra/repositories. If this package is published, consider providing a deprecation path (re-export stubs that throw with a clear message, or keep exports temporarily), and ensure a semver-major + changeset/changelog entry documents the migration to external DB packages.

// MongoDB/Mongoose schema (reference)
export * from "./mongoose/notification.schema";

[sonarqubecloud]

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud