Skip to content

test(redact): report each size of the long-run test as its own case - #598

Open
royanger wants to merge 1 commit into
mainfrom
ra/redact-test-each
Open

royanger wants to merge 1 commit into
mainfrom
ra/redact-test-each

Conversation

@royanger

Copy link
Copy Markdown
Member

Summary

"a secret followed by a long run" looped over three input sizes inside one test, which .claude/rules/testing.md forbids. When one size failed, the report didn't say which. Each shape now gets its own describe, with one test per size.

-describe("a secret followed by a long run", () => {
-  test.each([...shapes])("%s", (_label, shape) => {
-    for (const size of [10_000, 60_000, 200_000]) {
-      expect(redact(shape("a".repeat(size))).text).not.toContain("hunter2xyz");
-    }
-  });
-});
+describe.each([...shapes])("a secret followed by a long run: %s", (_label, shape) => {
+  test.each([10_000, 60_000, 200_000])("%d characters", (size) => {
+    expect(redact(shape("a".repeat(size))).text).not.toContain("hunter2xyz");
+  });
+});

The four shapes and the assertion are unchanged. CodeRabbit flagged it on #596, which showed this file only because that branch had merged main; the test came from #566.

Evidence

  • Before: bun test packages/cli-core/src/lib/redact.test.ts: 423 pass. The block reports 4 tests, one per shape.
    After: 431 pass, 0 fail. The block reports 12 tests, one per shape and size, such as a secret followed by a long run: flag > 200000 characters.

Merge Danger

Door: two-way

Blast Radius: tests

Test-only, so no changeset and no change to the CLI.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Oct 28, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 50e684e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Team
  • Run ID: ecb51bb3-0b3c-4428-899c-9437d8207db4
Commits

Reviewing files that changed from the base of the PR and between 8415146 and 50e684e.

Files selected for processing: 1
  • packages/cli-core/src/lib/redact.test.ts
Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

Limit details: You’ve used all 6 included reviews currently available. Your 41 included PR review attempts over the past 7 days set your current allowance at 6 reviews per hour.

Walkthrough

The redaction test now runs each of four input shapes with trailing runs of 10,000, 60,000, and 200,000 characters. Each case checks that hunter2xyz is absent from the redacted output.

Priority: Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

  • clerk/cli#566: Adds earlier long-value redaction test coverage that this change refines.
  • clerk/cli#596: Includes the earlier redaction work; this change updates the same test cases.

Suggested reviewers: shane-kercheval

Merge Risk: Minimal · up to 50e68

No actionable merge-blocking risk was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly identifies the test-only change: reporting each long-run input size as a separate test case.
Description check Passed The description accurately explains the test restructuring, the reason for the change, and the reported test results.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Linked repositories: Some configured repositories could not be included in this review. Cross-repository analysis may be incomplete. Please check linked-repository access and settings.

Comment @coderabbitai help to get the list of available commands.

@royanger

Copy link
Copy Markdown
Member Author

Hey @shane-kercheval, I added you as a reviewer for this as I think you authored the original work here.

@royanger
royanger requested a review from manovotny October 29, 2026 18:11
@manovotny manovotny self-assigned this Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants