Skip to content

D-040: test: parametrise make_symbol helper by line range#9

Closed
Sephyi wants to merge 1 commit intodevelopmentfrom
audit/d-040-make-symbol-line-range
Closed

D-040: test: parametrise make_symbol helper by line range#9
Sephyi wants to merge 1 commit intodevelopmentfrom
audit/d-040-make-symbol-line-range

Conversation

@Sephyi
Copy link
Copy Markdown
Owner

@Sephyi Sephyi commented Apr 22, 2026

Summary

test: parametrise make_symbol helper by line range.

Audit context

Closes audit entry D-040 from #3.

Verification

  • cargo fmt --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --all-targets

Note: one pre-existing test porcelain_exits_within_timeout_with_no_staged_changes is a known macOS cold-start flake that reproduces on unmodified development — unrelated to this change.

Consolidate the two duplicated `make_symbol` helpers (one in
`tests/context.rs`, one in `tests/splitter.rs`) into `tests/helpers.rs`
and add a `make_symbol_at(..., line, end_line)` variant that lets
callers pin symbols to arbitrary line ranges. `make_symbol` is kept as
a thin wrapper that delegates to `make_symbol_at` with the historical
defaults `line: 1, end_line: 10`, so every existing call site compiles
and runs unchanged.

The audit entry proposed either (a) adding a `line, end_line` parameter
pair to `make_symbol` and updating ~50 call sites, or (b) introducing a
sibling `make_symbol_at` alongside. Chose (b) because the diff touches
only the three helper files (instead of rewriting every single
`make_symbol(...)` invocation), eliminates two exact duplicate function
bodies in the process, and keeps behaviour bit-identical for every
existing test. Future tests that need hunk-to-span mapping (for example
around `classify_span_change`) can now express real line positions
without cloning the whole `CodeSymbol` literal.

Two small smoke tests in `tests/context.rs` lock down the defaults
(`make_symbol` ⇒ `line: 1, end_line: 10`) and confirm that
`make_symbol_at` honours arbitrary positions and the other passthrough
fields (`name`, `is_public`, `is_added`).

Closes audit entry D-040 from #3.
Copilot AI review requested due to automatic review settings April 22, 2026 19:50
@Sephyi Sephyi added the audit Codebase audit cleanup (issue #3) label Apr 22, 2026
@Sephyi Sephyi self-assigned this Apr 22, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR centralizes the make_symbol test helper into the shared tests/helpers.rs module and introduces a new make_symbol_at variant to allow tests to pin CodeSymbol line ranges (addressing audit item D-040).

Changes:

  • Move duplicated make_symbol helper implementations out of individual test files into tests/helpers.rs.
  • Add make_symbol_at(..., line, end_line) to support line-range-specific symbol construction in tests.
  • Update affected tests to import the shared helpers and add small smoke tests covering helper defaults/overrides.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/splitter.rs Removes local make_symbol and uses helpers::make_symbol instead.
tests/helpers.rs Adds shared make_symbol + new make_symbol_at helper for configurable line ranges.
tests/context.rs Removes local make_symbol, imports shared helpers, and adds smoke tests for default/pinned line ranges.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sephyi added a commit that referenced this pull request Apr 30, 2026
Originally PR #9; rebased onto current development. Adds shared
`make_symbol` and `make_symbol_at` helpers in tests/helpers.rs and
collapses the per-file copies in tests/splitter.rs and tests/context.rs.

Conflict resolution: kept `make_renamed_file_with_diff` (added by
D-037 / #7) — PR #9's branch was based on a pre-#7 state that
incidentally removed it. Only the make_symbol parametrisation is
in scope here.

Closes #9.
@Sephyi
Copy link
Copy Markdown
Owner Author

Sephyi commented Apr 30, 2026

Superseded by #33 — rebased onto current development. Closing this stale branch.

@Sephyi Sephyi closed this Apr 30, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 30, 2026
@Sephyi Sephyi deleted the audit/d-040-make-symbol-line-range branch April 30, 2026 17:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

audit Codebase audit cleanup (issue #3)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants