feature: add tests for unit and other tests packages as well

[Anmol1696]

No description provided.

[Copilot]

Pull request overview

This pull request adds comprehensive test infrastructure for the constructive-functions repository. It introduces Jest as the testing framework with TypeScript support via ts-jest, adds unit tests for all three function handlers (example, simple-email, send-email-link), creates integration tests for the runtime HTTP layer, implements mock helpers and module mocks, and sets up a CI workflow to run tests automatically.

Changes:

• Added Jest testing framework with TypeScript support (ts-jest) and comprehensive test configuration
• Implemented unit tests for all function handlers (example, simple-email, send-email-link) with validation and behavior coverage
• Created integration tests for the fn-runtime HTTP layer with helper utilities for function server management
• Added mock implementations for external dependencies (postmaster, SMTP, logger, env utilities, MJML, GraphQL tag)
• Set up GitHub Actions CI workflow with separate unit and integration test jobs

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
jest.config.ts	Jest configuration with ts-jest preset, test patterns, module mocks, and 30s timeout
package.json	Added test scripts (test, test:unit, test:integration) and Jest/ts-jest dependencies
.github/workflows/test.yaml	CI workflow with separate unit and integration test jobs running on Node 22
tests/helpers/mock-context.ts	Mock context factory for unit testing function handlers with configurable options
tests/mocks/*.ts	Mock implementations for external dependencies (7 mocks total)
tests/integration/runtime.test.ts	Integration tests for fn-runtime HTTP layer testing POST requests and error handling
tests/integration/helpers/start-function.ts	Helper utility to start function servers for integration testing
functions/example/tests/handler.test.ts	Unit tests for example handler covering response shape and error throwing
functions/simple-email/tests/handler.test.ts	Comprehensive unit tests for simple-email covering validation, sending, env config, and dry-run mode
functions/send-email-link/tests/handler.test.ts	Unit tests for send-email-link handler focusing on validation logic
functions/send-email-link/tests/handler.graphql.test.ts	TODO placeholders for future GraphQL integration tests
pnpm-lock.yaml	Added ~200+ Jest ecosystem packages and dependencies

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The integration test references generated/example/dist/index.js which requires building the example function first. However, the test:integration script only runs jest --testPathPatterns='tests/integration' without ensuring the functions are built.

The test workflow does include pnpm build before running pnpm test:integration, but this dependency should also be documented in the test script or the integration test file itself to prevent local test failures.

Consider either:

1. Adding a comment in the integration test file noting the build requirement
2. Creating a pretest:integration script that runs the build
3. Adding a check in the integration test to fail early with a helpful message if the file doesn't exist

[Copilot]

The test "passes job headers to handler context" doesn't verify that the headers are actually passed correctly - it only checks that the response is 200. The test should verify that the context received the expected job headers (X-Job-Id, X-Worker-Id, X-Database-Id) by checking the handler's behavior or response.

Consider either mocking the handler to verify it received the correct context, or having the example handler echo back the context so you can verify the headers were properly parsed.

Suggested change

	expect(res.status).toBe(200);
	expect(res.status).toBe(200);
	const body = await res.json();
	expect(body).toHaveProperty('context');
	expect(body.context).toMatchObject({
	jobId: 'job-123',
	workerId: 'worker-456',
	databaseId: 'db-789'
	});

[Copilot]

The FunctionContext type is duplicated here as a comment says it mirrors the type from packages/fn-runtime/src/types.ts. This creates a maintenance risk - if the real type changes, tests could pass with an outdated mock type.

Consider either:

1. Importing the actual type from @constructive-io/fn-runtime (preferred)
2. Using a build step to generate this type
3. Adding a test that validates the mock type matches the real type structure

[Copilot]

The test uses 'any' type for the handler variable, which disables TypeScript's type checking. This could hide potential type mismatches. Consider using the proper type:

let handler: FunctionHandler<any>;

or importing the specific handler type from the module.

Suggested change

	describe('send-email-link handler (validation)', () => {
	let handler: any;
	import { FunctionHandler } from '@azure/functions';
	describe('send-email-link handler (validation)', () => {
	let handler: FunctionHandler<any>;

[Copilot]

Hardcoded port 19876 could cause conflicts if multiple test suites run in parallel or if the port is already in use. Consider using a dynamic port assignment (port 0) and retrieving the actual assigned port from the server, or use a port range with retry logic.

Example approach:

const server = app.listen(0, () => {  // 0 = dynamic port
  const actualPort = (server.address() as any).port;
  resolve({
    server,
    url: `http://localhost:${actualPort}`,
    close: ...
  });
});

Suggested change

	const PORT = 19876;
	const PORT = 0;

[Copilot]

The mock context uses 'any' type for the client and meta request functions. This reduces type safety and could hide bugs. Consider defining a proper mock type or using a more specific type like:

client: {
  request: jest.fn<Promise<any>, [string, Record<string, any>?]>().mockResolvedValue(options.clientResponse ?? {})
} as any,

Or better yet, import and use the actual GraphQLClient type from graphql-request.

[Copilot]

The test pattern for validation in send-email-link differs from simple-email. In send-email-link, some validations return an error object { error: '...' } while others throw errors. This inconsistency could be confusing.

Looking at the handler code (lines 271-298 in handler.ts), the handler wraps validation failures that return { missing: '...' } into thrown errors. The test on line 11-20 correctly tests the early-return error case for missing databaseId, but the remaining tests check for thrown errors after validation failures.

Consider documenting this dual error handling pattern or normalizing to a single approach for consistency.

[Copilot]

The test script uses --testPathPatterns with a pattern that may not match correctly. The pattern 'functions/.+/__tests__' will match any path containing functions/ followed by any characters and then /__tests__, but test files are typically in __tests__/*.test.ts.

Consider using a more explicit pattern:

"test:unit": "jest --testPathPattern='functions/.*/__tests__/.*\\.test\\.ts$'"

