test(doctor): add doctor module coverage - #686
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis pull request adds comprehensive Jest test suites for the Doctor diagnostic system. Three test files validate individual health checks (covering Node version, config files, rules, agents, git hooks, skills, commands, and IDE sync), the orchestration layer (versioning, output formatting, fix modes), and the auto-fix handler (status filtering, check-specific remediation, dry-run behavior). All tests use isolated temporary directories for filesystem operations and make no changes to public APIs. ChangesDoctor System Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Coverage ReportCoverage report not available
Generated by PR Automation (Story 6.1) |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/core/doctor/fix-handler.test.js (1)
160-175: ⚡ Quick winStrengthen
settings-jsonfixer assertions.This test only checks
fixResults[0].check, so it can pass even if the fixer no longer applies/returns expected behavior. Add assertions onappliedand/or expected message pattern (similar to other fixer tests).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/core/doctor/fix-handler.test.js` around lines 160 - 175, The test currently only asserts fixResults[0].check and can pass even if the fixer didn't actually apply; update the test that calls applyFixes(results, context) to also assert expected behavior for the 'settings-json' fixer by checking either fixResults[0].applied === true (when generator ran) or that fixResults[0].message matches the expected install/force pattern used by other fixer tests (e.g., contains "install --force" or similar); ensure you reference the same result object (fixResults[0]) and keep the existing check assertion for 'settings-json'.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/core/doctor/doctor-checks.test.js`:
- Around line 45-56: The test "deve retornar PASS quando Node >= 18" currently
only asserts when major >= 18, allowing a silent pass on older Node versions;
update the test around the run() call and major variable to include an else
branch that asserts the expected behavior when major < 18 (e.g. check
result.status is the failing value, result.check equals 'node-version',
result.message mentions Node.js, and that result.fixCommand is set/not null), or
replace the conditional with explicit assertions for both branches so the test
always validates the result variable.
- Line 39: The test uses relative requires for Doctor check modules (e.g.,
destructuring { run, name } from node-version) which violates the
absolute-import rule; replace each relative require with the project's absolute
import for the corresponding Doctor check module and update all other
occurrences in this file (the imports that provide run and name for each check).
Locate the require calls that import check modules (they destructure run and
name) and change them to use the project's absolute import style consistently
for every check import mentioned in the review.
In `@tests/core/doctor/doctor-index.test.js`:
- Line 16: The test imports runDoctorChecks and DOCTOR_VERSION via a relative
path into .aiox-core; change that require to the repo's absolute import for the
core doctor module (e.g., import the module from the absolute package name such
as aiox-core/core/doctor) so runDoctorChecks and DOCTOR_VERSION are loaded via
the project's absolute import convention instead of
"../../../.aiox-core/core/doctor".
In `@tests/core/doctor/fix-handler.test.js`:
- Around line 16-17: The test uses relative paths to import applyFixes and
EXPECTED_RULES; replace those relative requires with the project's absolute
module paths (importing the same symbols applyFixes and EXPECTED_RULES) so the
test follows the repository's absolute import convention—update the require
calls that reference '../../../.aiox-core/core/doctor/fix-handler' and
'../../../.aiox-core/core/doctor/checks/rules-files' to their equivalent
absolute import module specifiers while keeping the imported identifiers
applyFixes and EXPECTED_RULES unchanged.
---
Nitpick comments:
In `@tests/core/doctor/fix-handler.test.js`:
- Around line 160-175: The test currently only asserts fixResults[0].check and
can pass even if the fixer didn't actually apply; update the test that calls
applyFixes(results, context) to also assert expected behavior for the
'settings-json' fixer by checking either fixResults[0].applied === true (when
generator ran) or that fixResults[0].message matches the expected install/force
pattern used by other fixer tests (e.g., contains "install --force" or similar);
ensure you reference the same result object (fixResults[0]) and keep the
existing check assertion for 'settings-json'.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c23d5e2-833d-467a-a4e5-85fe0f996c2e
📒 Files selected for processing (3)
tests/core/doctor/doctor-checks.test.jstests/core/doctor/doctor-index.test.jstests/core/doctor/fix-handler.test.js
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…rchestrator (#52) Cobertura completa dos 10 checks modulares (node-version, core-config, rules-files, agent-memory, entity-registry, git-hooks, skills-count, commands-count, hooks-claude-count, settings-json, ide-sync), do fix-handler (dry-run, apply, erros) e do orchestrator (json, quiet, fix).
54eab23 to
db2fb44
Compare
Summary
Supersedes #596 by porting the doctor module coverage onto current
mainand updating theide-syncassertions for the current skills-first contract.Adds coverage for:
Validation
npm test -- tests/core/doctor --runInBand --forceExitnpm run typechecknpm run lint(passes with existing warnings)git diff --check origin/main...HEADPart of #52.
Summary by CodeRabbit