From 364b90195a1692482fe25ceea563012c2bbb4b77 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Fri, 27 Feb 2026 19:22:56 +0100 Subject: [PATCH] docs(tests): add test style guide - Document naming conventions, AAA pattern, JSDoc requirements - Cover constants usage, mock factories, cleanup hooks - Include anti-patterns with explanations (as any, duplication, flaky tests) - 18 sections with concrete examples from codebase - Establishes foundation for consistent test quality --- .../ocpp/2.0/__testable__/index.ts | 181 ++++++++++++ tests/TEST_STYLE_GUIDE.md | 259 ++++++++++++++++++ 2 files changed, 440 insertions(+) create mode 100644 src/charging-station/ocpp/2.0/__testable__/index.ts create mode 100644 tests/TEST_STYLE_GUIDE.md diff --git a/src/charging-station/ocpp/2.0/__testable__/index.ts b/src/charging-station/ocpp/2.0/__testable__/index.ts new file mode 100644 index 00000000..8c4c461a --- /dev/null +++ b/src/charging-station/ocpp/2.0/__testable__/index.ts @@ -0,0 +1,181 @@ +/** + * Testable interface for OCPP 2.0 IncomingRequestService + * + * This module provides type-safe access to private handler methods for testing purposes. + * It replaces `as any` casts with a properly typed interface, enabling: + * - Type-safe method invocations in tests + * - IntelliSense and autocompletion for handler parameters/returns + * - Compile-time checking for test code + * @example + * ```typescript + * import { createTestableIncomingRequestService } from './__testable__/index.js' + * + * const testable = createTestableIncomingRequestService(incomingRequestService) + * const response = await testable.handleRequestReset(mockChargingStation, resetRequest) + * ``` + */ + +import type { + OCPP20CertificateSignedRequest, + OCPP20CertificateSignedResponse, + OCPP20ClearCacheResponse, + OCPP20DeleteCertificateRequest, + OCPP20DeleteCertificateResponse, + OCPP20GetBaseReportRequest, + OCPP20GetBaseReportResponse, + OCPP20GetInstalledCertificateIdsRequest, + OCPP20GetInstalledCertificateIdsResponse, + OCPP20GetVariablesRequest, + OCPP20GetVariablesResponse, + OCPP20InstallCertificateRequest, + OCPP20InstallCertificateResponse, + OCPP20RequestStartTransactionRequest, + OCPP20RequestStartTransactionResponse, + OCPP20RequestStopTransactionRequest, + OCPP20RequestStopTransactionResponse, + OCPP20ResetRequest, + OCPP20ResetResponse, + OCPP20SetVariablesRequest, + OCPP20SetVariablesResponse, +} from '../../../../types/index.js' +import type { ChargingStation } from '../../../index.js' +import type { OCPP20IncomingRequestService } from '../OCPP20IncomingRequestService.js' + +/** + * Interface exposing private handler methods of OCPP20IncomingRequestService for testing. + * Each method signature matches the corresponding private method in the service class. + */ +export interface TestableOCPP20IncomingRequestService { + /** + * Handles OCPP 2.0 CertificateSigned request from central system. + * Receives signed certificate chain from CSMS and stores it in the charging station. + */ + handleRequestCertificateSigned: ( + chargingStation: ChargingStation, + commandPayload: OCPP20CertificateSignedRequest + ) => Promise + + /** + * Handles OCPP 2.0.1 ClearCache request by clearing the Authorization Cache. + * Per C11.FR.04: Returns Rejected if AuthCacheEnabled is false. + */ + handleRequestClearCache: (chargingStation: ChargingStation) => Promise + + /** + * Handles OCPP 2.0 DeleteCertificate request from central system. + * Deletes a certificate matching the provided hash data from the charging station. + */ + handleRequestDeleteCertificate: ( + chargingStation: ChargingStation, + commandPayload: OCPP20DeleteCertificateRequest + ) => Promise + + /** + * Handles OCPP 2.0 GetBaseReport request. + * Returns device model report based on the requested report base type. + */ + handleRequestGetBaseReport: ( + chargingStation: ChargingStation, + commandPayload: OCPP20GetBaseReportRequest + ) => OCPP20GetBaseReportResponse + + /** + * Handles OCPP 2.0 GetInstalledCertificateIds request from central system. + * Returns list of installed certificates matching the optional filter types. + */ + handleRequestGetInstalledCertificateIds: ( + chargingStation: ChargingStation, + commandPayload: OCPP20GetInstalledCertificateIdsRequest + ) => Promise + + /** + * Handles OCPP 2.0 GetVariables request. + * Returns values for requested variables from the device model. + */ + handleRequestGetVariables: ( + chargingStation: ChargingStation, + commandPayload: OCPP20GetVariablesRequest + ) => OCPP20GetVariablesResponse + + /** + * Handles OCPP 2.0 InstallCertificate request from central system. + * Installs a certificate of the specified type in the charging station. + */ + handleRequestInstallCertificate: ( + chargingStation: ChargingStation, + commandPayload: OCPP20InstallCertificateRequest + ) => Promise + + /** + * Handles OCPP 2.0 Reset request. + * Performs immediate or scheduled reset of charging station or specific EVSE. + */ + handleRequestReset: ( + chargingStation: ChargingStation, + commandPayload: OCPP20ResetRequest + ) => Promise + + /** + * Handles OCPP 2.0 SetVariables request. + * Sets values for requested variables in the device model. + */ + handleRequestSetVariables: ( + chargingStation: ChargingStation, + commandPayload: OCPP20SetVariablesRequest + ) => OCPP20SetVariablesResponse + + /** + * Handles OCPP 2.0 RequestStartTransaction request from central system. + * Initiates charging transaction on specified EVSE with enhanced authorization. + */ + handleRequestStartTransaction: ( + chargingStation: ChargingStation, + commandPayload: OCPP20RequestStartTransactionRequest + ) => Promise + + /** + * Handles OCPP 2.0 RequestStopTransaction request from central system. + * Stops an ongoing transaction on the charging station. + */ + handleRequestStopTransaction: ( + chargingStation: ChargingStation, + commandPayload: OCPP20RequestStopTransactionRequest + ) => Promise +} + +/** + * Creates a testable wrapper around OCPP20IncomingRequestService. + * Provides type-safe access to private handler methods without `as any` casts. + * @param service - The OCPP20IncomingRequestService instance to wrap + * @returns A typed interface exposing private handler methods + * @example + * ```typescript + * // Before (with as any cast): + * const response = await (service as any).handleRequestReset(station, request) + * + * // After (with testable interface): + * const testable = createTestableIncomingRequestService(service) + * const response = await testable.handleRequestReset(station, request) + * ``` + */ +export function createTestableIncomingRequestService ( + service: OCPP20IncomingRequestService +): TestableOCPP20IncomingRequestService { + // Cast to unknown first to satisfy TypeScript while preserving runtime behavior + const serviceImpl = service as unknown as TestableOCPP20IncomingRequestService + + return { + handleRequestCertificateSigned: serviceImpl.handleRequestCertificateSigned.bind(service), + handleRequestClearCache: serviceImpl.handleRequestClearCache.bind(service), + handleRequestDeleteCertificate: serviceImpl.handleRequestDeleteCertificate.bind(service), + handleRequestGetBaseReport: serviceImpl.handleRequestGetBaseReport.bind(service), + handleRequestGetInstalledCertificateIds: + serviceImpl.handleRequestGetInstalledCertificateIds.bind(service), + handleRequestGetVariables: serviceImpl.handleRequestGetVariables.bind(service), + handleRequestInstallCertificate: serviceImpl.handleRequestInstallCertificate.bind(service), + handleRequestReset: serviceImpl.handleRequestReset.bind(service), + handleRequestSetVariables: serviceImpl.handleRequestSetVariables.bind(service), + handleRequestStartTransaction: serviceImpl.handleRequestStartTransaction.bind(service), + handleRequestStopTransaction: serviceImpl.handleRequestStopTransaction.bind(service), + } +} diff --git a/tests/TEST_STYLE_GUIDE.md b/tests/TEST_STYLE_GUIDE.md new file mode 100644 index 00000000..9a0cfa34 --- /dev/null +++ b/tests/TEST_STYLE_GUIDE.md @@ -0,0 +1,259 @@ +# Test Style Guide + +This document establishes conventions for writing maintainable, consistent tests in the e-mobility charging stations simulator project. + +## Naming Conventions + +- **Files**: Use descriptive names matching the module under test: `ModuleName.test.ts` +- **Test suites**: Use `describe()` with clear, specific descriptions +- **Test cases**: Use `it()` or `test()` with descriptive names starting with action verbs +- **Variables**: Use camelCase for variables, functions, and test helpers +- **Constants**: Use SCREAMING_SNAKE_CASE for test constants + +**Example:** + +```typescript +describe('ChargingStation lifecycle', () => { + it('should start successfully with valid configuration', async () => { + // test implementation + }) +}) +``` + +## Test Structure (AAA Pattern) + +Follow the Arrange-Act-Assert pattern for clarity: + +1. **Arrange**: Set up test data, mocks, and preconditions +2. **Act**: Execute the code under test +3. **Assert**: Verify the expected outcome + +**Example:** + +```typescript +it('should calculate total power correctly', () => { + // Arrange + const station = createMockChargingStation() + const expectedPower = 22000 + + // Act + const actualPower = station.getTotalPower() + + // Assert + expect(actualPower).toBe(expectedPower) +}) +``` + +## Comments & JSDoc + +### File Headers + +Every test file MUST include a JSDoc header: + +```typescript +/** + * @file Tests for ModuleName + * @description Brief description of what is being tested + */ +``` + +### Inline Comments + +- Use comments sparingly - prefer self-documenting test names +- Comment WHY, not WHAT (the code shows what) +- Document non-obvious setup or complex assertions + +## Constants + +**ALWAYS use consolidated test constants from the canonical source:** + +- ✅ Import from: `tests/charging-station/ChargingStationTestConstants.ts` +- ❌ NEVER duplicate constants in individual test files +- ❌ NEVER create inline magic values + +**Good:** + +```typescript +import { TEST_CHARGING_STATION_BASE_NAME } from '../ChargingStationTestConstants.js' +``` + +**Bad:** + +```typescript +// Don't do this! +const TEST_STATION_NAME = 'CS-TEST-001' // Duplicate constant +``` + +## Mocks & Factories + +### When to Use Mock Factories + +Use centralized mock factories for complex objects: + +- `createChargingStation()` - From `ChargingStationFactory.ts` +- `createMockChargingStation()` - From `ChargingStationTestUtils.ts` +- Auth mocks - From `tests/charging-station/ocpp/auth/helpers/MockFactories.ts` + +**Example:** + +```typescript +import { createChargingStation } from '../ChargingStationFactory.js' + +const station = await createChargingStation({ + ocppVersion: OCPPVersion.VERSION_20, + numberOfConnectors: 2, +}) +``` + +### Mocking Best Practices + +- Use `mock.method()` for function mocking (Node.js native) +- Use `mock.timers` for time-dependent tests +- Keep mocks focused - mock only what's necessary +- Verify mock calls when behavior depends on them + +## Cleanup Hooks + +**ALWAYS include `afterEach()` cleanup to prevent test pollution:** + +```typescript +afterEach(() => { + mock.restoreAll() + mock.timers.reset() + // Clean up any resources created in tests +}) +``` + +### What to Clean Up + +- Mock timers: `mock.timers.reset()` +- Mock functions: `mock.restoreAll()` +- Charging stations: `await chargingStation.stop()` +- File handles, network connections, database connections +- Any global state modifications + +**Missing cleanup causes flaky tests and false positives.** + +## Anti-Patterns to Avoid + +### 1. Inline `as any` Casts + +❌ **Bad:** + +```typescript +;(incomingRequestService as any).handleRequestReset(station, request) +``` + +✅ **Good:** + +```typescript +import { createTestableIncomingRequestService } from '../__testable__/index.js' + +const testable = createTestableIncomingRequestService(incomingRequestService) +await testable.handleRequestReset(station, request) +``` + +**Why:** Type safety prevents bugs. Use testable interfaces instead of breaking the type system. + +### 2. Duplicate Constants + +❌ **Bad:** + +```typescript +// In multiple test files: +const TEST_STATION_NAME = 'CS-TEST-001' +``` + +✅ **Good:** + +```typescript +import { TEST_CHARGING_STATION_BASE_NAME } from '../ChargingStationTestConstants.js' +``` + +**Why:** Single source of truth. Changes propagate automatically, reduces maintenance burden. + +### 3. Missing Cleanup + +❌ **Bad:** + +```typescript +describe('Tests', () => { + it('test 1', () => { + /* ... */ + }) + it('test 2', () => { + /* ... */ + }) + // No afterEach cleanup! +}) +``` + +✅ **Good:** + +```typescript +describe('Tests', () => { + afterEach(() => { + mock.restoreAll() + // Clean up resources + }) + + it('test 1', () => { + /* ... */ + }) + it('test 2', () => { + /* ... */ + }) +}) +``` + +**Why:** Test isolation. Each test should run independently without side effects. + +### 4. Probabilistic Assertions + +❌ **Bad:** + +```typescript +const successRate = calculateSuccessRate() +expect(successRate).toBeGreaterThan(50) // Flaky! +``` + +✅ **Good:** + +```typescript +const result = await authenticateUser(mockCredentials) +expect(result.success).toBe(true) +expect(result.token).toBeDefined() +``` + +**Why:** Tests must be deterministic. Use mocks to control behavior, not probabilistic thresholds. + +### 5. Over-Use of `eslint-disable` + +❌ **Bad:** + +```typescript +/* eslint-disable @typescript-eslint/no-unsafe-member-access */ +/* eslint-disable @typescript-eslint/no-unsafe-assignment */ +/* eslint-disable @typescript-eslint/no-unsafe-call */ +/* eslint-disable @typescript-eslint/no-explicit-any */ +``` + +✅ **Good:** + +```typescript +// Use proper types and testable interfaces - no disables needed +``` + +**Why:** Disabling linting rules hides real problems. Fix the underlying type issues instead. + +## Summary + +- **Name clearly**: Descriptive names for files, suites, and test cases +- **Structure with AAA**: Arrange, Act, Assert +- **Document minimally**: JSDoc headers required, inline comments only when necessary +- **Use canonical constants**: Single source of truth +- **Leverage mock factories**: Centralized, reusable mocks +- **Clean up always**: `afterEach()` hooks prevent test pollution +- **Avoid anti-patterns**: No `as any`, no duplication, no probabilistic tests + +Following these guidelines ensures tests are maintainable, reliable, and easy to understand. -- 2.53.0