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?

[sonarqubecloud]

Quality Gate failed

Failed conditions
4.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

[Copilot]

Pull request overview

This PR adds comprehensive unit, integration, and smoke tests for the notification-kit package while removing previously-bundled concrete repository implementations (InMemoryNotificationRepository and MongooseNotificationRepository) in favor of separate database packages.

Changes:

• Adds extensive test files across the core, infra, and nest layers (service, errors, DTOs, providers, controllers, decorators, module configuration, and integration tests)
• Removes concrete repository implementations and restructures src/infra/repositories/index.ts to export only the Mongoose schema as a reference
• Updates jest.config.ts, tsconfig.json, and package.json/package-lock.json to support the new test infrastructure (coverage thresholds, NestJS testing dependencies, Node type declarations)

Reviewed changes

Copilot reviewed 18 out of 20 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tsconfig.json	Adds "node" to types array to support Node.js globals in tests
jest.config.ts	Adds resetMocks/restoreMocks, expands testMatch to include *.test.ts, adds coverage thresholds and reporters
package.json	Promotes @nestjs/common, @nestjs/core, @nestjs/testing from peer deps to devDependencies
package-lock.json	Lockfile reflecting promoted NestJS packages and related dependency updates
test/smoke.spec.ts	Replaces trivial smoke test with export verification tests for core, infra, and nest layers
test/integration.test.ts	New end-to-end integration test covering the full notification flow (create, send, retry, cancel, deliver, query)
src/core/notification.service.test.ts	New comprehensive unit tests for NotificationService methods
src/core/errors.test.ts	New tests validating all error classes (inheritance, messages, details)
src/core/dtos.test.ts	New tests for all DTO schemas including validation and edge cases
src/nest/module.test.ts	New tests for NotificationKitModule.register() and registerAsync()
src/nest/decorators.test.ts	New tests verifying all DI decorator functions and injection token constants
src/nest/controllers/notification.controller.test.ts	New controller unit tests with mocked service for all CRUD operations
src/nest/controllers/webhook.controller.test.ts	New controller unit tests for webhook processing, auth, and batch operations
src/infra/providers/providers.test.ts	New tests for UUID generator, ObjectId generator, datetime provider, template engine, and event emitters
src/infra/repositories/in-memory/in-memory.repository.ts	Removes in-memory repository implementation
src/infra/repositories/mongoose/mongoose.repository.ts	Removes Mongoose repository implementation
src/infra/repositories/index.ts	Updates to export only Mongoose schema (not implementations)
src/infra/repositories/mongoose/notification.schema.ts	Removes stale @ts-expect-error comment
src/infra/index.ts	Updates comment to reflect removed repository implementations
src/infra/README.md	Updates documentation for the new separate-package repository pattern

[Copilot]

The test "should have proper type definitions" contains only expect(true).toBe(true), which is a no-op assertion that doesn't actually test anything meaningful. While the comment says "This test ensures TypeScript compilation works correctly", TypeScript compilation is verified by the build step, not by running Jest. This test provides false confidence and should either be given a real assertion or removed.

Suggested change

	describe("TypeScript Types", () => {
	it("should have proper type definitions", () => {
	// This test ensures TypeScript compilation works correctly
	expect(true).toBe(true);
	});
	});

[Copilot]

The MockRepository.findReadyToSend() in this file does not accept the _limit parameter required by the INotificationRepository interface (which declares findReadyToSend(_limit: number): Promise<Notification[]>). In TypeScript, implementing an interface method with fewer parameters than the interface specifies is technically allowed (functions can ignore extra arguments), but it means the mock doesn't correctly model the real interface's contract. The same issue is present in src/nest/module.test.ts line 52 and test/integration.test.ts line 67.

Suggested change

	async findReadyToSend(): Promise<Notification[]> {
	const now = new Date().toISOString();
	return Array.from(this.notifications.values()).filter(
	(n) => n.status === NotificationStatus.PENDING && n.scheduledFor && n.scheduledFor <= now,
	);
	async findReadyToSend(_limit: number): Promise<Notification[]> {
	const now = new Date().toISOString();
	const ready = Array.from(this.notifications.values()).filter(
	(n) => n.status === NotificationStatus.PENDING && n.scheduledFor && n.scheduledFor <= now,
	);
	if (Number.isFinite(_limit) && _limit > 0) {
	return ready.slice(0, _limit);
	}
	return ready;

[Copilot]

The "should generate unique IDs" test for ObjectIdGenerator is duplicated at lines 41-50 and 52-61. The second instance is an exact copy of the first with the same test name and logic. This produces a redundant test that adds no additional coverage or validation.

Suggested change

	it("should generate unique IDs", () => {
	const generator = new ObjectIdGenerator();
	const ids = new Set();
	for (let i = 0; i < 100; i++) {
	ids.add(generator.generate());
	}
	expect(ids.size).toBe(100);
	});

[Copilot]

The sentNotifications array is declared at the describe-block scope and never reset between tests (there's no beforeEach to clear it). Since the entire suite uses beforeAll instead of beforeEach, all tests share the same repository and sentNotifications state. This causes test-ordering dependencies: for example, the assertion expect(sentNotifications.length).toBeGreaterThan(0) at line 189 and the assertion expect(lastSent.recipient.phone).toBe("+1234567890") at line 212 rely on the particular test execution order and the cumulative state from prior tests. If test order changes or a prior test fails, these assertions may produce incorrect results.

[Copilot]

The _MockEventEmitter class is defined at line 159 but is never instantiated or used anywhere in the test file. This is dead code that could be safely removed to keep the test file clean.

Suggested change

	class _MockEventEmitter implements INotificationEventEmitter {
	async emit(_event: any): Promise<void> {
	// Event emitted
	}
	}

[Copilot]

The _service variable is set up in beforeEach (line 438/446) but is never used in any of the tests within this describe block. All tests in the "Retry" suite create their own local service instances. The beforeEach and _service variable should either be removed or the tests should be refactored to actually use the shared _service where appropriate.

Suggested change

	let _service: NotificationService;
	beforeEach(() => {
	const sender = new MockSender();
	const repository = new MockRepository();
	const idGenerator = new MockIdGenerator();
	const dateTimeProvider = new MockDateTimeProvider();
	_service = new NotificationService(repository, idGenerator, dateTimeProvider, [sender]);
	});