From 68617a9f408dede1245a0ede853e800045ba3d5e Mon Sep 17 00:00:00 2001 From: Sephyi Date: Sun, 19 Apr 2026 19:07:19 +0200 Subject: [PATCH] test: parametrise make_symbol helper by line range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/context.rs | 53 +++++++++++++++++++++++++++-------------------- tests/helpers.rs | 50 +++++++++++++++++++++++++++++++++++++++++++- tests/splitter.rs | 28 ++----------------------- 3 files changed, 81 insertions(+), 50 deletions(-) diff --git a/tests/context.rs b/tests/context.rs index 79958a6..d6aa8b8 100644 --- a/tests/context.rs +++ b/tests/context.rs @@ -11,7 +11,9 @@ use commitbee::domain::{ ChangeStatus, CodeSymbol, CommitType, FileCategory, IntentKind, SymbolKind, }; use commitbee::services::context::ContextBuilder; -use helpers::{make_file_change, make_renamed_file, make_staged_changes}; +use helpers::{ + make_file_change, make_renamed_file, make_staged_changes, make_symbol, make_symbol_at, +}; // ─── Helpers ───────────────────────────────────────────────────────────────── @@ -19,28 +21,6 @@ fn default_config() -> Config { Config::default() } -fn make_symbol( - name: &str, - kind: SymbolKind, - file: &str, - is_public: bool, - is_added: bool, -) -> CodeSymbol { - CodeSymbol { - kind, - name: name.to_string(), - file: PathBuf::from(file), - line: 1, - end_line: 10, - is_public, - is_added, - is_whitespace_only: None, - span_change_kind: None, - signature: None, - parent_scope: None, - } -} - // ─── CommitType inference ───────────────────────────────────────────────────── #[test] @@ -2059,3 +2039,30 @@ fn intents_capped_at_three() { ctx.intents.len() ); } + +// ─── make_symbol helper smoke tests ────────────────────────────────────────── + +#[test] +fn make_symbol_defaults_to_lines_1_through_10() { + let sym = make_symbol("foo", SymbolKind::Function, "src/lib.rs", true, true); + assert_eq!(sym.line, 1); + assert_eq!(sym.end_line, 10); +} + +#[test] +fn make_symbol_at_pins_arbitrary_line_range() { + let sym = make_symbol_at( + "bar", + SymbolKind::Function, + "src/lib.rs", + false, + true, + 42, + 57, + ); + assert_eq!(sym.line, 42); + assert_eq!(sym.end_line, 57); + assert_eq!(sym.name, "bar"); + assert!(!sym.is_public); + assert!(sym.is_added); +} diff --git a/tests/helpers.rs b/tests/helpers.rs index c825c5a..0ab99dc 100644 --- a/tests/helpers.rs +++ b/tests/helpers.rs @@ -5,7 +5,9 @@ use std::path::PathBuf; use std::sync::Arc; -use commitbee::domain::{ChangeStatus, DiffStats, FileCategory, FileChange, StagedChanges}; +use commitbee::domain::{ + ChangeStatus, CodeSymbol, DiffStats, FileCategory, FileChange, StagedChanges, SymbolKind, +}; /// Create a minimal FileChange for testing #[allow(dead_code)] @@ -45,6 +47,52 @@ pub fn make_renamed_file(old_path: &str, new_path: &str, similarity: u8) -> File } } +/// Create a minimal `CodeSymbol` for testing, with `line: 1, end_line: 10`. +/// +/// For tests that need the symbol to sit at a specific line range (e.g. to +/// exercise hunk-to-span mapping), use [`make_symbol_at`] instead. +#[allow(dead_code)] +pub fn make_symbol( + name: &str, + kind: SymbolKind, + file: &str, + is_public: bool, + is_added: bool, +) -> CodeSymbol { + make_symbol_at(name, kind, file, is_public, is_added, 1, 10) +} + +/// Create a minimal `CodeSymbol` at an arbitrary line range. +/// +/// Prefer this variant when a test needs to pin the symbol to specific +/// `line` / `end_line` positions (for example, to line up with a manually +/// crafted diff hunk). For the common case where positions are irrelevant, +/// [`make_symbol`] uses the defaults `line: 1, end_line: 10`. +#[allow(dead_code)] +pub fn make_symbol_at( + name: &str, + kind: SymbolKind, + file: &str, + is_public: bool, + is_added: bool, + line: usize, + end_line: usize, +) -> CodeSymbol { + CodeSymbol { + kind, + name: name.to_string(), + file: PathBuf::from(file), + line, + end_line, + is_public, + is_added, + is_whitespace_only: None, + span_change_kind: None, + signature: None, + parent_scope: None, + } +} + /// Create StagedChanges from a list of FileChanges #[allow(dead_code)] pub fn make_staged_changes(files: Vec) -> StagedChanges { diff --git a/tests/splitter.rs b/tests/splitter.rs index 2adffdb..89f5686 100644 --- a/tests/splitter.rs +++ b/tests/splitter.rs @@ -6,33 +6,9 @@ mod helpers; use std::path::PathBuf; -use commitbee::domain::{ChangeStatus, CodeSymbol, SymbolKind}; +use commitbee::domain::{ChangeStatus, SymbolKind}; use commitbee::services::splitter::{CommitSplitter, SplitSuggestion}; -use helpers::{make_file_change, make_staged_changes}; - -// ─── Helpers ───────────────────────────────────────────────────────────────── - -fn make_symbol( - name: &str, - kind: SymbolKind, - file: &str, - is_public: bool, - is_added: bool, -) -> CodeSymbol { - CodeSymbol { - kind, - name: name.to_string(), - file: PathBuf::from(file), - line: 1, - end_line: 10, - is_public, - is_added, - is_whitespace_only: None, - span_change_kind: None, - signature: None, - parent_scope: None, - } -} +use helpers::{make_file_change, make_staged_changes, make_symbol}; // ─── Split detection tests ───────────────────────────────────────────────────