From 0610e9e21aee608f3bc4e71e6742110c454b7f90 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Fri, 27 Feb 2026 22:45:03 +0100 Subject: [PATCH] test: improve test suite quality and add lifecycle helpers - Add TestLifecycleHelpers module with reusable setup/cleanup classes - Add afterEach hooks to prevent test pollution - Fix lint errors and add complete JSDoc documentation - Update TEST_STYLE_GUIDE with acceptable eslint-disable patterns - Rename OCPP20AuthAdapter.offline.test.ts for naming consistency --- tests/TEST_STYLE_GUIDE.md | 31 ++ .../charging-station/ChargingStation.test.ts | 12 +- .../ChargingStationTestUtils.ts | 13 +- tests/charging-station/Helpers.test.ts | 10 +- ...CPP20ServiceUtils-TransactionEvent.test.ts | 8 +- .../ocpp/2.0/OCPP20VariableManager.test.ts | 2 + ...t.ts => OCPP20AuthAdapter-Offline.test.ts} | 0 tests/helpers/TestLifecycleHelpers.ts | 359 ++++++++++++++++++ tests/utils/Utils.test.ts | 6 +- 9 files changed, 434 insertions(+), 7 deletions(-) rename tests/charging-station/ocpp/auth/adapters/{OCPP20AuthAdapter.offline.test.ts => OCPP20AuthAdapter-Offline.test.ts} (100%) create mode 100644 tests/helpers/TestLifecycleHelpers.ts diff --git a/tests/TEST_STYLE_GUIDE.md b/tests/TEST_STYLE_GUIDE.md index a26b3757..903ce704 100644 --- a/tests/TEST_STYLE_GUIDE.md +++ b/tests/TEST_STYLE_GUIDE.md @@ -298,6 +298,37 @@ expect(result.token).toBeDefined() **Why:** Disabling linting rules hides real problems. Fix the underlying type issues instead. +**Exception - Legitimate Uses of eslint-disable:** + +Some eslint-disable comments are acceptable when testing defensive code that validates inputs at runtime: + +```typescript +// Testing that validators handle invalid types gracefully +// This is legitimate because the function is designed to handle runtime type errors +await it('should return false for non-string input', () => { + // Testing runtime type validation - intentionally passing wrong type + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-explicit-any + expect(AuthValidators.isValidIdentifierValue(123 as any)).toBe(false) +}) + +// Testing async function detection requires empty function expressions +// eslint-disable-next-line @typescript-eslint/no-empty-function +expect(isAsyncFunction(() => {})).toBe(false) +``` + +**Acceptable Rules to Disable (with justification):** + +- `@typescript-eslint/no-empty-function` - When testing function type detection +- `@typescript-eslint/no-explicit-any` - When testing runtime type validation +- `@typescript-eslint/unbound-method` - When testing method type detection +- `@cspell/spellchecker` - For intentional misspellings in test data + +**Still NOT Acceptable:** + +- File-level disables (`/* eslint-disable ... */` at top of file) +- Disabling rules to bypass type safety in test setup +- Disabling rules because proper interfaces haven't been created + ## Summary - **Name clearly**: Descriptive names for files, suites, and test cases diff --git a/tests/charging-station/ChargingStation.test.ts b/tests/charging-station/ChargingStation.test.ts index 32546187..2f2a1901 100644 --- a/tests/charging-station/ChargingStation.test.ts +++ b/tests/charging-station/ChargingStation.test.ts @@ -9,11 +9,12 @@ * - ChargingStation-Configuration.test.ts: boot notification, config persistence, WebSocket, error handling */ import { expect } from '@std/expect' -import { describe, it } from 'node:test' +import { afterEach, describe, it } from 'node:test' import type { ChargingStation } from '../../src/charging-station/ChargingStation.js' import { RegistrationStatusEnumType } from '../../src/types/index.js' +import { standardCleanup } from '../helpers/TestLifecycleHelpers.js' import { cleanupChargingStation, createMockChargingStation, @@ -25,6 +26,9 @@ import { await describe('ChargingStation Integration Tests', async () => { await describe('Test Utilities Verification', async () => { + afterEach(() => { + standardCleanup() + }) await it('should create mock charging station with default options', () => { const result = createMockChargingStation() const station = result.station @@ -105,6 +109,12 @@ await describe('ChargingStation Integration Tests', async () => { }) await describe('Cross-Domain Integration', async () => { + afterEach(() => { + standardCleanup() + if (station != null) { + cleanupChargingStation(station) + } + }) let station: ChargingStation | undefined await it('should support full lifecycle with transactions', async () => { diff --git a/tests/charging-station/ChargingStationTestUtils.ts b/tests/charging-station/ChargingStationTestUtils.ts index c23bb1a6..66c697bc 100644 --- a/tests/charging-station/ChargingStationTestUtils.ts +++ b/tests/charging-station/ChargingStationTestUtils.ts @@ -12,6 +12,17 @@ * @see tests/charging-station/ChargingStationTestConstants.ts for test constants */ +// Re-export test lifecycle helpers +export { + clearConnectorTransaction, + setupConnectorWithTransaction, + standardCleanup, + TestEnvironmentHelper, + TestStationHelper, + TestTimerHelper, +} from '../helpers/TestLifecycleHelpers.js' +export type { MockableTimerAPI, TimerHelperOptions } from '../helpers/TestLifecycleHelpers.js' + // Re-export all helper functions and types export type { ChargingStationMocks, @@ -19,6 +30,7 @@ export type { MockChargingStationOptions, MockChargingStationResult, } from './helpers/StationHelpers.js' + export { cleanupChargingStation, createConnectorStatus, @@ -29,6 +41,5 @@ export { } from './helpers/StationHelpers.js' export { MockIdTagsCache, MockSharedLRUCache } from './mocks/MockCaches.js' - // Re-export all mock classes export { MockWebSocket, WebSocketReadyState } from './mocks/MockWebSocket.js' diff --git a/tests/charging-station/Helpers.test.ts b/tests/charging-station/Helpers.test.ts index 271028f8..b26fe190 100644 --- a/tests/charging-station/Helpers.test.ts +++ b/tests/charging-station/Helpers.test.ts @@ -2,10 +2,9 @@ * @file Tests for Helpers * @description Unit tests for charging station helper functions and utilities */ -/* eslint-disable @typescript-eslint/no-unsafe-member-access */ import { expect } from '@std/expect' -import { describe, it } from 'node:test' +import { afterEach, describe, it } from 'node:test' import { checkChargingStationState, @@ -35,11 +34,16 @@ import { } from '../../src/types/index.js' import { logger } from '../../src/utils/Logger.js' import { createChargingStation, createChargingStationTemplate } from '../ChargingStationFactory.js' +import { standardCleanup } from '../helpers/TestLifecycleHelpers.js' await describe('Helpers test suite', async () => { const baseName = 'CS-TEST' const chargingStationTemplate = createChargingStationTemplate(baseName) + afterEach(() => { + standardCleanup() + }) + // Helper to create test reservations with configurable expiry const createTestReservation = (expired = false): Reservation => ({ @@ -63,7 +67,7 @@ await describe('Helpers test suite', async () => { // For validation edge cases, we need to manually create invalid states // since the factory is designed to create valid configurations const stationNoInfo = createChargingStation({ baseName }) - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access delete (stationNoInfo as any).stationInfo expect(() => { validateStationInfo(stationNoInfo) diff --git a/tests/charging-station/ocpp/2.0/OCPP20ServiceUtils-TransactionEvent.test.ts b/tests/charging-station/ocpp/2.0/OCPP20ServiceUtils-TransactionEvent.test.ts index 1e4a5dbe..9752ea95 100644 --- a/tests/charging-station/ocpp/2.0/OCPP20ServiceUtils-TransactionEvent.test.ts +++ b/tests/charging-station/ocpp/2.0/OCPP20ServiceUtils-TransactionEvent.test.ts @@ -8,7 +8,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { expect } from '@std/expect' -import { describe, it } from 'node:test' +import { afterEach, describe, it } from 'node:test' import { OCPP20ServiceUtils } from '../../../../src/charging-station/ocpp/2.0/OCPP20ServiceUtils.js' import { @@ -23,6 +23,7 @@ import { type OCPP20TransactionContext, } from '../../../../src/types/ocpp/2.0/Transaction.js' import { Constants, generateUUID } from '../../../../src/utils/index.js' +import { standardCleanup } from '../../../../tests/helpers/TestLifecycleHelpers.js' import { createChargingStation } from '../../../ChargingStationFactory.js' import { TEST_CHARGING_STATION_BASE_NAME } from '../../ChargingStationTestConstants.js' import { createMockOCPP20TransactionTestStation, resetLimits } from './OCPP20TestUtils.js' @@ -33,6 +34,11 @@ await describe('E01-E04 - OCPP 2.0.1 TransactionEvent Implementation', async () // Reset limits before tests resetLimits(mockChargingStation) + // Reset singleton state and timers after each test to ensure test isolation + afterEach(() => { + standardCleanup() + }) + // FR: E01.FR.01 - TransactionEventRequest structure validation await describe('buildTransactionEvent', async () => { await it('should build valid TransactionEvent Started with sequence number 0', () => { diff --git a/tests/charging-station/ocpp/2.0/OCPP20VariableManager.test.ts b/tests/charging-station/ocpp/2.0/OCPP20VariableManager.test.ts index 8891b140..605dd1c7 100644 --- a/tests/charging-station/ocpp/2.0/OCPP20VariableManager.test.ts +++ b/tests/charging-station/ocpp/2.0/OCPP20VariableManager.test.ts @@ -31,6 +31,7 @@ import { type VariableType, } from '../../../../src/types/index.js' import { Constants } from '../../../../src/utils/index.js' +import { standardCleanup } from '../../../../tests/helpers/TestLifecycleHelpers.js' import { createChargingStation } from '../../../ChargingStationFactory.js' import { TEST_CHARGING_STATION_BASE_NAME } from '../../ChargingStationTestConstants.js' import { @@ -76,6 +77,7 @@ await describe('B05/B06 - OCPP20VariableManager test suite', async () => { // Reset singleton state after each test to ensure test isolation afterEach(() => { + standardCleanup() OCPP20VariableManager.getInstance().resetRuntimeOverrides() }) diff --git a/tests/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.offline.test.ts b/tests/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter-Offline.test.ts similarity index 100% rename from tests/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.offline.test.ts rename to tests/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter-Offline.test.ts diff --git a/tests/helpers/TestLifecycleHelpers.ts b/tests/helpers/TestLifecycleHelpers.ts new file mode 100644 index 00000000..3919b73e --- /dev/null +++ b/tests/helpers/TestLifecycleHelpers.ts @@ -0,0 +1,359 @@ +/** + * @file Test Lifecycle Helpers + * @description Reusable lifecycle helpers for test setup and cleanup to reduce boilerplate. + * + * These helpers encapsulate common beforeEach/afterEach patterns used across the test suite, + * ensuring consistent test isolation and preventing pollution between tests. + * @example + * ```typescript + * import { TestTimerHelper, withMockStation } from '../helpers/TestLifecycleHelpers.js' + * + * describe('MyTest', () => { + * const timerHelper = new TestTimerHelper() + * + * beforeEach(() => timerHelper.setup()) + * afterEach(() => timerHelper.cleanup()) + * + * // Or use the functional pattern + * withMockStation({ connectorsCount: 2 }, (getStation) => { + * it('should test something', () => { + * const station = getStation() + * // test with station + * }) + * }) + * }) + * ``` + */ + +import { mock } from 'node:test' + +import type { ChargingStation } from '../../src/charging-station/ChargingStation.js' +import type { + MockChargingStationOptions, + MockChargingStationResult, +} from '../charging-station/helpers/StationHelpers.js' + +import { + cleanupChargingStation, + createMockChargingStation, +} from '../charging-station/helpers/StationHelpers.js' +import { MockIdTagsCache, MockSharedLRUCache } from '../charging-station/mocks/MockCaches.js' + +/** + * Timer APIs that can be mocked in tests + */ +export type MockableTimerAPI = 'setImmediate' | 'setInterval' | 'setTimeout' + +/** + * Configuration options for TestTimerHelper + */ +export interface TimerHelperOptions { + /** + * Timer APIs to mock (default: all three) + */ + apis?: MockableTimerAPI[] +} + +/** + * Helper class for managing mock charging stations in tests + * + * Provides automatic cleanup of charging station resources including + * timers, WebSocket connections, and singleton mocks. + * @example + * ```typescript + * describe('MyTest', () => { + * const stationHelper = new TestStationHelper({ connectorsCount: 2 }) + * + * beforeEach(() => stationHelper.setup()) + * afterEach(() => stationHelper.cleanup()) + * + * it('should test station', () => { + * const { station, mocks } = stationHelper.get() + * // test with station + * }) + * }) + * ``` + */ +export class TestStationHelper { + private readonly options: MockChargingStationOptions + private result: MockChargingStationResult | null = null + + constructor (options: MockChargingStationOptions = {}) { + this.options = options + } + + /** + * Clean up the charging station (call in afterEach) + */ + cleanup (): void { + if (this.result != null) { + cleanupChargingStation(this.result.station) + this.result = null + } + // Also reset singleton mocks + MockSharedLRUCache.resetInstance() + MockIdTagsCache.resetInstance() + } + + /** + * Get the current mock result (throws if not setup) + * @returns The mock charging station result + */ + get (): MockChargingStationResult { + if (this.result == null) { + throw new Error('TestStationHelper.setup() must be called before get()') + } + return this.result + } + + /** + * Get just the station (convenience method) + * @returns The charging station instance + */ + getStation (): ChargingStation { + return this.get().station + } + + /** + * Check if station is currently setup + * @returns True if station is setup + */ + isSetup (): boolean { + return this.result != null + } + + /** + * Create the mock charging station (call in beforeEach) + * @returns The mock charging station result + */ + setup (): MockChargingStationResult { + this.result = createMockChargingStation(this.options) + return this.result + } +} + +/** + * Helper class for managing mock timers in tests + * + * Encapsulates the common pattern of enabling and resetting mock timers, + * ensuring consistent cleanup and preventing timer leaks between tests. + * @example + * ```typescript + * describe('MyTest', () => { + * const timerHelper = new TestTimerHelper() + * + * beforeEach(() => timerHelper.setup()) + * afterEach(() => timerHelper.cleanup()) + * + * it('should handle timer-based logic', () => { + * // Timer-dependent code works with mock timers + * mock.timers.tick(1000) + * }) + * }) + * ``` + */ +export class TestTimerHelper { + private readonly apis: MockableTimerAPI[] + private isSetup = false + + constructor (options: TimerHelperOptions = {}) { + this.apis = options.apis ?? ['setInterval', 'setTimeout', 'setImmediate'] + } + + /** + * Reset mock timers (call in afterEach) + */ + cleanup (): void { + if (this.isSetup) { + mock.timers.reset() + this.isSetup = false + } + } + + /** + * Enable mock timers (call in beforeEach) + */ + setup (): void { + if (!this.isSetup) { + mock.timers.enable({ apis: this.apis }) + this.isSetup = true + } + } + + /** + * Advance mock timers by specified milliseconds + * @param ms - Milliseconds to advance + */ + tick (ms: number): void { + mock.timers.tick(ms) + } +} + +/** + * Combined helper for tests that need both timers and station + * @example + * ```typescript + * describe('MyTest', () => { + * const helper = new TestEnvironmentHelper({ connectorsCount: 2 }) + * + * beforeEach(() => helper.setup()) + * afterEach(() => helper.cleanup()) + * + * it('should test with timers and station', () => { + * const station = helper.getStation() + * helper.tick(1000) // Advance time + * }) + * }) + * ``` + */ +export class TestEnvironmentHelper { + private readonly stationHelper: TestStationHelper + private readonly timerHelper: TestTimerHelper + + constructor ( + stationOptions: MockChargingStationOptions = {}, + timerOptions: TimerHelperOptions = {} + ) { + this.timerHelper = new TestTimerHelper(timerOptions) + this.stationHelper = new TestStationHelper(stationOptions) + } + + /** + * Cleanup both timers and station + */ + cleanup (): void { + this.stationHelper.cleanup() + this.timerHelper.cleanup() + mock.restoreAll() + } + + /** + * Get the mock station result + * @returns The mock charging station result + */ + get (): MockChargingStationResult { + return this.stationHelper.get() + } + + /** + * Get just the station + * @returns The charging station instance + */ + getStation (): ChargingStation { + return this.stationHelper.getStation() + } + + /** + * Setup both timers and station + * @returns The mock charging station result + */ + setup (): MockChargingStationResult { + this.timerHelper.setup() + return this.stationHelper.setup() + } + + /** + * Advance mock timers + * @param ms - Milliseconds to advance + */ + tick (ms: number): void { + this.timerHelper.tick(ms) + } +} + +/** + * Clear transaction state from a connector + * @param station - ChargingStation instance + * @param connectorId - Connector to clear + */ +export function clearConnectorTransaction (station: ChargingStation, connectorId: number): void { + const connector = station.getConnectorStatus(connectorId) + if (connector == null) { + return + } + + connector.transactionStarted = false + connector.transactionId = undefined + connector.transactionIdTag = undefined + connector.transactionEnergyActiveImportRegisterValue = 0 + connector.transactionRemoteStarted = false + connector.transactionStart = undefined + connector.idTagAuthorized = false + connector.idTagLocalAuthorized = false + + // Clear any transaction interval + if (connector.transactionSetInterval != null) { + clearInterval(connector.transactionSetInterval) + connector.transactionSetInterval = undefined + } +} + +/** + * Setup a connector with an active transaction + * + * Reduces boilerplate when tests need a connector in transaction state. + * @param station - ChargingStation instance + * @param connectorId - Connector to setup + * @param options - Transaction options + * @param options.transactionId - Transaction ID to set + * @param options.idTag - ID tag for the transaction (default: TAG-{transactionId}) + * @param options.energyImport - Energy import value in Wh (default: 0) + * @param options.remoteStarted - Whether transaction was remote started (default: false) + * @example + * ```typescript + * setupConnectorWithTransaction(station, 1, { + * transactionId: 100, + * idTag: 'TEST-TAG-001', + * energyImport: 1000 + * }) + * ``` + */ +export function setupConnectorWithTransaction ( + station: ChargingStation, + connectorId: number, + options: { + energyImport?: number + idTag?: string + remoteStarted?: boolean + transactionId: number + } +): void { + const connector = station.getConnectorStatus(connectorId) + if (connector == null) { + throw new Error(`Connector ${String(connectorId)} not found`) + } + + connector.transactionStarted = true + connector.transactionId = options.transactionId + connector.transactionIdTag = options.idTag ?? `TAG-${String(options.transactionId)}` + connector.transactionEnergyActiveImportRegisterValue = options.energyImport ?? 0 + connector.transactionRemoteStarted = options.remoteStarted ?? false + connector.transactionStart = new Date() + connector.idTagAuthorized = true +} + +/** + * Standard afterEach cleanup function + * + * Use this when you need a simple cleanup without the helper classes. + * Restores all mocks and resets timers. + * @example + * ```typescript + * afterEach(() => { + * standardCleanup() + * if (station != null) { + * cleanupChargingStation(station) + * } + * }) + * ``` + */ +export function standardCleanup (): void { + mock.restoreAll() + try { + mock.timers.reset() + } catch { + // Timers may not have been enabled, ignore + } + MockSharedLRUCache.resetInstance() + MockIdTagsCache.resetInstance() +} diff --git a/tests/utils/Utils.test.ts b/tests/utils/Utils.test.ts index a980380d..8b3db1a4 100644 --- a/tests/utils/Utils.test.ts +++ b/tests/utils/Utils.test.ts @@ -7,7 +7,7 @@ import { hoursToMilliseconds, hoursToSeconds } from 'date-fns' import { CircularBuffer } from 'mnemonist' import { randomInt } from 'node:crypto' import { version } from 'node:process' -import { describe, it } from 'node:test' +import { afterEach, describe, it } from 'node:test' import { satisfies } from 'semver' import type { TimestampedData } from '../../src/types/index.js' @@ -43,8 +43,12 @@ import { validateIdentifierString, validateUUID, } from '../../src/utils/Utils.js' +import { standardCleanup } from '../helpers/TestLifecycleHelpers.js' await describe('Utils test suite', async () => { + afterEach(() => { + standardCleanup() + }) await it('should verify generateUUID()/validateUUID()', () => { const uuid = generateUUID() expect(uuid).toBeDefined() -- 2.53.0