# Implementation Guide: Fixing Test Isolation Issues This guide provides a step-by-step approach to resolving the test isolation issues in the network library. By implementing these changes, the tests that currently fail when run together will pass, maintaining proper isolation between test cases. ## Phase 1: Refactor TimerController ### Step 1: Update TimerController Class First, we need to modify the TimerController to support multiple instances: ```typescript // libs/network/src/utils/timer-controller.ts export class TimerController { // Remove static instance // private static _instance: TimerController; private _currentTime = 0; private _timers: Map = new Map(); private _nextTimerId = 1; /** * Create a new TimerController instance * This replaces the singleton getInstance method */ public static createInstance(): TimerController { return new TimerController(); } // Keep the original getInstance for backward compatibility // but have it create a new instance each time in test environment public static getInstance(): TimerController { if (process.env.NODE_ENV === 'test') { console.warn('Using TimerController.getInstance() in tests may cause isolation issues.'); return new TimerController(); } if (!TimerController._instance) { TimerController._instance = new TimerController(); } return TimerController._instance; } // Remaining methods stay the same } // Add factory function for tests export function createTestTimerController(): TimerController { return TimerController.createInstance(); } ``` ### Step 2: Create Timer Context Utility Next, create a utility to provide contextual timers for tests: ```typescript // libs/network/src/utils/test/timer-context.ts import { TimerController } from '../timer-controller'; interface TimerContext { timer: TimerController; getCurrentTime: () => number; setTimeout: (callback: () => void, delay: number) => number; setInterval: (callback: () => void, interval: number) => number; clearTimeout: (id: number) => void; clearInterval: (id: number) => void; advanceTime: (timeMs: number) => void; reset: () => void; } /** * Create a timer context for use in tests * This provides an isolated timer instance with utility methods */ export function createTimerContext(): TimerContext { const timer = TimerController.createInstance(); return { timer, getCurrentTime: () => timer.getCurrentTime(), setTimeout: (callback, delay) => timer.setTimeout(callback, delay), setInterval: (callback, interval) => timer.setInterval(callback, interval), clearTimeout: (id) => timer.clearTimeout(id), clearInterval: (id) => timer.clearInterval(id), advanceTime: (timeMs) => timer.advanceTime(timeMs), reset: () => timer.reset() }; } ``` ## Phase 2: Fix MemoryTransport Tests ### Step 1: Update MemoryTransport Class Modify MemoryTransport to accept a TimerController and add registry reset: ```typescript // libs/network/src/transport/memory-transport.ts export interface MemoryTransportOptions { eventEmitter?: INetworkEventEmitter; simulationOptions?: MemoryTransportSimulationOptions; timerController?: TimerController; // Add this } export class MemoryTransport extends BaseTransport { private static _instances: Map = new Map(); private _simOptions: MemoryTransportSimulationOptions; private _timer: TimerController; constructor(id: string, options?: MemoryTransportOptions) { super(id, options?.eventEmitter); this._simOptions = options?.simulationOptions || {}; this._timer = options?.timerController || TimerController.getInstance(); MemoryTransport._instances.set(id, this); } // Add this static method for test cleanup public static resetRegistry(): void { MemoryTransport._instances.clear(); } // Update time-sensitive methods to use the timer private async _simulateDelay(baseDelay: number): Promise { let delay = baseDelay; if (this._simOptions.jitter) { const jitterAmount = (Math.random() * 2 - 1) * this._simOptions.jitter; delay = Math.max(0, delay + jitterAmount); } if (delay > 0) { // Replace setTimeout with timer return new Promise(resolve => { this._timer.setTimeout(() => resolve(), delay); this._timer.advanceTime(delay); }); } } } ``` ### Step 2: Update MemoryTransport Tests Modify the tests to use isolated timers: ```typescript // libs/network/src/transport/memory-transport.spec.ts import { TimerController } from '../utils/timer-controller'; describe('MemoryTransport', () => { let transport1: MemoryTransport; let transport2: MemoryTransport; let eventEmitter1: NetworkEventEmitter; let eventEmitter2: NetworkEventEmitter; let timer1: TimerController; let timer2: TimerController; beforeEach(() => { // Reset static registry MemoryTransport.resetRegistry(); // Create isolated timer controllers timer1 = TimerController.createInstance(); timer2 = TimerController.createInstance(); eventEmitter1 = new NetworkEventEmitter(); eventEmitter2 = new NetworkEventEmitter(); // Create transports with isolated timers transport1 = new MemoryTransport('transport-1', { eventEmitter: eventEmitter1, timerController: timer1 }); transport2 = new MemoryTransport('transport-2', { eventEmitter: eventEmitter2, timerController: timer2 }); }); afterEach(() => { // Clean up transport1.disconnect().catch(e => console.error('Cleanup error:', e)); transport2.disconnect().catch(e => console.error('Cleanup error:', e)); MemoryTransport.resetRegistry(); // Reset timers timer1.reset(); timer2.reset(); }); // Update the time-sensitive tests describe('Simulated network conditions', () => { it('should simulate connection delay', async () => { const testTimer = TimerController.createInstance(); const simulationOptions: MemoryTransportSimulationOptions = { connectionDelay: 50 }; const transport = new MemoryTransport('delayed-transport', { simulationOptions, eventEmitter: new NetworkEventEmitter(), timerController: testTimer }); // Capture current timer time instead of Date.now() const startTime = testTimer.getCurrentTime(); // Start connection but don't await yet const connectPromise = transport.connect(); // Advance time testTimer.advanceTime(60); // More than the delay // Now wait for connection to complete await connectPromise; const endTime = testTimer.getCurrentTime(); expect(endTime - startTime).toBeGreaterThanOrEqual(50); // Clean up await transport.disconnect(); }); it('should simulate latency', async () => { const testTimer = TimerController.createInstance(); const simulationOptions: MemoryTransportSimulationOptions = { latency: 50 }; const testTransport = new MemoryTransport('latency-transport', { simulationOptions, eventEmitter: new NetworkEventEmitter(), timerController: testTimer }); const receiver = new MemoryTransport('receiver', { eventEmitter: new NetworkEventEmitter(), timerController: testTimer }); // Connect both transports await testTransport.connect(); await receiver.connect(); // Capture start time const startTime = testTimer.getCurrentTime(); // Start send but don't await yet const sendPromise = testTransport.sendTo('receiver', { test: true }); // Advance time testTimer.advanceTime(60); // More than the latency // Now wait for send to complete await sendPromise; const endTime = testTimer.getCurrentTime(); expect(endTime - startTime).toBeGreaterThanOrEqual(50); // Clean up await testTransport.disconnect(); await receiver.disconnect(); }); }); }); ``` ## Phase 3: Fix MessageBatch Tests ### Step 1: Update MessageBatch Class Modify MessageBatch to support TimerController: ```typescript // libs/network/src/router/message-batch.ts import { TimerController } from '../utils/timer-controller'; export interface MessageBatchOptions { timerController?: TimerController; } export class MessageBatch { private _timer: TimerController; private _creationTime: number; constructor( private _priority: MessagePriority, private _maxSize: number, options?: MessageBatchOptions ) { this._timer = options?.timerController || TimerController.getInstance(); this._creationTime = this._timer.getCurrentTime(); } // Update age getter to use timer get age(): number { return this._timer.getCurrentTime() - this._creationTime; } // Other methods stay the same } ``` ### Step 2: Update MessageBatch Tests ```typescript // libs/network/src/router/message-batch.spec.ts import { TimerController } from '../utils/timer-controller'; describe('MessageBatch', () => { let batch: MessageBatch; let timer: TimerController; beforeEach(() => { timer = TimerController.createInstance(); batch = new MessageBatch(MessagePriority.MEDIUM, 3, { timerController: timer }); }); afterEach(() => { timer.reset(); }); // Update the time-sensitive test describe('Age tracking', () => { it('should track batch age', () => { // Get initial age const initialAge = batch.age; expect(initialAge).toBe(0); // Advance timer instead of waiting timer.advanceTime(10); // Check age increased expect(batch.age).toBeGreaterThan(initialAge); expect(batch.age).toBe(10); // Advance more timer.advanceTime(15); expect(batch.age).toBe(25); }); }); }); ``` ## Phase 4: Fix TransportTester Tests ### Step 1: Update TransportTester Class ```typescript // libs/network/src/transport/pdk/transport-testing.ts import { TimerController } from '../../utils/timer-controller'; export interface TestOptions { timerController?: TimerController; } export class TransportTester { /** * Test message delivery between two transports */ public static async testMessageDelivery( sender: ITransport, receiver: ITransport, message: any = { test: true }, timeoutMs: number = 1000, options?: TestOptions ): Promise { const timer = options?.timerController || TimerController.getInstance(); let messageReceived = false; // Connect both transports await sender.connect(); await receiver.connect(); // Listen for message received event const onMessageReceived = (msg: any, senderId: string) => { if (senderId === sender.id) { messageReceived = true; } }; receiver.on(TransportEvents.MESSAGE_RECEIVED, onMessageReceived); try { // Send the message await sender.sendTo(receiver.id, message); // Instead of a timeout, use timer advancement if (!messageReceived) { timer.advanceTime(timeoutMs); await timer.waitForPromises(); } return messageReceived; } catch (error) { console.error('Error in testMessageDelivery:', error); return false; } finally { // Clean up receiver.off(TransportEvents.MESSAGE_RECEIVED, onMessageReceived); } } // Update other methods similarly } ``` ### Step 2: Update MockTransport for Tests ```typescript // Update in transport-testing.spec.ts class MockTransport implements ITransport { private _timer: TimerController; constructor( id: string, eventEmitter?: NetworkEventEmitter, timerController?: TimerController ) { this.id = id; this._eventEmitter = eventEmitter || new NetworkEventEmitter(); this._timer = timerController || TimerController.getInstance(); } // Update delay method private async _delay(ms: number): Promise { if (ms <= 0) return; return new Promise(resolve => { this._timer.setTimeout(() => resolve(), ms); this._timer.advanceTime(ms); }); } } ``` ### Step 3: Update TransportTester Tests ```typescript // libs/network/src/transport/pdk/transport-testing.spec.ts describe('TransportTester', () => { let sender: MockTransport; let receiver: MockTransport; let timer: TimerController; beforeEach(() => { timer = TimerController.createInstance(); sender = new MockTransport('sender', undefined, timer); receiver = new MockTransport('receiver', undefined, timer); }); afterEach(() => { timer.reset(); }); // Update time-sensitive tests describe('testMessageDelivery', () => { it('should return true when message is delivered', async () => { // Setup receiver to simulate receiving messages sender.on(TransportEvents.MESSAGE_SENT, (msg) => { receiver.simulateMessageReceived(msg, sender.id); }); const result = await TransportTester.testMessageDelivery( sender, receiver, { test: true }, 1000, { timerController: timer } ); expect(result).toBe(true); }); }); }); ``` ## Phase 5: Fix NetworkNodePhase4 Tests Ensure proper cleanup in tests and use isolated timer instances: ```typescript // libs/network/src/node/network-node-phase4.spec.ts describe('NetworkNodePhase4', () => { let nodePhase4: NetworkNodePhase4; let eventEmitter: NetworkEventEmitter; let timer: TimerController; beforeEach(() => { // Reset any shared state MemoryTransport.resetRegistry(); // Create isolated timer timer = TimerController.createInstance(); // Create a new event emitter for each test eventEmitter = new NetworkEventEmitter(); // Reset mocks jest.clearAllMocks(); // Create the Phase 4 node extension with timer nodePhase4 = new NetworkNodePhase4(mockNode, eventEmitter, { timerController: timer }); }); afterEach(() => { nodePhase4.dispose(); timer.reset(); MemoryTransport.resetRegistry(); }); // Update time-sensitive tests }); ``` ## Phase 6: Fix VotingManager Tests Update the VotingManager to use TimerController: ```typescript // libs/network/src/voting/voting-manager.spec.ts describe('VotingManager', () => { let votingManager: VotingManager; let eventEmitter: NetworkEventEmitter; let timer: TimerController; beforeEach(() => { // Create isolated timer timer = TimerController.createInstance(); // Create a new event emitter for each test eventEmitter = new NetworkEventEmitter(); // Reset mocks jest.clearAllMocks(); // Set test environment variable process.env['NODE_ENV'] = 'test'; // Create the voting manager with timer votingManager = new VotingManager( mockNode, eventEmitter, mockMetricsManager, { timerController: timer } ); }); afterEach(() => { votingManager.dispose(); timer.reset(); }); // Update timing-dependent tests }); ``` ## Final Steps 1. After implementing these changes, run individual test suites to verify they still pass 2. Run the full test suite to ensure isolation issues are resolved 3. Document the approach in the project README to ensure future tests maintain proper isolation By systematically implementing these changes, we will ensure proper test isolation and eliminate the test failures that occur when running the full test suite.