fix: isolate CodeRabbit unit tests from WSL probe on Windows#809
fix: isolate CodeRabbit unit tests from WSL probe on Windows#809grupogegedelivery-cpu wants to merge 1 commit into
Conversation
- Mock child_process.spawnSync in layer2-pr-automation.test.js and quality-gate-pipeline.test.js so runCodeRabbit's WSL availability check does not run for real, letting the mocked runCommand() control test outcomes instead of the host environment. - Replace bash-specific `|| true` in aiox-install integration test with try/catch around execSync, since cmd.exe on Windows does not understand that syntax. - chore: npm audit fix (60 packages updated, 0 vulnerabilities) - chore: npm approve-scripts for unrs-resolver and @aiox-squads/core - chore: entity-registry re-sync (auto-generated timestamps)
|
@grupogegedelivery-cpu is attempting to deploy a commit to the SINKRA - AIOX Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
type: test
|
Welcome to aiox-core! Thanks for your first pull request. What happens next?
PR Checklist:
Thanks for contributing! |
WalkthroughRegistry and install-manifest metadata were refreshed, package script permissions were added, and tests were updated to isolate ChangesCore metadata and test execution
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/packages/aiox-install/integration.test.js (1)
181-189: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse
execFileSyncto prevent path escaping vulnerabilities.Passing dynamic variables like
binPathdirectly into a shell command string usingexecSynccan lead to path escaping issues or command injection. Since the PR focuses on Windows test compatibility, it is especially important to avoid shell strings, as Windows paths frequently contain spaces that can break unescaped shell execution. As per path instructions, look for potential security vulnerabilities.Switching to
execFileSyncbypasses the shell entirely, making it safer. The shell-specific2>&1redirection can be removed entirely because thetry/catchblock already accurately aggregateserror.stdoutanderror.stderrfrom the exception.♻️ Proposed refactor
Update the execution block to use
execFileSync(ensureexecFileSyncis imported fromchild_processif not already available):- try { - result = execSync(`node "${binPath}" --invalid-flag 2>&1`, { - encoding: 'utf8', - timeout: 10000, - }); - } catch (error) { - result = (error.stdout || '') + (error.stderr || ''); - } + try { + result = require('child_process').execFileSync('node', [binPath, '--invalid-flag'], { + encoding: 'utf8', + timeout: 10000, + }); + } catch (error) { + result = (error.stdout || '') + (error.stderr || ''); + }🤖 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/packages/aiox-install/integration.test.js` around lines 181 - 189, Replace the execSync invocation in the invalid-flag execution test with execFileSync, passing binPath and --invalid-flag as separate arguments and removing the shell-specific 2>&1 redirection. Ensure execFileSync is imported from child_process, while preserving the existing timeout, encoding, and catch-block aggregation of stdout and stderr.Sources: Path instructions, Linters/SAST tools
tests/integration/quality-gate-pipeline.test.js (1)
10-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse absolute imports instead of relative paths.
As per coding guidelines, use absolute imports in JavaScript and TypeScript source code. These imports use extensive relative path traversal (
../../and../../../) to reach the.aiox-coredirectory. Utilizing absolute imports prevents reference breakages during file moves.
tests/integration/quality-gate-pipeline.test.js#L10-L14: Update theserequirestatements to use absolute import paths (e.g., using configured path aliases).tests/unit/quality-gates/layer2-pr-automation.test.js#L8-L8: Update thisrequirestatement to use an absolute import path.🤖 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/integration/quality-gate-pipeline.test.js` around lines 10 - 14, Replace the relative require paths in tests/integration/quality-gate-pipeline.test.js lines 10-14 for QualityGateManager, Layer1PreCommit, Layer2PRAutomation, Layer3HumanReview, and ChecklistGenerator with configured absolute import paths. Also update the require at tests/unit/quality-gates/layer2-pr-automation.test.js line 8 to use the corresponding absolute path; preserve the imported symbols and test behavior.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@tests/integration/quality-gate-pipeline.test.js`:
- Around line 10-14: Replace the relative require paths in
tests/integration/quality-gate-pipeline.test.js lines 10-14 for
QualityGateManager, Layer1PreCommit, Layer2PRAutomation, Layer3HumanReview, and
ChecklistGenerator with configured absolute import paths. Also update the
require at tests/unit/quality-gates/layer2-pr-automation.test.js line 8 to use
the corresponding absolute path; preserve the imported symbols and test
behavior.
In `@tests/packages/aiox-install/integration.test.js`:
- Around line 181-189: Replace the execSync invocation in the invalid-flag
execution test with execFileSync, passing binPath and --invalid-flag as separate
arguments and removing the shell-specific 2>&1 redirection. Ensure execFileSync
is imported from child_process, while preserving the existing timeout, encoding,
and catch-block aggregation of stdout and stderr.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c0155d9b-93cb-4b59-8ee8-e6c9948bba31
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.aiox-core/data/entity-registry.yaml.aiox-core/install-manifest.yamlpackage.jsontests/integration/quality-gate-pipeline.test.jstests/packages/aiox-install/integration.test.jstests/unit/quality-gates/layer2-pr-automation.test.js
1 participant
Summary
child_process.spawnSyncinlayer2-pr-automation.test.jsandquality-gate-pipeline.test.jssorunCodeRabbit's WSL availability probe does not run for real on Windows hosts, letting the mockedrunCommand()control test outcomes instead of the host environment.|| truein theaiox-installintegration test with atry/catcharoundexecSync, sincecmd.exeon Windows does not understand that syntax.Why
On a Windows host without WSL installed,
npm run testfailed 10 tests across 3 suites:Layer2PRAutomation(4 tests) andQuality Gate Pipeline Integration/Smoke Tests(5 tests) failed becauserunCodeRabbit()performs a realspawnSync(\'wsl\', [\'-l\'])probe before ever reaching the mockedrunCommand(), so tests that mockrunCommandstill hit the real (failing) WSL check.aiox-install integration.test.jsfailed because it shells out withnode ... || true, a bash idiom not understood bycmd.exe(Windows' defaultexecSyncshell).Testing
npm run test: 375/375 suites passed (12 skipped), 9002/9174 tests passed (172 skipped), exit code 0.Additional changes bundled in this branch
npm audit fix(60 packages updated, 0 vulnerabilities remaining)npm approve-scriptsforunrs-resolverand@aiox-squads/core.aiox-core/data/entity-registry.yamland.aiox-core/install-manifest.yamlauto-regenerated by the repo's own pre-commit/pre-push hooks (timestamps only)Summary by CodeRabbit
Chores
Tests