Current Coverage
TransportManager has comprehensive basic test coverage (13 tests), but is missing some edge case scenarios that could help catch race conditions and error handling issues.
Current tests: packages/transport/src/manager.test.ts
- ✅ Transport pooling by configuration
- ✅ Mutex serialization for RTU operations
- ✅ Separate mutexes for different connections
- ✅ Basic error propagation
- ✅ Lifecycle management
Missing Edge Cases
1. Concurrent Operations on Shared RTU Transport
What to test: Verify that multiple concurrent operations on the same RTU bus are actually serialized by the mutex, not just pooled.
Test scenario:
test('should serialize concurrent read operations on shared RTU transport', async () => {
const config: RTUConfig = { /* same config */ }
const transport1 = await manager.getTransport(config)
const transport2 = await manager.getTransport(config)
// Start multiple operations concurrently
const results = await Promise.all([
transport1.readHoldingRegisters(0, 10),
transport2.readHoldingRegisters(10, 10),
transport1.readInputRegisters(20, 10),
])
// Verify operations executed in sequence (mock can track call order)
expect(executionOrder).toEqual(['op1-start', 'op1-end', 'op2-start', ...])
})
2. Transport Failure During Operation
What to test: What happens when the underlying transport throws an error mid-operation?
Test scenario:
test('should propagate transport errors through mutex wrapper', async () => {
mockTransport.readHoldingRegisters.mockRejectedValue(new Error('Connection lost'))
const transport = await manager.getTransport(config)
await expect(transport.readHoldingRegisters(0, 10)).rejects.toThrow('Connection lost')
// Verify mutex was released and subsequent operations can proceed
mockTransport.readHoldingRegisters.mockResolvedValue(Buffer.alloc(20))
await expect(transport.readHoldingRegisters(0, 10)).resolves.toBeDefined()
})
3. Close During Active Operation
What to test: Race condition when closeAll() is called while operations are in progress.
Test scenario:
test('should handle closeAll during active operations', async () => {
const transport = await manager.getTransport(config)
// Start long-running operation
const slowOperation = transport.readHoldingRegisters(0, 100)
// Close while operation is running
const closePromise = manager.closeAll()
// Both should complete without deadlock
await Promise.race([
slowOperation.catch(() => {}), // May fail or succeed
closePromise
])
})
4. Multiple Devices Adding/Removing Concurrently
What to test: Concurrent getTransport() and closeAll() calls don't corrupt internal state.
Test scenario:
test('should handle concurrent getTransport calls safely', async () => {
const promises = Array.from({ length: 10 }, (_, i) =>
manager.getTransport({
port: '/dev/ttyUSB0',
baudRate: 9600,
/* ... */
slaveId: i // Different slave IDs, same bus
})
)
const transports = await Promise.all(promises)
// All should get the same pooled transport
expect(new Set(transports).size).toBe(1)
expect(manager.getStats().rtuTransports).toBe(1)
})
Benefits
- Confidence in mutex behavior: Explicit tests prove operations are truly serialized
- Error recovery: Ensures system recovers gracefully from transport failures
- Race condition detection: Catches potential deadlocks or state corruption
- Documentation: Tests serve as examples of expected behavior under stress
Priority
Low-Medium - Current tests are good, these are "nice to have" for increased confidence
Related
From Claude Code Review of PR #125
Current Coverage
TransportManager has comprehensive basic test coverage (13 tests), but is missing some edge case scenarios that could help catch race conditions and error handling issues.
Current tests:
packages/transport/src/manager.test.tsMissing Edge Cases
1. Concurrent Operations on Shared RTU Transport
What to test: Verify that multiple concurrent operations on the same RTU bus are actually serialized by the mutex, not just pooled.
Test scenario:
2. Transport Failure During Operation
What to test: What happens when the underlying transport throws an error mid-operation?
Test scenario:
3. Close During Active Operation
What to test: Race condition when
closeAll()is called while operations are in progress.Test scenario:
4. Multiple Devices Adding/Removing Concurrently
What to test: Concurrent
getTransport()andcloseAll()calls don't corrupt internal state.Test scenario:
Benefits
Priority
Low-Medium - Current tests are good, these are "nice to have" for increased confidence
Related
packages/transport/src/manager.test.tsFrom Claude Code Review of PR #125