test(cli-tools): add tests for formatters, prompts, and updateEnvFile#71
test(cli-tools): add tests for formatters, prompts, and updateEnvFile#71ak68a wants to merge 3 commits intoagentcommercekit:mainfrom
Conversation
Cover all utility functions that previously had zero tests: formatters: sectionHeader divider width and step numbering, successMessage/errorMessage prefixes, wordWrap at custom and default widths, demoHeader/demoFooter non-empty output, link URL preservation. update-env-file: create new file, update existing keys in-place, append new keys, preserve comments and blank lines, multiple keys, values containing equals signs. Uses real temp directories. prompts: log with default wrapping, log with wrap disabled, logJson formatted output.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThree new Vitest test suites were added under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/cli-tools/src/formatters.test.ts`:
- Around line 15-93: Several tests use non-assertive titles; update each it(...)
description to follow the mandated assertive prefixes
(creates|throws|requires|returns). For tests referencing sectionHeader, change
"creates a header with dividers matching the message width" to "creates a header
with dividers matching the message width" (already correct) and change "includes
a step number when provided" → "creates a header that includes a step number",
"omits step prefix when no step is given" → "creates a header that omits the
step prefix", for successMessage/errorMessage change "prefixes with a check
mark" → "returns a message prefixed with a check mark" and "prefixes with an X"
→ "returns a message prefixed with an X", for wordWrap change "wraps long text
to the specified width" → "wraps long text to the specified width" (keep) and
"defaults to 80 characters" → "wraps text defaulting to 80 characters", and for
demoHeader/demoFooter/link rename their titles to start with "returns" (e.g.,
"returns a non-empty demo header", "returns a non-empty demo footer", "returns
the URL with formatting applied") so all it(...) descriptions for sectionHeader,
successMessage, errorMessage, wordWrap, demoHeader, demoFooter, and link follow
the required assertive naming convention.
In `@tools/cli-tools/src/prompts.test.ts`:
- Around line 17-69: Rename the non-assertive test titles for the log and
logJson specs to start with the required assertive prefixes
(creates|throws|requires|returns); specifically, update the it(...) descriptions
that currently read "prints a message to the console", "wraps text by default",
"skips wrapping when wrap is false", "accepts multiple messages", "respects
custom width", and "prints formatted JSON" to assertive forms like "creates a
log that prints a message to the console", "returns wrapped text by default",
"returns unwrapped text when wrap is false", "creates a log that accepts
multiple messages", "returns lines within custom width", and "creates formatted
JSON output" respectively so tests for the log and logJson behaviors
(references: log(), logJson()) conform to the naming pattern.
- Around line 8-10: The test currently installs a persistent global spy via
vi.spyOn(console, "log") that mutates the shared logged array; move that spy
setup into a beforeEach hook (create/reset logged there and call
vi.spyOn(console, "log").mockImplementation(...)) and add an afterEach hook that
calls vi.restoreAllMocks() (and optionally clears logged) to ensure the
console.log spy is restored between tests; update references to the logged array
to rely on the per-test reset.
In `@tools/cli-tools/src/update-env-file.test.ts`:
- Around line 26-78: Rename the test descriptions so they start with one of the
required assertive verbs (creates|throws|requires|returns) and fix the "dont"
typo; specifically update the it(...) titles that mention "updates an existing
key in-place", "appends new keys that dont exist yet", "preserves comments and
blank lines", "handles multiple keys at once", and "handles values containing
equals signs" to begin with a valid verb (e.g., "creates/returns updates...",
"creates appends...", or "preserves" -> "returns preserved..." depending on
intent) and change "dont" to "don't" (or "do not") where applicable; locate
these tests in this file around the it(...) blocks that call updateEnvFile and
reference envPath and adjust only the string descriptions.
- Around line 21-23: The global spies on console.log and console.error (created
with vi.spyOn) are set once and never restored; move those vi.spyOn calls into a
beforeEach hook so each test gets fresh spies and remove the top-level spies,
and in the existing afterEach add vi.restoreAllMocks() to restore the spies
after each test; reference the console.log/console.error spies and the vi.spyOn,
beforeEach, afterEach, and vi.restoreAllMocks symbols when making the change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 088bf0ba-b529-4951-bfec-0f28f1798a34
📒 Files selected for processing (3)
tools/cli-tools/src/formatters.test.tstools/cli-tools/src/prompts.test.tstools/cli-tools/src/update-env-file.test.ts
…ycle Rename all it() descriptions to start with creates/throws/requires/returns. Move vi.spyOn(console, ...) from top-level into beforeEach and add afterEach(() => vi.restoreAllMocks()) in prompts.test.ts and update-env-file.test.ts. Fix "dont" typo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
tools/cli-tools/which previously had zero test coverageWhat's tested
formatters — sectionHeader divider width and step numbering, successMessage/errorMessage prefixes, wordWrap at custom and default widths, link URL preservation
update-env-file — create new .env, update existing keys in-place, append new keys, preserve comments/blank lines, multiple keys at once, values containing equals signs. Uses real temp directories, not mocks.
prompts — log with default wrapping, log with wrap disabled, logJson formatted output
Test plan
pnpm --filter ./tools/cli-tools test— 20 tests passpnpm run check— cleanAI Disclosure: This PR was developed with assistance from Claude Code (Claude Opus).
Summary by CodeRabbit
=).