test(doctor): 71 testes unitários para módulos doctor (#52)#596
Open
nikolasdehor wants to merge 1 commit intoSynkraAI:mainfrom
Open
test(doctor): 71 testes unitários para módulos doctor (#52)#596nikolasdehor wants to merge 1 commit intoSynkraAI:mainfrom
nikolasdehor wants to merge 1 commit intoSynkraAI:mainfrom
Conversation
…rchestrator (SynkraAI#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).
nikolasdehor
requested review from
Pedrovaleriolopez and
oalanicolas
as code owners
|
@nikolasdehor is attempting to deploy a commit to the Pedro Valério Lopez's projects Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
bot
added
area: agents
WalkthroughThree new test suites are introduced for the Doctor system: comprehensive tests for individual checks (node-version, core-config, rules-files, etc.), the Doctor Index orchestrator with version and output formatting, and the Fix Handler for applying and validating remediation operations across various scenarios and configurations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 unit tests (beta)
📝 Coding Plan
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (8)
tests/core/doctor/fix-handler.test.js (2)
16-17: Use absolute imports and remove unused import.The imports use relative paths, and
EXPECTED_RULESappears unused in this test file.♻️ Suggested change
-const { applyFixes } = require('../../../.aiox-core/core/doctor/fix-handler'); -const { EXPECTED_RULES } = require('../../../.aiox-core/core/doctor/checks/rules-files'); +const { applyFixes } = require('@aiox-core/core/doctor/fix-handler');As per coding guidelines: "Use absolute imports instead of relative imports in all code".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/doctor/fix-handler.test.js` around lines 16 - 17, The test file imports use relative paths and include an unused import EXPECTED_RULES; update the imports to use absolute module paths and remove the unused EXPECTED_RULES import. Specifically, replace the relative require for applyFixes with the project's absolute import path for the .aiox-core core/doctor/fix-handler module (referencing applyFixes) and delete the line that requires EXPECTED_RULES so only the needed applyFixes symbol is imported.
160-175: Test assertion is weak — only verifies check name.This test validates that
fixResults[0].checkequals'settings-json'but doesn't assert onappliedormessage. The comment on line 173 acknowledges uncertainty, but this makes the test less useful for regression detection.♻️ Consider strengthening the assertion
const fixResults = await applyFixes(results, context); expect(fixResults).toHaveLength(1); -// Pode ser applied=true (com generator) ou message com install --force expect(fixResults[0].check).toBe('settings-json'); +expect(typeof fixResults[0].applied).toBe('boolean'); +expect(typeof fixResults[0].message).toBe('string');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/doctor/fix-handler.test.js` around lines 160 - 175, The test only checks fixResults[0].check and is too weak; update the assertion to validate whether the fix actually applied or produced a clear message: after calling applyFixes(results, context) assert that fixResults has length 1 and then check fixResults[0] for either applied === true (indicating the generator ran) or that fixResults[0].message contains the expected guidance (e.g., 'install --force' or other known instruction). Use the existing variables fixResults, applyFixes, results and context to locate the test and add a conditional or explicit assertions to cover both possible outcomes.tests/core/doctor/doctor-index.test.js (3)
101-106: Test usingcwd()may be environment-dependent.This test omits
projectRoot, defaulting toprocess.cwd(). In CI or different working directories, this could yield varying results. The test passes currently, but consider documenting this dependency or explicitly settingprojectRootfor deterministic behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/doctor/doctor-index.test.js` around lines 101 - 106, The test relies on process.cwd() via runDoctorChecks() which is environment-dependent; make it deterministic by explicitly providing a known projectRoot when calling runDoctorChecks (or temporarily set process.cwd() to a fixture), e.g., call runDoctorChecks({ projectRoot: pathToTestFixture }) or wrap the test in a try/finally that changes cwd and restores it; update the test invoking runDoctorChecks and reference the fixture path so the expectations on data.checks remain stable across environments.
33-35: Hardcoded version may cause test brittleness.The test directly asserts
'2.0.0'which will fail wheneverDOCTOR_VERSIONis bumped. Consider verifying the format (semver) or comparing against the exported constant dynamically if the goal is just to ensure the export exists and has a valid value.♻️ Alternative approach
it('deve exportar DOCTOR_VERSION', () => { - expect(DOCTOR_VERSION).toBe('2.0.0'); + expect(typeof DOCTOR_VERSION).toBe('string'); + expect(DOCTOR_VERSION).toMatch(/^\d+\.\d+\.\d+$/); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/doctor/doctor-index.test.js` around lines 33 - 35, The test currently hardcodes '2.0.0' which will break when DOCTOR_VERSION is bumped; update the test for DOCTOR_VERSION to assert a valid semver format instead of a specific value—e.g., use a semver regex or the semver library to assert semver.valid(DOCTOR_VERSION) or expect(DOCTOR_VERSION).toMatch(/^\d+\.\d+\.\d+(-.+)?$/)—so the test verifies DOCTOR_VERSION exists and is a valid semver without depending on a fixed version string.
16-16: Use absolute imports instead of relative imports.The import uses a relative path which violates the project's coding guidelines.
♻️ Suggested change
Consider configuring Jest's
moduleNameMapperor using a path alias to enable absolute imports:-const { runDoctorChecks, DOCTOR_VERSION } = require('../../../.aiox-core/core/doctor'); +const { runDoctorChecks, DOCTOR_VERSION } = require('@aiox-core/core/doctor');As per coding guidelines: "Use absolute imports instead of relative imports in all code".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/doctor/doctor-index.test.js` at line 16, Replace the relative require "../../../.aiox-core/core/doctor" in the test that imports runDoctorChecks and DOCTOR_VERSION with an absolute import using your project's configured path alias (e.g., "core/doctor" or "@aiox-core/core/doctor"); then update Jest's configuration (moduleNameMapper) to map that alias to the actual .aiox-core folder so tests resolve the module, and ensure the require/require-equivalent in the test is updated to use the alias for runDoctorChecks and DOCTOR_VERSION.tests/core/doctor/doctor-checks.test.js (3)
39-39: Use absolute imports for all check modules.All
requirestatements for doctor checks use relative paths. Consider configuring module aliases to enable absolute imports across the test suite.Example transformation:
-const { run, name } = require('../../../.aiox-core/core/doctor/checks/node-version'); +const { run, name } = require('@aiox-core/core/doctor/checks/node-version');As per coding guidelines: "Use absolute imports instead of relative imports in all code".
Also applies to: 64-64, 126-126, 198-198, 262-262, 320-320, 375-375, 465-465, 557-557, 656-656, 733-733
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/doctor/doctor-checks.test.js` at line 39, The test imports doctor check modules with relative paths (e.g., require('../../../.aiox-core/core/doctor/checks/node-version') in doctor-checks.test.js); change these to the project's absolute module alias (e.g., require('@aiox-core/core/doctor/checks/node-version') or the alias your repo uses) for all occurrences (lines referenced in the review). Update the require calls for the symbols { run, name } and the other listed check imports to use the absolute alias, and if necessary adjust the test resolver/moduleNameMapper or module-alias config so Jest/node can resolve the new absolute paths.
45-56: Conditional assertion may silently pass without testing.The
if (major >= 18)block means that if the test runs on Node < 18, no assertions execute and the test passes silently. Consider usingexpect.assertions(n)or restructuring to ensure assertions always run, or add an explicit skip/fail for unsupported Node versions.♻️ Suggested approach
it('deve retornar PASS quando Node >= 18', async () => { - // Rodando neste ambiente, Node deve ser >= 18 const result = await run(); const major = parseInt(process.version.replace('v', '').split('.')[0], 10); - if (major >= 18) { - expect(result.status).toBe('PASS'); - expect(result.check).toBe('node-version'); - expect(result.message).toContain('Node.js'); - expect(result.fixCommand).toBeNull(); - } + // This test suite requires Node >= 18 + expect(major).toBeGreaterThanOrEqual(18); + expect(result.status).toBe('PASS'); + expect(result.check).toBe('node-version'); + expect(result.message).toContain('Node.js'); + expect(result.fixCommand).toBeNull(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/doctor/doctor-checks.test.js` around lines 45 - 56, The test's conditional "if (major >= 18)" causes assertions to be skipped on Node < 18; change the test so assertions always run or the test explicitly fails/skips on unsupported Node versions: compute major via parseInt(process.version...), call run() and then either (a) add an explicit guard that throws or calls test.skip/fail when major < 18, or (b) use expect.assertions(n) and restructure to assert the expected PASS behavior for major >= 18 and assert a specific failure/skip outcome for major < 18; update references to run(), result, major and the test case description accordingly so the test cannot silently pass without executing assertions.
4-6: Doc comment lists 11 checks but claims 15.The JSDoc mentions "15 checks modulares" but only lists 11 names. Consider updating the comment to match the actual count.
♻️ Suggested fix
-* Cobre todos os 15 checks modulares: node-version, core-config, +* Cobre todos os 11 checks modulares: node-version, core-config, * rules-files, agent-memory, entity-registry, git-hooks, ide-sync, * settings-json, skills-count, commands-count, hooks-claude-count.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/doctor/doctor-checks.test.js` around lines 4 - 6, Update the JSDoc block that currently says "15 checks modulares" to accurately reflect the checks listed: either change the number to 11 or add the missing 4 check names so the count matches; edit the JSDoc comment in the doctor-checks.test.js header (the JSDoc listing node-version, core-config, rules-files, agent-memory, entity-registry, git-hooks, ide-sync, settings-json, skills-count, commands-count, hooks-claude-count) to keep the written number and the enumerated list consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/core/doctor/doctor-checks.test.js`:
- Line 39: The test imports doctor check modules with relative paths (e.g.,
require('../../../.aiox-core/core/doctor/checks/node-version') in
doctor-checks.test.js); change these to the project's absolute module alias
(e.g., require('@aiox-core/core/doctor/checks/node-version') or the alias your
repo uses) for all occurrences (lines referenced in the review). Update the
require calls for the symbols { run, name } and the other listed check imports
to use the absolute alias, and if necessary adjust the test
resolver/moduleNameMapper or module-alias config so Jest/node can resolve the
new absolute paths.
- Around line 45-56: The test's conditional "if (major >= 18)" causes assertions
to be skipped on Node < 18; change the test so assertions always run or the test
explicitly fails/skips on unsupported Node versions: compute major via
parseInt(process.version...), call run() and then either (a) add an explicit
guard that throws or calls test.skip/fail when major < 18, or (b) use
expect.assertions(n) and restructure to assert the expected PASS behavior for
major >= 18 and assert a specific failure/skip outcome for major < 18; update
references to run(), result, major and the test case description accordingly so
the test cannot silently pass without executing assertions.
- Around line 4-6: Update the JSDoc block that currently says "15 checks
modulares" to accurately reflect the checks listed: either change the number to
11 or add the missing 4 check names so the count matches; edit the JSDoc comment
in the doctor-checks.test.js header (the JSDoc listing node-version,
core-config, rules-files, agent-memory, entity-registry, git-hooks, ide-sync,
settings-json, skills-count, commands-count, hooks-claude-count) to keep the
written number and the enumerated list consistent.
In `@tests/core/doctor/doctor-index.test.js`:
- Around line 101-106: The test relies on process.cwd() via runDoctorChecks()
which is environment-dependent; make it deterministic by explicitly providing a
known projectRoot when calling runDoctorChecks (or temporarily set process.cwd()
to a fixture), e.g., call runDoctorChecks({ projectRoot: pathToTestFixture }) or
wrap the test in a try/finally that changes cwd and restores it; update the test
invoking runDoctorChecks and reference the fixture path so the expectations on
data.checks remain stable across environments.
- Around line 33-35: The test currently hardcodes '2.0.0' which will break when
DOCTOR_VERSION is bumped; update the test for DOCTOR_VERSION to assert a valid
semver format instead of a specific value—e.g., use a semver regex or the semver
library to assert semver.valid(DOCTOR_VERSION) or
expect(DOCTOR_VERSION).toMatch(/^\d+\.\d+\.\d+(-.+)?$/)—so the test verifies
DOCTOR_VERSION exists and is a valid semver without depending on a fixed version
string.
- Line 16: Replace the relative require "../../../.aiox-core/core/doctor" in the
test that imports runDoctorChecks and DOCTOR_VERSION with an absolute import
using your project's configured path alias (e.g., "core/doctor" or
"@aiox-core/core/doctor"); then update Jest's configuration (moduleNameMapper)
to map that alias to the actual .aiox-core folder so tests resolve the module,
and ensure the require/require-equivalent in the test is updated to use the
alias for runDoctorChecks and DOCTOR_VERSION.
In `@tests/core/doctor/fix-handler.test.js`:
- Around line 16-17: The test file imports use relative paths and include an
unused import EXPECTED_RULES; update the imports to use absolute module paths
and remove the unused EXPECTED_RULES import. Specifically, replace the relative
require for applyFixes with the project's absolute import path for the
.aiox-core core/doctor/fix-handler module (referencing applyFixes) and delete
the line that requires EXPECTED_RULES so only the needed applyFixes symbol is
imported.
- Around line 160-175: The test only checks fixResults[0].check and is too weak;
update the assertion to validate whether the fix actually applied or produced a
clear message: after calling applyFixes(results, context) assert that fixResults
has length 1 and then check fixResults[0] for either applied === true
(indicating the generator ran) or that fixResults[0].message contains the
expected guidance (e.g., 'install --force' or other known instruction). Use the
existing variables fixResults, applyFixes, results and context to locate the
test and add a conditional or explicit assertions to cover both possible
outcomes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 903026dc-a49f-4ad6-bb7a-824e7e8e6831
📒 Files selected for processing (3)
tests/core/doctor/doctor-checks.test.jstests/core/doctor/doctor-index.test.jstests/core/doctor/fix-handler.test.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resumo
Adiciona 71 testes unitários cobrindo todo o módulo
doctor/:node-version,core-config,rules-files,agent-memory,entity-registry,git-hooks,skills-count,commands-count,hooks-claude-count,settings-json,ide-sync--json,--quiet,--fix,--dry-run, summary counts, tratamento de errosDetalhes
Cada check é testado com:
nameCloses #52 (parcial)
Plano de teste
npx jest tests/core/doctor/— 71/71 testes passandofs.mkdtemppara isolamentoafterEachSummary by CodeRabbit