Or rely on Jest's default test matching and use --testMatch instead:

"test:unit": "jest functions"

Suggested change

	"test:unit": "jest --testPathPatterns='functions/.+/__tests__'",
	"test:unit": "jest functions",

[Copilot]

The testRegex pattern (/__tests__/.*|(\\.|/)(test|spec))\\.tsx?$ is complex and might not match all intended test files. According to Jest best practices, it's generally better to use testMatch instead of testRegex for clarity.

Consider replacing with:

testMatch: [
  '**/__tests__/**/*.test.ts',
  '**/__tests__/**/*.spec.ts',
  '**/*.test.ts',
  '**/*.spec.ts'
],

This is more explicit and easier to understand.

Suggested change

	testRegex: '(/__tests__/.*|(\\.|/)(test|spec))\\.tsx?$',
	testMatch: [
	'**/__tests__/**/*.test.ts',
	'**/__tests__/**/*.test.tsx',
	'**/__tests__/**/*.spec.ts',
	'**/__tests__/**/*.spec.tsx',
	'**/*.test.ts',
	'**/*.test.tsx',
	'**/*.spec.ts',
	'**/*.spec.tsx'
	],

[Copilot]

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 7 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The @pgpmjs/env mock implements the actual logic rather than using Jest mocks. This means tests are testing the real parseEnvBoolean and parseEnvNumber implementations instead of being able to control their behavior. Consider using jest.fn() mocks if you want to isolate the function being tested, or rename this file to indicate it's not a mock but a test implementation of the actual module.

Suggested change

	export const parseEnvBoolean = (
	value: string | undefined
	): boolean | undefined => {
	if (value === undefined || value === '') return undefined;
	const lower = value.toLowerCase();
	if (['true', '1', 'yes'].includes(lower)) return true;
	if (['false', '0', 'no'].includes(lower)) return false;
	return undefined;
	};
	export const parseEnvNumber = (
	value: string | undefined
	): number | undefined => {
	if (value === undefined || value === '') return undefined;
	const num = Number(value);
	return isNaN(num) ? undefined : num;
	};
	export const parseEnvBoolean = jest.fn(
	(value: string | undefined): boolean | undefined => {
	if (value === undefined || value === '') return undefined;
	const lower = value.toLowerCase();
	if (['true', '1', 'yes'].includes(lower)) return true;
	if (['false', '0', 'no'].includes(lower)) return false;
	return undefined;
	}
	);
	export const parseEnvNumber = jest.fn(
	(value: string | undefined): number | undefined => {
	if (value === undefined || value === '') return undefined;
	const num = Number(value);
	return isNaN(num) ? undefined : num;
	}
	);

[Copilot]

The testTimeout is set to 30000ms (30 seconds) globally. This is quite long for unit tests and might hide performance issues. Consider using a shorter timeout for unit tests (e.g., 5000ms) and only increasing it for specific integration tests that need it using jest.setTimeout() within those test files.

Suggested change

	testTimeout: 30000
	testTimeout: 5000

[Copilot]

The workflow uses a hardcoded pnpm version (10.12.2) which may become outdated. Consider using 'latest' or defining the version in a central location (like package.json packageManager field) that can be referenced by both local development and CI to ensure version consistency.

[Copilot]

The createMockContext uses 'databaseId' as a key in options but checks for it using 'databaseId' in options on line 32. This works but the ternary condition could be simplified to options.databaseId ?? 'test-db' which is more idiomatic and handles both undefined and missing keys correctly.

Suggested change

	databaseId: 'databaseId' in options ? options.databaseId : 'test-db'
	databaseId: options.databaseId ?? 'test-db'

[Copilot]

