Skip to content

test: consolidate the byte-identity file-compare idiom into a shared helper - #950

Merged
nh13 merged 1 commit into
mainfrom
nh/consolidate-file-compare-helper
Sep 11, 2026
Merged

nh13 merged 1 commit into
mainfrom
nh/consolidate-file-compare-helper

Conversation

@nh13

@nh13 nh13 commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

Pure test-infra hygiene. The cutover-parity tests each reimplemented the byte-identity file-compare idiom assert_eq!(read_to_string(a), read_to_string(b)) inline — 7 sites across 5 files (dedup, group, clip, retag, correct). This consolidates them into a single assert_text_files_eq(actual, expected, label) in the shared tests/integration/helpers/assertions.rs, with unit tests for the identical / divergent / missing-file cases.

Behavior-preserving: same files, same actual/expected ordering, same per-site labels — the existing cutover assertions are unchanged and the full suite stays green. The helper deliberately does not guard for non-empty content: whether an output file must carry data rows is specific to the metric being compared, so any such check stays with its caller.

Risk: command output changes: none; unsafe changes: none, and CLAUDE.md allowlist changes: none; memory bounds, queue capacity, and thread/backpressure policy changes: none.

Added assert_text_files_eq for byte-identical text-file assertions. Added tests for identical, divergent, and missing files. Replaced seven inline comparisons in cutover-parity tests while preserving file order, labels, assertion behavior, and caller-specific non-empty checks.

…helper

The cutover-parity tests each reimplemented `assert_eq!(read_to_string(a),
read_to_string(b))` inline (dedup, group, clip, retag, correct — 7 sites). Add
a single `assert_text_files_eq(actual, expected, label)` to the shared
`helpers/assertions.rs` (with unit tests for the identical / divergent / missing
cases) and route those sites through it. Pure test-infra hygiene; the existing
cutover assertions are unchanged in behavior. The helper deliberately does not
guard for non-empty content — whether an output file must carry data rows is
specific to the metric being compared, so that check stays with its caller.
@nh13
nh13 deployed to github-actions September 9, 2026 20:41 — with GitHub Actions Active
@coderabbitai

coderabbitai Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Note

Reviews paused

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: ce293b9e-a9c3-48d8-bd7e-81497d168fae

📥 Commits

Reviewing files that changed from the base of the PR and between a54a17e and 1d9b94a.

📒 Files selected for processing (6)
  • tests/integration/helpers/assertions.rs
  • tests/integration/test_clip_cutover_parity.rs
  • tests/integration/test_correct_cutover_parity.rs
  • tests/integration/test_dedup_cutover_parity.rs
  • tests/integration/test_group_cutover_parity.rs
  • tests/integration/test_retag_cutover_parity.rs

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


Walkthrough

The changes add a shared byte-for-byte text-file assertion helper and replace direct string comparisons in cutover parity tests for metrics and histogram outputs.

Changes

Text-file parity assertions

Layer / File(s) Summary
Text-file assertion helper
tests/integration/helpers/assertions.rs
Adds assert_text_files_eq with labeled read errors and mismatch reporting. Tests cover identical files, divergent content, and missing files.
Parity test adoption
tests/integration/test_*_cutover_parity.rs
Uses the shared helper for metrics TSV and family-size histogram comparisons while preserving strategy-specific failure messages.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 1d9b9

This change centralizes parity-test file comparisons without changing their behavior, so no merge-blocking production risk remains.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title uses the valid test: prefix, a lowercase imperative description, and accurately summarizes the shared helper change. It does not end with a period.
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.

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

@nh13

nh13 commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai pause

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews paused.

@codecov

codecov Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.44%. Comparing base (a54a17e) to head (1d9b94a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #950      +/-   ##
==========================================
- Coverage   94.44%   94.44%   -0.01%     
==========================================
  Files         307      307              
  Lines      151152   151152              
==========================================
- Hits       142763   142751      -12     
- Misses       8389     8401      +12     

☔ 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.

@nh13

nh13 commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13
nh13 added this pull request to the merge queue Sep 11, 2026
Merged via the queue into main with commit 42fae66 Sep 11, 2026
17 checks passed
@nh13
nh13 deleted the nh/consolidate-file-compare-helper branch September 11, 2026 08:23
@nh13 nh13 mentioned this pull request Sep 10, 2026

This branch was successfully deployed

1 active deployment
github-actions — 1d9b94a9 Deployed Sep 9, 2026 by nh13 via coverage #4402
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.

1 participant