test: add usage-validation coverage for cli/run.js#350
Conversation
`supercli run <plugin> <resource> <action>` (issue #335) had no dedicated test file. Cover the three early-return usage-error paths (missing plugin, missing resource+action, missing action only) so future edits to the argument-validation logic get regression coverage.
A local `npm install` run to verify the previous commit's tests rewrote package-lock.json because package.json currently declares no dependencies (a pre-existing regression from c10ee2b, out of scope here). Restore the lockfile to its prior committed state.
cli/skills-mcp.js (buildMcpServersUsageSkillMarkdown) had no test file even though it's a pure, easily-tested function. Cover the default frontmatter shape, the showDag toggle, and the custom skillId option.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
🏛️ Automaintainer architecture review: VIOLATION The new files live at top-level The change passed verification, but it violates this repo's architecture rules. The PR was set to draft and auto-merge is held — review before marking it ready. |
📝 WalkthroughWalkthroughAdds Jest tests for ChangesTest coverage
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
Automaintainer Review Verdict: APPROVED — Both test files are well-structured, focused on early-return validation paths, and follow the same |
|
Approved by automaintainer review team — ready for human merge |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
__tests__/run-command.test.js (1)
13-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify the mocked
executeis not called in each test.The
executemock is set up but never asserted. Addingexpect(execute).not.toHaveBeenCalled()to each test would confirm the usage-validation paths return before any execution attempt, closing a gap where a missing early-return could go undetected.♻️ Proposed addition
jest.mock("../cli/executor", () => ({ execute: jest.fn() })); -const { handleRunCommand } = require("../cli/run"); +const { handleRunCommand } = require("../cli/run"); +const { execute } = require("../cli/executor"); + +afterEach(() => { + jest.clearAllMocks(); +});Then add to each test, after the existing
expect(output).not.toHaveBeenCalled()line:expect(output).not.toHaveBeenCalled(); + expect(execute).not.toHaveBeenCalled(); expect(outputError).toHaveBeenCalledWith(🤖 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__/run-command.test.js` around lines 13 - 15, Update every test in __tests__/run-command.test.js covering usage validation to assert the mocked execute function was not called, adding the assertion after the existing output-not-called expectation. Import or reference the execute mock from the ../cli/executor mock and preserve the current test behavior.
🤖 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__/run-command.test.js`:
- Around line 13-15: Update every test in __tests__/run-command.test.js covering
usage validation to assert the mocked execute function was not called, adding
the assertion after the existing output-not-called expectation. Import or
reference the execute mock from the ../cli/executor mock and preserve the
current test behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d945660-9788-4aee-8341-0728aa9921f4
📒 Files selected for processing (2)
__tests__/run-command.test.js__tests__/skills-mcp.test.js
1 participant
Automated maintenance run by automaintainer.
Focus:
WORK ALREADY IN FLIGHT — do not overlap:
These automaintainer pull requests are already open and awaiting review. Do NOT re-implement, refactor, or restructure the files they touch — pick non-overlapping work, and never recreate a change an open PR already makes. If your objective unavoidably overlaps one of these, choose a different, complementary improvement instead.
touches: tests/chainloop-plugin.test.js, tests/overmind-plugin.test.js, tests/resvg-plugin.test.js
touches: tests/amass-plugin.test.js, tests/goss-plugin.test.js, tests/hivemind-plugin.test.js, tests/mods-plugin.test.js, tests/tlrc-plugin.test.js
touches: tests/komiser-plugin.test.js, tests/upx-plugin.test.js, tests/wtfutil-plugin.test.js
touches: tests/coder-plugin.test.js, tests/pkl-plugin.test.js, tests/rbspy-plugin.test.js
touches: tests/diun-plugin.test.js, tests/notion-cli-plugin.test.js, tests/slim-plugin.test.js
touches: tests/help.test.js, tests/mcp-diagnostics.test.js, package.json
touches: plugins/atac/install-guidance.json, plugins/atac/meta.json, plugins/atac/plugin.json, plugins/dolt/install-guidance.json, plugins/dolt/meta.json, plugins/dolt/plugin.json, plugins/otree/install-guidance.json, plugins/otree/meta.json, plugins/otree/plugin.json, plugins/ov/install-guidance.json, plugins/ov/meta.json, plugins/ov/plugin.json (+3 more)
Branch:
am/am-f17c27-ti4digDiff:
Summary by CodeRabbit