The port 19876 is hardcoded but there's no check for port availability. If this port is already in use when tests run, the tests will fail with an unclear error. Consider using a random available port or documenting that this specific port is required for integration tests.

Suggested change

	const PORT = 19876;
	const PORT = 0; // use an ephemeral port to avoid conflicts

[Copilot]

This file contains only TODO tests with no implementation. While it's acceptable to have placeholder tests for future GraphQL integration testing, consider either implementing at least one test or moving this to a separate tracking issue/document until the test infrastructure is ready. TODO tests can create noise in test output and may be forgotten.

[Copilot]

The startFunction helper deletes the module from require.cache but doesn't handle potential errors during module loading. If the module path is incorrect or the module fails to load, the error will be unclear. Consider adding error handling and validation to provide better error messages during test debugging.

Suggested change

	delete require.cache[require.resolve(modulePath)];
	const mod = require(modulePath);
	const app = mod.default ?? mod;
	let mod: any;
	let resolvedPath: string;
	try {
	resolvedPath = require.resolve(modulePath);
	delete require.cache[resolvedPath];
	// eslint-disable-next-line @typescript-eslint/no-var-requires
	mod = require(resolvedPath);
	} catch (err: any) {
	const message =
	err && err.message
	? err.message
	: String(err);
	throw new Error(
	`Failed to load module for startFunction from path "${modulePath}": ${message}`
	);
	}
	const app = (mod as any).default ?? mod;
	if (!app || typeof (app as any).listen !== 'function') {
	throw new Error(
	`Module "${modulePath}" does not export an app with a "listen" method expected by startFunction.`
	);
	}

[Anmol1696]

I think these tests are not needed right now. i think we need to get the crd up and running before we can merge these tests.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The integration suite uses a fixed port (19876). This can lead to flaky failures when the port is already in use (especially in local/dev or when running Jest with parallelism). Prefer binding to an ephemeral port and deriving the URL from server.address(), or implement a simple free-port selection strategy.

Suggested change

	const EXAMPLE_MODULE = path.resolve(
	__dirname,
	'../../generated/example/dist/index.js'
	);
	const PORT = 19876;
	describe('fn-runtime HTTP layer', () => {
	let url: string;
	let close: () => Promise<void>;
	beforeAll(async () => {
	const started = await startFunction(EXAMPLE_MODULE, PORT);
	import * as net from 'net';
	const EXAMPLE_MODULE = path.resolve(
	__dirname,
	'../../generated/example/dist/index.js'
	);
	async function getFreePort(): Promise<number> {
	return await new Promise((resolve, reject) => {
	const server = net.createServer();
	.server.once('error', reject);
	server.listen(0, () => {
	const address = server.address();
	if (!address || typeof address === 'string') {
	server.close(() =>
	reject(new Error('Failed to determine free port from server address'))
	);
	return;
	}
	const port = address.port;
	server.close(err => (err ? reject(err) : resolve(port)));
	});
	});
	}
	describe('fn-runtime HTTP layer', () => {
	let url: string;
	let close: () => Promise<void>;
	beforeAll(async () => {
	const port = await getFreePort();
	const started = await startFunction(EXAMPLE_MODULE, port);

[Copilot]

modulePathIgnorePatterns includes dist/ and generated/, which can make modules under those paths non-requireable in Jest. The integration test loads generated/example/dist/index.js via require(), so this setting is likely to break the integration suite. Consider removing these entries from modulePathIgnorePatterns and using testPathIgnorePatterns (or narrower regexes) to avoid test discovery in those folders instead.

Suggested change

	moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'],
	modulePathIgnorePatterns: ['dist/', 'generated/'],
	testPathIgnorePatterns: ['/dist/', '/generated/'],
	moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'],

[Anmol1696]

Heads-up: repository history was rewritten on 2026-05-12 to scrub leaked secrets (Postgres/pgAdmin default passwords, an AWS access key ID, generated k8s/manifests/interweb-*.yaml files) from every commit. Every branch on origin was force-pushed with new commit SHAs.

This PR shows "DIRTY" / merge-conflict status because main has a cleanup commit and this branch is based on pre-rewrite main. To clear it:

git fetch --all --prune
git checkout <this-branch>
git reset --hard origin/<this-branch>    # your local branch tip moved too — pick up the rewritten version
git rebase origin/main                    # rebase onto rewritten main
# resolve conflicts (usually trivial — mostly secret-placeholder + the 4 deleted interweb-*.yaml files)
git push --force-with-lease

Or merge instead of rebase if the branch is shared with others:

git merge origin/main
git push

Notes:

• The secrets that leaked were all either dead (rotated AWS key) or were defaults that have since been rotated/replaced; no active credential is exposed.
• Old commit SHAs are still accessible by direct URL on GitHub for ~90 days (can be expedited via GitHub Support if needed).
• See k8s/SECRET-EXPOSURE-AUDIT.md on main for the full incident audit.