Skip to content

test(corpus): name the flag that exists - #51

Merged
michen00 merged 2 commits into
mainfrom
test/name-the-flag-that-exists
Sep 9, 2026
Merged

test(corpus): name the flag that exists#51
michen00 merged 2 commits into
mainfrom
test/name-the-flag-that-exists

Conversation

@michen00

@michen00 michen00 commented Sep 3, 2026

Copy link
Copy Markdown
Owner

What changes, and why

tests/corpus.rs:150 justified splitting the count assertion from the output assertion by naming what "--check and --json consumers actually read."

There is no --check, in either implementation, and there never has been:

$ grep -rn "'--check'\|\"--check\"" src/ tests/   # excluding this comment
  (nothing)

Rust:   --help --files-from --write --json --fail-on-change --ignore-file --exclude
Python: --write --json --fail-on-change --files-from --ignore-file --exclude

The option the comment means is --fail-on-change, which reports without rewriting and has existed since the first commit. A reader taking the comment at face value goes looking for an option neither binary accepts — in the file whose whole subject is what the two implementations must agree about.

The Python counterpart at tests/test_corpus.py:86 makes the same argument without naming a flag at all ("a single combined assertion would let the first failure hide the second"), so either wording is consistent with it. Naming the real flag is the more useful of the two here, because the point being made is which consumers the reported counts reach.

Corpus

The corpus is the specification, and both implementations answer to it. Tick what applies.

  • This changes no behavior the corpus specifies.
  • This changes what gets joined, and a case in corpus/ pins the new behavior. The case was written first and failed first.
  • The change makes the tool join more than it did. The section above says what it will not eat.

One comment in a test file.

Checks

  • make check passes, or make test does and this touches no Rust.

This touches Rust, so make rust-lint (clippy at -D warnings) and make rust-test — 104 unit tests and the 4 corpus harness tests, all passing.

The count assertion's comment justified itself by what `--check` and
`--json` consumers read. There is no `--check` in either
implementation and there never has been: the flag set is `--help`,
`--files-from`, `--write`, `--json`, `--fail-on-change`,
`--ignore-file` and `--exclude`. The option this means is
`--fail-on-change`, which reports without rewriting and has existed
since the first commit.

A reader taking the comment at face value goes looking for an option
neither binary accepts, in a file whose whole subject is what the two
must agree about. The Python counterpart makes the same argument without
naming a flag, so either wording is consistent with it; naming the real
one is the more useful of the two here, because the point is which
consumers the counts reach.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Correct flag name in corpus test comment

📝 Documentation 🕐 Less than 5 minutes

Grey Divider

AI Description

• Replaces the nonexistent --check reference with --fail-on-change.
• Clarifies which command consumers read reported corpus counts.
High-Level Assessment

The current approach is optimal: naming the actual --fail-on-change flag is more informative than using generic wording and avoids directing readers toward a nonexistent option.

Files changed (1) +1 / -1

Documentation (1) +1 / -1
corpus.rsCorrect the reporting flag named in a corpus test comment +1/-1

Correct the reporting flag named in a corpus test comment

• Replaces the nonexistent '--check' option with '--fail-on-change' when explaining which consumers depend on corpus counts. Runtime behavior and test assertions remain unchanged.

tests/corpus.rs

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.40%. Comparing base (9a8399f) to head (66bf92b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #51   +/-   ##
=======================================
  Coverage   87.40%   87.40%           
=======================================
  Files           3        3           
  Lines         691      691           
=======================================
  Hits          604      604           
  Misses         87       87           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is a verified documentation-only correction that improves accuracy without affecting behavior or test logic.

Pull request overview

This PR corrects a misleading Rust corpus test comment by naming the actual CLI flag (--fail-on-change) that corresponds to “reporting without rewriting,” aligning the test documentation with the real, cross-implementation CLI contract.

Changes:

  • Replace a nonexistent flag reference (--check) with the existing --fail-on-change in tests/corpus.rs test commentary.
File summaries
File Description
tests/corpus.rs Updates an explanatory comment to reference the correct CLI flag consumed by reporting paths.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@michen00
michen00 merged commit 8fe4356 into main Sep 9, 2026
26 checks passed
@michen00
michen00 deleted the test/name-the-flag-that-exists branch September 9, 2026 08:23
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.

3 participants