Skip to content

Commit 2c47648

Browse files
committed
test: Fix error log assertions for EADDRINUSE non-cc server
1 parent 7d9f5bb commit 2c47648

2 files changed

Lines changed: 25 additions & 4 deletions

File tree

RETROSPECTION.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,26 @@
1717
- After applying substantial code changes or refactoring (especially those involving deletion or large replacements), perform a targeted review to ensure that all necessary imports, function calls, and logic blocks are still present and correctly connected.
1818
- Consider if parts of `startServer`'s non-MCP HTTP setup (like EADDRINUSE handling) could be further modularized to improve readability, though its current state is functional.
1919

20+
---
21+
# Retrospection for Unit Test Fix (server.test.ts - EADDRINUSE Non-CodeCompass Server Log Assertions) (Git Commit ID: [GIT_COMMIT_ID_PLACEHOLDER])
22+
23+
## What went well?
24+
- The detailed failure output from Vitest, showing all calls to the mocked `ml.error` function, was instrumental in understanding the discrepancy and formulating the correct assertions.
25+
- The fix involved making the test assertions more precise, which improves test reliability.
26+
27+
## What could be improved?
28+
- **Initial Assertion Granularity:** The original assertion for `ml.error` was too broad. When a function is expected to be called multiple times with different arguments, assertions should ideally verify each call specifically or use matchers that accommodate the variations if a single assertion is intended to cover multiple calls (though the latter is less precise).
29+
- **Log Message Stability:** While not an issue here, if log messages change frequently, tests asserting their exact content can become brittle. Using `expect.stringContaining` for key phrases is a good balance, but the overall sequence of logs still needs to be stable for `toHaveBeenNthCalledWith`.
30+
31+
## What did we learn?
32+
- When testing functions that log multiple distinct messages (especially error messages in different phases of handling an error), it's important to assert each log call accurately.
33+
- Vitest's `toHaveBeenNthCalledWith` is very useful for verifying the arguments of specific calls in a sequence.
34+
- Detailed mock call reporting in test runner output is invaluable for debugging assertion failures.
35+
36+
## Action Items / Follow-ups
37+
- When writing tests that assert log outputs, if multiple log calls are expected, use specific assertions for each call (e.g., `toHaveBeenNthCalledWith`) rather than a single, broad assertion, to improve test precision and debuggability.
38+
- Review other tests that assert multiple log calls to ensure they are using precise assertions.
39+
2040
---
2141
# Retrospection for Unit Test Fix (server.test.ts - Incorrect Assertion) (Git Commit ID: [GIT_COMMIT_ID_PLACEHOLDER])
2242

src/tests/server.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -566,10 +566,11 @@ describe('Server Startup and Port Handling', () => {
566566
})
567567
);
568568

569-
570-
expect(ml.error).toHaveBeenCalledWith(expect.stringContaining(`Port ${mcs.HTTP_PORT} is in use, but it does not appear to be a CodeCompass server.`));
571-
572-
expect(ml.error).toHaveBeenCalledWith(expect.stringContaining('Please free the port or configure a different one'));
569+
// Verify the specific error log calls in order
570+
expect(ml.error).toHaveBeenNthCalledWith(1, expect.stringContaining(`Port ${mcs.HTTP_PORT} is in use, but it does not appear to be a CodeCompass server. Response: {"service":"OtherService"}`));
571+
expect(ml.error).toHaveBeenNthCalledWith(2, expect.stringContaining('Please free the port or configure a different one'));
572+
expect(ml.error).toHaveBeenNthCalledWith(3, "Failed to start CodeCompass", expect.objectContaining({ message: `Port ${mcs.HTTP_PORT} in use by non-CodeCompass server.` }));
573+
573574
expect(mockedMcpServerConnect).not.toHaveBeenCalled();
574575
});
575576

0 commit comments

Comments
 (0)