Skip to content

test(agent): add end-to-end regression test for review item fingerprint stability - #898

Merged
lizhengfeng101 merged 1 commit into
alibaba:mainfrom
yingjiexu2002:test/fingerprint-position-stability
Aug 14, 2026
Merged

test(agent): add end-to-end regression test for review item fingerprint stability#898
lizhengfeng101 merged 1 commit into
alibaba:mainfrom
yingjiexu2002:test/fingerprint-position-stability

Conversation

@yingjiexu2002

Copy link
Copy Markdown
Collaborator

Description

Adds the regression test described in #897: TestReviewItemFingerprintStableAcrossPatchPosition derives fingerprints from real diff.ParseDiffText output and asserts they do not depend on a file's position within the patch.

#732 fixed #718 by normalizing trailing line endings in reviewItemFingerprint, but the existing helper-level test feeds hand-written strings and never exercises ParseDiffText — the component that actually produces the position-dependent trailing newlines. This test pins the user-visible --resume guarantee (same unchanged file keeps the same fingerprint regardless of patch position) on the real parser path:

  • plain patch: two-file patch with the target file last vs. first (the splitter leaves one trailing newline on the last file)
  • untracked join: workspace-mode "\n\n" joining, where the trailing run is 2-3 newlines — the case only TrimRight (not TrimSuffix) handles

Test-only change; no production code modified.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that changes existing functionality)
  • Refactoring (no functional changes)
  • Documentation update
  • CI / Build / Tooling

How Has This Been Tested?

  • New test passes on main with both sub-cases (plain patch, untracked join)
  • Mutation check: removing the strings.TrimRight(d.Diff, "\r\n") normalization makes the test fail (verified locally, then reverted)
  • No spurious cannot read file warnings on stderr
  • make test passes (full suite)
  • make check passes

Checklist

  • My code follows the project's coding style (go fmt, go vet)
  • I have performed a self-review of my code
  • I have added tests that prove the fix/feature works
  • New and existing unit tests pass locally with my changes
  • I have signed the CLA

Related Issues

closes #897

…nt stability

Derive fingerprints from real diff.ParseDiffText output and assert they
do not depend on a file's position in the patch, covering both plain
multi-file patches and the workspace-mode "\n\n" joining case. This pins
the --resume reuse guarantee on the parser path rather than on the
helper's string handling alone.

Closes alibaba#897
@github-actions

Copy link
Copy Markdown
Contributor

OpenCodeReview: Review skipped: no items were selected.

@lizhengfeng101 lizhengfeng101 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@lizhengfeng101
lizhengfeng101 merged commit 51750bf into alibaba:main Aug 14, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants