test(statusline): add buildSlotsLine coverage#146
Conversation
Adds 9 tests covering the quorum-slots row added in #141. Coverage: TC30: provider in providers.json but NOT mcpServers → · dim TC31: registered + recent OK probe → green ● TC32: registered + recent failed probe → red ⊘ TC33: registered + no health cache → hollow ○ TC34: registered + STALE (>5min) probe → ○ (stale OK doesn't render green) TC35: providers.json absent → no slots line emitted (fail-silent) TC36: multiple providers preserve declaration order (not alphabetized) TC37: malformed slot-health.json → fail-open to ○ (no crash) TC38: slots line renders ABOVE the tools line (coderlm/River/embed) A `setupSlotsHome` helper lays down all three files (providers.json, .claude.json mcpServers, slot-health.json) in the temp HOME so each scenario is hermetic. Closes the follow-up Copilot flagged on #141 ("buildSlotsLine isn't covered by the existing test suite"). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughThe pull request expands the test suite for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 7/10 reviews remaining, refill in 16 minutes and 2 seconds. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hooks/nf-statusline.test.js (1)
744-763: ⚡ Quick winTighten the “no slots line” assertion to be line-structural, not regex-inferential.
This check can miss regressions if slot names or formatting change while still emitting a slots line. Prefer asserting exact non-empty line count and that the intermediate slots line is absent.
Proposed assertion hardening
- const lines = stdout.split('\n').filter(l => l.length > 0); - // Last line should be the tools line. Everything else is the main line. No middle slots line. - const hasSlotsLine = lines.some(l => /[·●⊘○] [a-z]+-\d+/.test(l) && !l.includes('coderlm')); - assert.ok(!hasSlotsLine, 'no slots line should be emitted when providers.json is absent'); + const lines = stdout.split('\n').filter(Boolean); + // main status line + tools line only (no middle slots line) + assert.ok(lines.length <= 2, `expected no slots line; got ${lines.length} non-empty lines`); + if (lines.length === 2) { + assert.ok(lines[1].includes('coderlm') || lines[1].includes('River') || lines[1].includes('embed'), + `second line should be tools line; got: ${JSON.stringify(lines[1])}`); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/nf-statusline.test.js` around lines 744 - 763, The current regex-based check for a missing slots line is brittle; instead assert the exact line structure returned by runHook: after splitting stdout into non-empty lines (the existing variable lines), assert there are exactly 2 lines (main statusline and tools line) and then assert that the second line (lines[1]) contains the expected tool indicators (e.g., 'coderlm'/'River'/'embed') while the first line does not contain any slot glyphs (·, ●, ⊘, ○); update the test using the existing tempHome, tempDir, runHook and lines variables to perform these structural assertions rather than the regex-based hasSlotsLine check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hooks/nf-statusline.test.js`:
- Around line 744-763: The current regex-based check for a missing slots line is
brittle; instead assert the exact line structure returned by runHook: after
splitting stdout into non-empty lines (the existing variable lines), assert
there are exactly 2 lines (main statusline and tools line) and then assert that
the second line (lines[1]) contains the expected tool indicators (e.g.,
'coderlm'/'River'/'embed') while the first line does not contain any slot glyphs
(·, ●, ⊘, ○); update the test using the existing tempHome, tempDir, runHook and
lines variables to perform these structural assertions rather than the
regex-based hasSlotsLine check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bd04c016-b92d-4f54-a50b-3bbdb5cfe866
📒 Files selected for processing (1)
hooks/nf-statusline.test.js
There was a problem hiding this comment.
Pull request overview
Adds missing unit test coverage for the quorum-slots status row (buildSlotsLine) introduced in PR #141, ensuring the statusline renders correct glyphs across provider registration and health-cache scenarios.
Changes:
- Added 9 new test cases (TC30–TC38) to validate glyph selection, cache freshness behavior, ordering, and line placement.
- Introduced a
setupSlotsHomehelper to create hermetic HOME fixtures forproviders.json,~/.claude.json(mcpServers), andslot-health.json. - Expanded coverage for fail-open / fail-silent behaviors (missing
providers.json, malformedslot-health.json).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Expands the nf-statusline hook’s test suite to cover the quorum-slots status row introduced in PR #141, ensuring correct glyph rendering and layout behavior across a range of slot registration/health-cache scenarios.
Changes:
- Added
setupSlotsHometest helper to create hermetic HOME-based fixtures (providers.json,~/.claude.json,slot-health.json). - Added 9 new test cases (TC30–TC38) covering slot registration, probe freshness, malformed cache handling, ordering, and row positioning.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adds 9 tests covering the quorum-slots row introduced in #141. Closes the follow-up Copilot review thread that asked for buildSlotsLine test coverage and was acknowledged as a follow-up at the time.
What's covered
A new
setupSlotsHomehelper lays down all three files (providers.json,~/.claude.jsonmcpServers,slot-health.json) in a temp HOME so each scenario is hermetic and CI-portable.Test plan
Summary by CodeRabbit