From 6559267cfb5d7b099288a2b00d53e9e953856085 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 19 May 2026 14:52:31 +0545 Subject: [PATCH 01/21] feat: lint messages from hash-range - remove projecet's git dependent tests from src/git_helpers.rs --- src/cli.rs | 8 ++++- src/command.rs | 33 +++++++++++++++++- src/git_helpers.rs | 85 +++++++++++++++++++--------------------------- 3 files changed, 74 insertions(+), 52 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 9111455..1f0f704 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -6,7 +6,7 @@ use clap::{ArgGroup, Parser}; #[command(version)] #[command(group( ArgGroup::new("input") - .args(["message", "file", "hash"]) + .args(["message", "file", "hash", "from_hash"]) .required(true) .multiple(false)))] pub struct Cli { @@ -18,4 +18,10 @@ pub struct Cli { #[arg(long)] pub hash: Option, + + #[arg(long)] + pub from_hash: Option, + + #[arg(long, requires = "from_hash", default_value = "HEAD")] + pub to_hash: Option, } diff --git a/src/command.rs b/src/command.rs index 8db583a..866c5bf 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1,5 +1,5 @@ use crate::cli::Cli; -use crate::git_helpers::get_commit_message_from_hash; +use crate::git_helpers::{get_commit_message_from_hash, get_commit_messages_from_hash_range}; use crate::linter::lint_commit_message; use crate::messages::{VALIDATION_FAILED, VALIDATION_SUCCESSFUL}; use crate::utils::{is_empty, is_ignored}; @@ -33,6 +33,33 @@ fn handle_commit_message(msg: &str) { std::process::exit(1); } +fn handle_multiple_commit_messages(messages: &[String]) { + let mut has_failure = false; + + for msg in messages { + if is_empty(msg) { + continue; + } + + if is_ignored(msg) { + continue; + } + + let success = lint_commit_message(msg); + if !success { + has_failure = true; + // TODO: Log proper error for individual message + } + } + + if has_failure { + eprintln!("{}", VALIDATION_FAILED); + std::process::exit(1); + } + + println!("{}", VALIDATION_SUCCESSFUL); +} + pub fn run(args: Cli) -> Result<()> { if let Some(msg) = &args.message { // direct message @@ -44,6 +71,10 @@ pub fn run(args: Cli) -> Result<()> { } else if let Some(hash) = &args.hash { let msg = get_commit_message_from_hash(hash)?; handle_commit_message(&msg); + } else if let Some(from_hash) = &args.from_hash { + let to_hash = &args.to_hash.unwrap(); + let messages = get_commit_messages_from_hash_range(from_hash, to_hash)?; + handle_multiple_commit_messages(&messages); } else { unreachable!("invalid option is handle by clap"); } diff --git a/src/git_helpers.rs b/src/git_helpers.rs index f5b47d1..c33ddec 100644 --- a/src/git_helpers.rs +++ b/src/git_helpers.rs @@ -15,61 +15,46 @@ pub fn get_commit_message_from_hash(commit_hash: &str) -> Result { Ok(message) } -#[cfg(test)] -mod tests { - use super::*; +pub fn get_commit_messages_from_hash_range(from_hash: &str, to_hash: &str) -> Result> { + let delimiter = "========commit-delimiter========"; + let range = format!("{}..{}", from_hash, to_hash); - fn git_show_head() -> Option { - let output = Command::new("git") - .args(["show", "--format=%B", "-s", "HEAD"]) - .output() - .ok()?; - if !output.status.success() { - return None; - } - Some(String::from_utf8_lossy(&output.stdout).trim().to_string()) - } + // range A..B returns all commits from B but not A ie (B - A), so A has to added seaprately + let from_hash_message = get_commit_message_from_hash(from_hash)?; - #[test] - fn returns_head_commit_message() { - let Some(expected) = git_show_head() else { - eprintln!("skipping: not a git repo or git unavailable"); - return; - }; + let output = Command::new("git") + .args([ + "log", + "--no-merges", + &format!("--pretty=format:%B{delimiter}"), + "--reverse", + &range, + ]) + .output() + .with_context(|| { + format!( + "failed to execute git log for range {}, {}", + from_hash, to_hash + ) + })?; - let got = get_commit_message_from_hash("HEAD").expect("HEAD should resolve"); - assert_eq!(got, expected); - assert!(!got.is_empty(), "HEAD message should not be empty"); + if !output.status.success() { + anyhow::bail!( + "failed to retrieve commit messages for range {}, {}", + from_hash, + to_hash + ); } - #[test] - fn returns_trimmed_message() { - // The function trims trailing whitespace/newlines that `git show` appends. - let Some(msg) = git_show_head() else { - eprintln!("skipping: not a git repo or git unavailable"); - return; - }; - assert_eq!(msg, msg.trim()); - } + let mut messages: Vec = String::from_utf8_lossy(&output.stdout) + .split(&delimiter) + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + .map(|s| s.to_owned()) + .collect(); - #[test] - fn errors_on_unknown_hash() { - // Use a clearly non-existent hash. 40 zeros is reserved/null in git. - let result = get_commit_message_from_hash("0000000000000000000000000000000000000000"); - assert!( - result.is_err(), - "expected error for unknown hash, got {:?}", - result - ); - } + // prepend the first message of the range + messages.insert(0, from_hash_message); - #[test] - fn errors_on_malformed_hash() { - let result = get_commit_message_from_hash("not-a-real-hash-zzz"); - assert!( - result.is_err(), - "expected error for malformed hash, got {:?}", - result - ); - } + Ok(messages) } From b87893386359f3ce47d7d8a10f2428ee93091b52 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 19 May 2026 14:54:35 +0545 Subject: [PATCH 02/21] test: update unittests and integration tests * dev-dependency added: `serial_test = "3"` for blocking threads - `tests/common/git_repo.rs` to create temporary git repo - `src/git_helpers.rs`: replaced project's git dependent unittests with integrationtests in tests/git_helpers.rs - improved integration tests for --hash flag - added integration tests for hash-range feature - extend Cli argument constrains test - mutual exclusion of message + --hash (#12) --- Cargo.lock | 142 ++++++++++++++++++ Cargo.toml | 1 + tests/cli.rs | 306 +++++++++++++++++++++++++++++++++++++-- tests/common/git_repo.rs | 72 +++++++++ tests/common/mod.rs | 3 + tests/git_helpers.rs | 114 +++++++++++++++ 6 files changed, 628 insertions(+), 10 deletions(-) create mode 100644 tests/common/git_repo.rs create mode 100644 tests/common/mod.rs create mode 100644 tests/git_helpers.rs diff --git a/Cargo.lock b/Cargo.lock index fb0ad57..051ec68 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -160,6 +160,7 @@ dependencies = [ "clap", "predicates", "regex", + "serial_test", "tempfile", ] @@ -212,6 +213,41 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" +[[package]] +name = "futures-core" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d" + +[[package]] +name = "futures-executor" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baf29c38818342a3b26b5b923639e7b1f4a61fc5e76102d4b1981c6dc7a7579d" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-task" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "037711b3d59c33004d3856fbdc83b99d4ff37a24768fa1be9ce3538a1cde4393" + +[[package]] +name = "futures-util" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "389ca41296e6190b48053de0321d02a77f32f8a5d2461dd38762c0593805c6d6" +dependencies = [ + "futures-core", + "futures-task", + "pin-project-lite", + "slab", +] + [[package]] name = "getrandom" version = "0.4.2" @@ -294,6 +330,15 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" +[[package]] +name = "lock_api" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "224399e74b87b5f3557511d98dff8b14089b3dadafcab6bb93eab67d3aace965" +dependencies = [ + "scopeguard", +] + [[package]] name = "log" version = "0.4.29" @@ -333,6 +378,35 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" +[[package]] +name = "parking_lot" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93857453250e3077bd71ff98b6a65ea6621a19bb0f559a85248955ac12c45a1a" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2621685985a2ebf1c516881c026032ac7deafcda1a2c9b7850dc81e3dfcb64c1" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-link", +] + +[[package]] +name = "pin-project-lite" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" + [[package]] name = "predicates" version = "3.1.4" @@ -397,6 +471,15 @@ version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" +[[package]] +name = "redox_syscall" +version = "0.5.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed2bf2547551a7053d6fdfafda3f938979645c44812fbfcda098faae3f1a362d" +dependencies = [ + "bitflags", +] + [[package]] name = "regex" version = "1.12.3" @@ -439,6 +522,27 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "scc" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46e6f046b7fef48e2660c57ed794263155d713de679057f2d0c169bfc6e756cc" +dependencies = [ + "sdd", +] + +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + +[[package]] +name = "sdd" +version = "3.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "490dcfcbfef26be6800d11870ff2df8774fa6e86d047e3e8c8a76b25655e41ca" + [[package]] name = "semver" version = "1.0.28" @@ -487,6 +591,44 @@ dependencies = [ "zmij", ] +[[package]] +name = "serial_test" +version = "3.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "911bd979bf1070a3f3aa7b691a3b3e9968f339ceeec89e08c280a8a22207a32f" +dependencies = [ + "futures-executor", + "futures-util", + "log", + "once_cell", + "parking_lot", + "scc", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a7d91949b85b0d2fb687445e448b40d322b6b3e4af6b44a29b21d9a5f33e6d9" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "slab" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" + +[[package]] +name = "smallvec" +version = "1.15.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" + [[package]] name = "strsim" version = "0.11.1" diff --git a/Cargo.toml b/Cargo.toml index e90f13a..1891dd3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ anyhow = "1.0" [dev-dependencies] assert_cmd = "2" predicates = "3" +serial_test = "3" tempfile = "3" # The profile that 'dist' will build with diff --git a/tests/cli.rs b/tests/cli.rs index f518267..386b0ae 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1,4 +1,7 @@ +mod common; + use assert_cmd::Command; +use common::TestRepo; use predicates::prelude::*; use std::io::Write; use tempfile::NamedTempFile; @@ -9,7 +12,8 @@ fn cocox() -> Command { fn write_temp(contents: &str) -> NamedTempFile { let mut file = NamedTempFile::new().expect("create temp file"); - file.write_all(contents.as_bytes()).expect("write temp file"); + file.write_all(contents.as_bytes()) + .expect("write temp file"); file } @@ -168,20 +172,24 @@ fn missing_file_fails() { .arg("/nonexistent/path/commit-msg.txt") .assert() .failure() - .stderr(predicate::str::contains("failed to read commit message file")); + .stderr(predicate::str::contains( + "failed to read commit message file", + )); } // --- --hash --------------------------------------------------------------- #[test] -fn hash_head_exits_cleanly() { - // We only assert exit-success here: HEAD's message depends on the - // environment (locally a conventional commit; under - // actions/checkout it's a synthetic merge commit, which falls into - // the ignore path and prints nothing). A stronger stdout assertion - // requires a fixture commit with a known message — tracked - // separately. - cocox().arg("--hash").arg("HEAD").assert().success(); +fn hash_head_valid_commit_succeeds() { + let repo = TestRepo::new(); + repo.commit("feat: add new feature"); + + cocox() + .arg("--hash") + .arg("HEAD") + .assert() + .success() + .stdout(predicate::str::contains("Commit validation: successful!")); } #[test] @@ -196,6 +204,195 @@ fn invalid_hash_fails() { )); } +#[test] +fn hash_exact_valid_commit_succeeds() { + let repo = TestRepo::new(); + let hash = repo.commit("feat: add new feature"); + + cocox() + .arg("--hash") + .arg(&hash) + .assert() + .success() + .stdout(predicate::str::contains("Commit validation: successful!")); +} + +#[test] +fn hash_invalid_commit_message_fails() { + let repo = TestRepo::new(); + let hash = repo.commit("not a conventional commit"); + + cocox() + .arg("--hash") + .arg(&hash) + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains("Commit validation: failed!")); +} + +#[test] +fn hash_ignored_commit_silently_succeeds() { + let repo = TestRepo::new(); + let hash = repo.commit("Merge pull request #123"); + + cocox() + .arg("--hash") + .arg(&hash) + .assert() + .success() + .stdout(predicate::str::is_empty()) + .stderr(predicate::str::is_empty()); +} + +// ---- hash range ---------------------------------------------------------- + +#[test] +fn hash_range_valid_commits_succeeds() { + let repo = TestRepo::new(); + + let a = repo.commit("feat: add new feature A"); + repo.commit("fix: fix a bug"); + repo.commit("fix: fix another bug"); + repo.commit("feat: add another feature"); + let c = repo.commit("perf: improve performence"); + + cocox() + .arg("--from-hash") + .arg(a) + .arg("--to-hash") + .arg(c) + .assert() + .success() + .stdout(predicate::str::contains("Commit validation: successful!")); +} + + +#[test] +fn hash_range_invalid_commit_in_middle_fails() { + let repo = TestRepo::new(); + + let a = repo.commit("feat: add new feature"); + repo.commit("not a conventional commit"); + let c = repo.commit("fix: fix a bug"); + + cocox() + .arg("--from-hash") + .arg(&a) + .arg("--to-hash") + .arg(&c) + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains("Commit validation: failed!")); +} + +#[test] +fn hash_range_invalid_from_commit_fails() { + let repo = TestRepo::new(); + + let a = repo.commit("not a conventional commit"); + let b = repo.commit("feat: add new feature"); + + cocox() + .arg("--from-hash") + .arg(&a) + .arg("--to-hash") + .arg(&b) + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains("Commit validation: failed!")); +} + +#[test] +fn hash_range_invalid_to_commit_fails() { + let repo = TestRepo::new(); + + let a = repo.commit("feat: add new feature"); + let b = repo.commit("not a conventional commit"); + + cocox() + .arg("--from-hash") + .arg(&a) + .arg("--to-hash") + .arg(&b) + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains("Commit validation: failed!")); +} + +#[test] +fn hash_range_ignored_commits_are_skipped() { + let repo = TestRepo::new(); + + let a = repo.commit("feat: add new feature"); + repo.commit("Merge pull request #123"); + let c = repo.commit("fix: fix a bug"); + + cocox() + .arg("--from-hash") + .arg(&a) + .arg("--to-hash") + .arg(&c) + .assert() + .success() + .stdout(predicate::str::contains("Commit validation: successful!")); +} + +#[test] +fn hash_range_single_commit_succeeds() { + let repo = TestRepo::new(); + + let a = repo.commit("feat: single commit"); + + cocox() + .arg("--from-hash") + .arg(&a) + .arg("--to-hash") + .arg(&a) + .assert() + .success() + .stdout(predicate::str::contains("Commit validation: successful!")); +} + +#[test] +fn hash_range_single_invalid_commit_fails() { + let repo = TestRepo::new(); + + let a = repo.commit("not a conventional commit"); + + cocox() + .arg("--from-hash") + .arg(&a) + .arg("--to-hash") + .arg(&a) + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains("Commit validation: failed!")); +} + + +#[test] +fn from_hash_only_valid_commits_succeeds() { + let repo = TestRepo::new(); + + let a = repo.commit("feat: add new feature A"); + repo.commit("fix: fix a bug"); + repo.commit("fix: fix another bug"); + repo.commit("feat: add another feature"); + repo.commit("perf: improve performence"); + + cocox() + .arg("--from-hash") + .arg(a) + .assert() + .success() + .stdout(predicate::str::contains("Commit validation: successful!")); +} + // --- clap argument constraints -------------------------------------------- #[test] @@ -207,6 +404,17 @@ fn no_args_fails_with_clap_error() { .stderr(predicate::str::contains("required")); } +#[test] +fn to_hash_only_fails() { + cocox() + .arg("--to-hash") + .arg("HEAD") + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("required")); +} + #[test] fn message_and_file_together_fail() { let file = write_temp("feat: a body"); @@ -234,6 +442,84 @@ fn file_and_hash_together_fail() { .stderr(predicate::str::contains("cannot be used")); } +#[test] +fn file_and_from_hash_together_fail() { + let file = write_temp("feat: a body"); + cocox() + .arg("--file") + .arg(file.path()) + .arg("--from-hash") + .arg("HEAD") + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("cannot be used")); +} + +#[test] +fn message_and_hash_together_fail() { + cocox() + .arg("feat: something") + .arg("--hash") + .arg("HEAD") + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("cannot be used")); +} + +#[test] +fn message_and_from_hash_together_fail() { + cocox() + .arg("feat: something") + .arg("--from-hash") + .arg("HEAD") + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("cannot be used")); +} + +#[test] +fn hash_and_from_hash_together_fail() { + cocox() + .arg("--hash") + .arg("HEAD") + .arg("--from-hash") + .arg("HEAD") + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("cannot be used")); +} + +#[test] +fn to_hash_with_hash_together_fail() { + // --to-hash requires --from-hash specifically, not just any input arg + cocox() + .arg("--hash") + .arg("HEAD") + .arg("--to-hash") + .arg("HEAD") + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("cannot be used")); +} + +#[test] +fn to_hash_with_message_fails() { + // --to-hash requires --from-hash, passing a message doesn't satisfy it + cocox() + .arg("feat: something") + .arg("--to-hash") + .arg("HEAD") + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("cannot be used")); +} + #[test] fn version_flag_succeeds() { cocox() diff --git a/tests/common/git_repo.rs b/tests/common/git_repo.rs new file mode 100644 index 0000000..898046f --- /dev/null +++ b/tests/common/git_repo.rs @@ -0,0 +1,72 @@ +use assert_cmd::Command; +use std::env; +use std::fs; +use std::path::PathBuf; +use std::sync::{Mutex, MutexGuard}; +use tempfile::TempDir; + +static SERIAL: Mutex<()> = Mutex::new(()); +pub struct TestRepo { + dir: TempDir, + original_dir: PathBuf, + _guard: MutexGuard<'static, ()>, +} + +impl TestRepo { + /// Initialize a temporary test repo with git config + ///Change working directory to that repo + /// working directory resets to project/original repo when out of scope. + pub fn new() -> Self { + let guard = SERIAL.lock().unwrap(); + let dir = TempDir::new().expect("failed to create temp dir"); + let path = dir.path(); + let original_dir = env::current_dir().expect("failed to get current dir"); + + // dir with git config + run_git(path, &["init"]); + run_git(path, &["config", "user.email", "test@test.com"]); + run_git(path, &["config", "user.name", "Test"]); + + // set working directory to temorary git repo + env::set_current_dir(path).expect("failed to chdir into test repo"); + + Self { + dir, + original_dir, + _guard: guard, + } + } + + pub fn commit(&self, message: &str) -> String { + let path = self.dir.path(); + let file = path.join("f.txt"); + fs::write(&file, message).unwrap(); + run_git(path, &["add", "."]); + run_git(path, &["commit", "-m", message]); + git_output(path, &["rev-parse", "HEAD"]) + } +} + +// set current dir to project repo when out of scope +impl Drop for TestRepo { + fn drop(&mut self) { + let _ = env::set_current_dir(&self.original_dir); + } +} + +fn run_git(dir: &std::path::Path, args: &[&str]) { + Command::new("git") + .args(args) + .current_dir(dir) + .output() + .expect("git command failed"); +} + +fn git_output(dir: &std::path::Path, args: &[&str]) -> String { + let out = Command::new("git") + .args(args) + .current_dir(dir) + .output() + .expect("git command failed"); + String::from_utf8_lossy(&out.stdout).trim().to_string() +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs new file mode 100644 index 0000000..8c4aa8c --- /dev/null +++ b/tests/common/mod.rs @@ -0,0 +1,3 @@ +pub mod git_repo; + +pub use git_repo::TestRepo; diff --git a/tests/git_helpers.rs b/tests/git_helpers.rs new file mode 100644 index 0000000..ab550d0 --- /dev/null +++ b/tests/git_helpers.rs @@ -0,0 +1,114 @@ +mod common; +#[path = "../src/git_helpers.rs"] +mod git_helpers; + +use common::TestRepo; +use git_helpers::{get_commit_message_from_hash, get_commit_messages_from_hash_range}; + +// ---- get_commit_message_from_hash ---------------------------------------- + +#[test] +fn returns_commit_message_for_known_hash() { + let repo = TestRepo::new(); + let hash = repo.commit("feat: add new feature"); + + let got = get_commit_message_from_hash(&hash).expect("should resolve known hash"); + assert_eq!(got, "feat: add new feature"); +} + +#[test] +fn returns_commit_message_for_head() { + let repo = TestRepo::new(); + repo.commit("fix: repair something"); + + let got = get_commit_message_from_hash("HEAD").expect("HEAD should resolve"); + assert_eq!(got, "fix: repair something"); +} + +#[test] +fn errors_on_unknown_hash() { + let null = "0000000000000000000000000000000000000000"; + assert!( + get_commit_message_from_hash(null).is_err(), + "expected error for unknown hash" + ); +} + +#[test] +fn errors_on_malformed_hash() { + let bad = "not-a-real-hash-zzz"; + assert!( + get_commit_message_from_hash(bad).is_err(), + "expected error for malformed hash" + ); +} + +// ---- get_commit_messages_from_hash_range --------------------------------- + +#[test] +fn range_returns_all_commits_inclusive() { + let repo = TestRepo::new(); + let a = repo.commit("feat: first commit"); + repo.commit("fix: second commit"); + let c = repo.commit("chore: third commit"); + + let messages = get_commit_messages_from_hash_range(&a, &c).expect("range should resolve"); + + assert_eq!(messages.len(), 3); + assert_eq!(messages[0], "feat: first commit"); + assert_eq!(messages[1], "fix: second commit"); + assert_eq!(messages[2], "chore: third commit"); +} + +#[test] +fn range_same_hash_yields_one_message() { + let repo = TestRepo::new(); + let hash = repo.commit("feat: only commit"); + + let messages = + get_commit_messages_from_hash_range(&hash, &hash).expect("same from/to should resolve"); + + assert_eq!( + messages.len(), + 1, + "same from/to should yield exactly one message" + ); + assert_eq!(messages[0], "feat: only commit"); +} + +#[test] +fn range_messages_are_trimmed() { + let repo = TestRepo::new(); + let a = repo.commit("feat: first"); + let b = repo.commit("fix: second"); + + let messages = get_commit_messages_from_hash_range(&a, &b).expect("range should resolve"); + + for msg in &messages { + assert_eq!(msg.as_str(), msg.trim(), "messages must be trimmed"); + } +} + +#[test] +fn range_errors_on_unknown_to_hash() { + let repo = TestRepo::new(); + let a = repo.commit(""); + let null = "0000000000000000000000000000000000000000"; + + assert!( + get_commit_messages_from_hash_range(&a, null).is_err(), + "expected error for unknown to_hash" + ); +} + +#[test] +fn range_errors_on_malformed_from_hash() { + let repo = TestRepo::new(); + repo.commit("feat: a commit"); + let bad = "not-a-real-hash-zzz"; + + assert!( + get_commit_messages_from_hash_range(bad, "HEAD").is_err(), + "expected error for malformed from_hash" + ); +} From 8a2ad0ab40a9bd51eaf5cbb66b7405f619085126 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 19 May 2026 17:33:22 +0545 Subject: [PATCH 03/21] fix(cli): addd strict constrain between cli arguments - from-hash and to-hash are exclusive to : message, hash, file - to-hash requires from-hash and from-hash only --- src/cli.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cli.rs b/src/cli.rs index 1f0f704..24c075b 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -22,6 +22,10 @@ pub struct Cli { #[arg(long)] pub from_hash: Option, - #[arg(long, requires = "from_hash", default_value = "HEAD")] + #[arg( + long, requires = "from_hash", + conflicts_with_all = ["message", "file", "hash"], + default_value = "HEAD" + )] pub to_hash: Option, } From 7f28c761a93602e6bebf7c8bdac6d9ba18d1baf5 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 26 May 2026 21:20:22 +0545 Subject: [PATCH 04/21] refactor(git): use null byte as delimiter for commit messages Replace the hardcoded string delimiter with a null byte (%x00) when fetching commit ranges. This prevents potential parsing collisions if a commit body contains the delimiter string and ensures more reliable splitting of git log output. --- src/git_helpers.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/git_helpers.rs b/src/git_helpers.rs index c33ddec..c997f06 100644 --- a/src/git_helpers.rs +++ b/src/git_helpers.rs @@ -16,7 +16,6 @@ pub fn get_commit_message_from_hash(commit_hash: &str) -> Result { } pub fn get_commit_messages_from_hash_range(from_hash: &str, to_hash: &str) -> Result> { - let delimiter = "========commit-delimiter========"; let range = format!("{}..{}", from_hash, to_hash); // range A..B returns all commits from B but not A ie (B - A), so A has to added seaprately @@ -26,7 +25,7 @@ pub fn get_commit_messages_from_hash_range(from_hash: &str, to_hash: &str) -> Re .args([ "log", "--no-merges", - &format!("--pretty=format:%B{delimiter}"), + "--pretty=format:%B%x00", // null byte for delimiter "--reverse", &range, ]) @@ -47,8 +46,8 @@ pub fn get_commit_messages_from_hash_range(from_hash: &str, to_hash: &str) -> Re } let mut messages: Vec = String::from_utf8_lossy(&output.stdout) - .split(&delimiter) - .map(|s| s.trim()) + .split('\0') + .map(|s| s.trim()) // trim removes the trailing newline git adds to %B .filter(|s| !s.is_empty()) .map(|s| s.to_owned()) .collect(); From d9dcb3d09bc31248b4292e4c810d42359fdfadf7 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Sun, 31 May 2026 21:43:33 +0545 Subject: [PATCH 05/21] style: format using cargo fmt - enforced rust formatting style - passes cargo clippy -D warning --- src/linter.rs | 7 ++----- src/main.rs | 2 +- src/messages.rs | 2 +- src/utils.rs | 8 +++++--- tests/cli.rs | 46 ++++++++++++++++++++++------------------------ 5 files changed, 31 insertions(+), 34 deletions(-) diff --git a/src/linter.rs b/src/linter.rs index 46e8ece..37c0003 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -10,7 +10,7 @@ pub fn lint_commit_message(message: &str) -> bool { ); let re = Regex::new(&pattern).unwrap(); - return re.is_match(message); + re.is_match(message) } #[cfg(test)] @@ -56,10 +56,7 @@ mod tests { #[test] fn accepts_body_separated_by_blank_line() { assert_lint("feat: add new feature\n\nthis is body", true); - assert_lint( - "feat: add new feature\n\nthis is body\n\ntest", - true, - ); + assert_lint("feat: add new feature\n\nthis is body\n\ntest", true); } #[test] diff --git a/src/main.rs b/src/main.rs index df177db..f5d8e21 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,5 +12,5 @@ use cli::Cli; fn main() -> Result<()> { let args = Cli::parse(); - return command::run(args); + command::run(args) } diff --git a/src/messages.rs b/src/messages.rs index ab52059..abb5329 100644 --- a/src/messages.rs +++ b/src/messages.rs @@ -1,2 +1,2 @@ pub const VALIDATION_SUCCESSFUL: &str = "Commit validation: successful!"; -pub const VALIDATION_FAILED: &str = "Commit validation: failed!"; \ No newline at end of file +pub const VALIDATION_FAILED: &str = "Commit validation: failed!"; diff --git a/src/utils.rs b/src/utils.rs index dca9c90..0031c74 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -11,11 +11,11 @@ pub fn is_ignored(message: &str) -> bool { return true; } } - return false; + false } pub fn is_empty(msg: &str) -> bool { - return msg.trim().is_empty(); + msg.trim().is_empty() } #[cfg(test)] @@ -52,7 +52,9 @@ mod tests { fn ignores_bitbucket_style_merged() { assert!(is_ignored("Merged bugfix-789 in master")); assert!(is_ignored("Merged PR #987: Update documentation")); - assert!(is_ignored("Merged PR #321: Bugfix - Resolve issue with login")); + assert!(is_ignored( + "Merged PR #321: Bugfix - Resolve issue with login" + )); } #[test] diff --git a/tests/cli.rs b/tests/cli.rs index 386b0ae..f269d50 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -208,7 +208,7 @@ fn invalid_hash_fails() { fn hash_exact_valid_commit_succeeds() { let repo = TestRepo::new(); let hash = repo.commit("feat: add new feature"); - + cocox() .arg("--hash") .arg(&hash) @@ -216,12 +216,12 @@ fn hash_exact_valid_commit_succeeds() { .success() .stdout(predicate::str::contains("Commit validation: successful!")); } - + #[test] fn hash_invalid_commit_message_fails() { let repo = TestRepo::new(); let hash = repo.commit("not a conventional commit"); - + cocox() .arg("--hash") .arg(&hash) @@ -230,12 +230,12 @@ fn hash_invalid_commit_message_fails() { .code(1) .stderr(predicate::str::contains("Commit validation: failed!")); } - + #[test] fn hash_ignored_commit_silently_succeeds() { let repo = TestRepo::new(); let hash = repo.commit("Merge pull request #123"); - + cocox() .arg("--hash") .arg(&hash) @@ -267,15 +267,14 @@ fn hash_range_valid_commits_succeeds() { .stdout(predicate::str::contains("Commit validation: successful!")); } - #[test] fn hash_range_invalid_commit_in_middle_fails() { let repo = TestRepo::new(); - + let a = repo.commit("feat: add new feature"); repo.commit("not a conventional commit"); let c = repo.commit("fix: fix a bug"); - + cocox() .arg("--from-hash") .arg(&a) @@ -286,14 +285,14 @@ fn hash_range_invalid_commit_in_middle_fails() { .code(1) .stderr(predicate::str::contains("Commit validation: failed!")); } - + #[test] fn hash_range_invalid_from_commit_fails() { let repo = TestRepo::new(); - + let a = repo.commit("not a conventional commit"); let b = repo.commit("feat: add new feature"); - + cocox() .arg("--from-hash") .arg(&a) @@ -304,14 +303,14 @@ fn hash_range_invalid_from_commit_fails() { .code(1) .stderr(predicate::str::contains("Commit validation: failed!")); } - + #[test] fn hash_range_invalid_to_commit_fails() { let repo = TestRepo::new(); - + let a = repo.commit("feat: add new feature"); let b = repo.commit("not a conventional commit"); - + cocox() .arg("--from-hash") .arg(&a) @@ -322,15 +321,15 @@ fn hash_range_invalid_to_commit_fails() { .code(1) .stderr(predicate::str::contains("Commit validation: failed!")); } - + #[test] fn hash_range_ignored_commits_are_skipped() { let repo = TestRepo::new(); - + let a = repo.commit("feat: add new feature"); repo.commit("Merge pull request #123"); let c = repo.commit("fix: fix a bug"); - + cocox() .arg("--from-hash") .arg(&a) @@ -340,13 +339,13 @@ fn hash_range_ignored_commits_are_skipped() { .success() .stdout(predicate::str::contains("Commit validation: successful!")); } - + #[test] fn hash_range_single_commit_succeeds() { let repo = TestRepo::new(); - + let a = repo.commit("feat: single commit"); - + cocox() .arg("--from-hash") .arg(&a) @@ -356,13 +355,13 @@ fn hash_range_single_commit_succeeds() { .success() .stdout(predicate::str::contains("Commit validation: successful!")); } - + #[test] fn hash_range_single_invalid_commit_fails() { let repo = TestRepo::new(); - + let a = repo.commit("not a conventional commit"); - + cocox() .arg("--from-hash") .arg(&a) @@ -374,7 +373,6 @@ fn hash_range_single_invalid_commit_fails() { .stderr(predicate::str::contains("Commit validation: failed!")); } - #[test] fn from_hash_only_valid_commits_succeeds() { let repo = TestRepo::new(); From 401e862480c633c51a9bdc868f42bf70718d7f97 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Mon, 1 Jun 2026 00:23:23 +0545 Subject: [PATCH 06/21] refactor: update git_repo and git_helpers removed manual mutex, use #[serial] attribute to avoid race condition check the return status for the success of respective git command refactored: use std::process::Command for instatiation. most importantly, fixed typos --- tests/cli.rs | 17 +++++++++++-- tests/common/git_repo.rs | 52 ++++++++++++++++++++++++---------------- tests/git_helpers.rs | 10 +++++++- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/tests/cli.rs b/tests/cli.rs index f269d50..b116a3c 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -3,6 +3,7 @@ mod common; use assert_cmd::Command; use common::TestRepo; use predicates::prelude::*; +use serial_test::serial; use std::io::Write; use tempfile::NamedTempFile; @@ -180,6 +181,7 @@ fn missing_file_fails() { // --- --hash --------------------------------------------------------------- #[test] +#[serial] fn hash_head_valid_commit_succeeds() { let repo = TestRepo::new(); repo.commit("feat: add new feature"); @@ -205,6 +207,7 @@ fn invalid_hash_fails() { } #[test] +#[serial] fn hash_exact_valid_commit_succeeds() { let repo = TestRepo::new(); let hash = repo.commit("feat: add new feature"); @@ -218,6 +221,7 @@ fn hash_exact_valid_commit_succeeds() { } #[test] +#[serial] fn hash_invalid_commit_message_fails() { let repo = TestRepo::new(); let hash = repo.commit("not a conventional commit"); @@ -232,6 +236,7 @@ fn hash_invalid_commit_message_fails() { } #[test] +#[serial] fn hash_ignored_commit_silently_succeeds() { let repo = TestRepo::new(); let hash = repo.commit("Merge pull request #123"); @@ -248,6 +253,7 @@ fn hash_ignored_commit_silently_succeeds() { // ---- hash range ---------------------------------------------------------- #[test] +#[serial] fn hash_range_valid_commits_succeeds() { let repo = TestRepo::new(); @@ -255,7 +261,7 @@ fn hash_range_valid_commits_succeeds() { repo.commit("fix: fix a bug"); repo.commit("fix: fix another bug"); repo.commit("feat: add another feature"); - let c = repo.commit("perf: improve performence"); + let c = repo.commit("perf: improve performance"); cocox() .arg("--from-hash") @@ -268,6 +274,7 @@ fn hash_range_valid_commits_succeeds() { } #[test] +#[serial] fn hash_range_invalid_commit_in_middle_fails() { let repo = TestRepo::new(); @@ -287,6 +294,7 @@ fn hash_range_invalid_commit_in_middle_fails() { } #[test] +#[serial] fn hash_range_invalid_from_commit_fails() { let repo = TestRepo::new(); @@ -305,6 +313,7 @@ fn hash_range_invalid_from_commit_fails() { } #[test] +#[serial] fn hash_range_invalid_to_commit_fails() { let repo = TestRepo::new(); @@ -323,6 +332,7 @@ fn hash_range_invalid_to_commit_fails() { } #[test] +#[serial] fn hash_range_ignored_commits_are_skipped() { let repo = TestRepo::new(); @@ -341,6 +351,7 @@ fn hash_range_ignored_commits_are_skipped() { } #[test] +#[serial] fn hash_range_single_commit_succeeds() { let repo = TestRepo::new(); @@ -357,6 +368,7 @@ fn hash_range_single_commit_succeeds() { } #[test] +#[serial] fn hash_range_single_invalid_commit_fails() { let repo = TestRepo::new(); @@ -374,6 +386,7 @@ fn hash_range_single_invalid_commit_fails() { } #[test] +#[serial] fn from_hash_only_valid_commits_succeeds() { let repo = TestRepo::new(); @@ -381,7 +394,7 @@ fn from_hash_only_valid_commits_succeeds() { repo.commit("fix: fix a bug"); repo.commit("fix: fix another bug"); repo.commit("feat: add another feature"); - repo.commit("perf: improve performence"); + repo.commit("perf: improve performance"); cocox() .arg("--from-hash") diff --git a/tests/common/git_repo.rs b/tests/common/git_repo.rs index 898046f..022fe69 100644 --- a/tests/common/git_repo.rs +++ b/tests/common/git_repo.rs @@ -1,23 +1,24 @@ -use assert_cmd::Command; use std::env; use std::fs; use std::path::PathBuf; -use std::sync::{Mutex, MutexGuard}; +use std::process::Command; use tempfile::TempDir; -static SERIAL: Mutex<()> = Mutex::new(()); pub struct TestRepo { dir: TempDir, original_dir: PathBuf, - _guard: MutexGuard<'static, ()>, } impl TestRepo { - /// Initialize a temporary test repo with git config - ///Change working directory to that repo - /// working directory resets to project/original repo when out of scope. + /// Initialize a temporary test repo with git config. + /// + /// ## Important: Process-Global State + /// This function changes the current working directory of the **entire process** /// to the temporary repository. Because Cargo runs tests in parallel threads within + /// the same process, **any test using `TestRepo` must be marked with `#[serial]`** /// from the `serial_test` crate to prevent race conditions and cross-test interference. + /// + /// The working directory will automatically reset back to the original project + /// directory when this `TestRepo` goes out of scope (via `Drop`). pub fn new() -> Self { - let guard = SERIAL.lock().unwrap(); let dir = TempDir::new().expect("failed to create temp dir"); let path = dir.path(); let original_dir = env::current_dir().expect("failed to get current dir"); @@ -27,14 +28,10 @@ impl TestRepo { run_git(path, &["config", "user.email", "test@test.com"]); run_git(path, &["config", "user.name", "Test"]); - // set working directory to temorary git repo + // set working directory to temporary git repo env::set_current_dir(path).expect("failed to chdir into test repo"); - Self { - dir, - original_dir, - _guard: guard, - } + Self { dir, original_dir } } pub fn commit(&self, message: &str) -> String { @@ -55,18 +52,33 @@ impl Drop for TestRepo { } fn run_git(dir: &std::path::Path, args: &[&str]) { - Command::new("git") + let status = Command::new("git") .args(args) .current_dir(dir) - .output() - .expect("git command failed"); + .status() + .expect("failed to execute git process"); + + assert!( + status.success(), + "git command '{}' failed with {:#}", + args.join(" "), + status + ); } fn git_output(dir: &std::path::Path, args: &[&str]) -> String { - let out = Command::new("git") + let output = Command::new("git") .args(args) .current_dir(dir) .output() - .expect("git command failed"); - String::from_utf8_lossy(&out.stdout).trim().to_string() + .expect("failed to execute git process"); + + if !output.status.success() { + panic!( + "git command failed with status: {}\nstderr: {}", + output.status, + String::from_utf8_lossy(&output.stderr) + ); + } + String::from_utf8_lossy(&output.stdout).trim().to_string() } diff --git a/tests/git_helpers.rs b/tests/git_helpers.rs index ab550d0..94474f5 100644 --- a/tests/git_helpers.rs +++ b/tests/git_helpers.rs @@ -4,10 +4,12 @@ mod git_helpers; use common::TestRepo; use git_helpers::{get_commit_message_from_hash, get_commit_messages_from_hash_range}; +use serial_test::serial; // ---- get_commit_message_from_hash ---------------------------------------- #[test] +#[serial] fn returns_commit_message_for_known_hash() { let repo = TestRepo::new(); let hash = repo.commit("feat: add new feature"); @@ -17,6 +19,7 @@ fn returns_commit_message_for_known_hash() { } #[test] +#[serial] fn returns_commit_message_for_head() { let repo = TestRepo::new(); repo.commit("fix: repair something"); @@ -46,6 +49,7 @@ fn errors_on_malformed_hash() { // ---- get_commit_messages_from_hash_range --------------------------------- #[test] +#[serial] fn range_returns_all_commits_inclusive() { let repo = TestRepo::new(); let a = repo.commit("feat: first commit"); @@ -61,6 +65,7 @@ fn range_returns_all_commits_inclusive() { } #[test] +#[serial] fn range_same_hash_yields_one_message() { let repo = TestRepo::new(); let hash = repo.commit("feat: only commit"); @@ -77,6 +82,7 @@ fn range_same_hash_yields_one_message() { } #[test] +#[serial] fn range_messages_are_trimmed() { let repo = TestRepo::new(); let a = repo.commit("feat: first"); @@ -90,9 +96,10 @@ fn range_messages_are_trimmed() { } #[test] +#[serial] fn range_errors_on_unknown_to_hash() { let repo = TestRepo::new(); - let a = repo.commit(""); + let a = repo.commit("feat: initial commit"); let null = "0000000000000000000000000000000000000000"; assert!( @@ -102,6 +109,7 @@ fn range_errors_on_unknown_to_hash() { } #[test] +#[serial] fn range_errors_on_malformed_from_hash() { let repo = TestRepo::new(); repo.commit("feat: a commit"); From 9be6118f0bfb07142a70df589739896749daf28c Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Mon, 1 Jun 2026 01:34:10 +0545 Subject: [PATCH 07/21] chore: better comment --- src/git_helpers.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/git_helpers.rs b/src/git_helpers.rs index c997f06..1fe42dc 100644 --- a/src/git_helpers.rs +++ b/src/git_helpers.rs @@ -18,7 +18,9 @@ pub fn get_commit_message_from_hash(commit_hash: &str) -> Result { pub fn get_commit_messages_from_hash_range(from_hash: &str, to_hash: &str) -> Result> { let range = format!("{}..{}", from_hash, to_hash); - // range A..B returns all commits from B but not A ie (B - A), so A has to added seaprately + // git's A..B range is exclusive of A (returns commits reachable from B + // but not from A). To make the range inclusive we fetch A's message + // separately and prepend it below. let from_hash_message = get_commit_message_from_hash(from_hash)?; let output = Command::new("git") From f90cf5314b5c02eb627eed35a2d8a262e1482f26 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Mon, 1 Jun 2026 01:32:04 +0545 Subject: [PATCH 08/21] refactor: make to-hash string to-hash doesn't need to be wrapped around Option, since it'll always have a value --- src/cli.rs | 2 +- src/command.rs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 24c075b..618c087 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -27,5 +27,5 @@ pub struct Cli { conflicts_with_all = ["message", "file", "hash"], default_value = "HEAD" )] - pub to_hash: Option, + pub to_hash: String, } diff --git a/src/command.rs b/src/command.rs index 866c5bf..28ff2a1 100644 --- a/src/command.rs +++ b/src/command.rs @@ -69,14 +69,16 @@ pub fn run(args: Cli) -> Result<()> { let msg = read_file(file)?; handle_commit_message(&msg); } else if let Some(hash) = &args.hash { + // commit hash let msg = get_commit_message_from_hash(hash)?; handle_commit_message(&msg); } else if let Some(from_hash) = &args.from_hash { - let to_hash = &args.to_hash.unwrap(); + // commit hash range + let to_hash = &args.to_hash; let messages = get_commit_messages_from_hash_range(from_hash, to_hash)?; handle_multiple_commit_messages(&messages); } else { - unreachable!("invalid option is handle by clap"); + unreachable!("invalid option is handled by clap"); } Ok(()) From e7e582cd5e8535a4aa6bd132c457c7e9b1ef4f65 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Mon, 1 Jun 2026 12:15:08 +0545 Subject: [PATCH 09/21] perf: complile regex patterns globally and only once --- src/linter.rs | 10 +++++++--- src/utils.rs | 18 +++++++++--------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/linter.rs b/src/linter.rs index 37c0003..4ee94c8 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -1,7 +1,8 @@ use crate::constants::COMMIT_TYPES; use regex::Regex; +use std::sync::LazyLock; -pub fn lint_commit_message(message: &str) -> bool { +static LINT_REGEX: LazyLock = LazyLock::new(|| { let re_types = COMMIT_TYPES.join("|"); let pattern = format!( @@ -9,8 +10,11 @@ pub fn lint_commit_message(message: &str) -> bool { re_types ); - let re = Regex::new(&pattern).unwrap(); - re.is_match(message) + Regex::new(&pattern).unwrap() +}); + +pub fn lint_commit_message(message: &str) -> bool { + LINT_REGEX.is_match(message) } #[cfg(test)] diff --git a/src/utils.rs b/src/utils.rs index 0031c74..cf5a003 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,17 +1,17 @@ use crate::constants::IGNORE_COMMIT_PATTERNS; use regex::Regex; +use std::sync::LazyLock; + +static IGNORE_REGEXES: LazyLock> = LazyLock::new(|| { + IGNORE_COMMIT_PATTERNS + .iter() + .map(|&pat| Regex::new(pat).unwrap()) + .collect() +}); pub fn is_ignored(message: &str) -> bool { let message = message.lines().next().unwrap(); - - for pattern in IGNORE_COMMIT_PATTERNS { - let re = Regex::new(pattern).unwrap(); - - if re.is_match(message) { - return true; - } - } - false + IGNORE_REGEXES.iter().any(|re| re.is_match(message)) } pub fn is_empty(msg: &str) -> bool { From 69ff95238b656dd28c63bc8922a2336f504b5f7e Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Mon, 1 Jun 2026 14:10:30 +0545 Subject: [PATCH 10/21] refactor: improve linter feedback and reorganize test suite - Update `lint_commit_message` to return a structured `LintOutcome` enum. - Add descriptive console output and banners in `command.rs` for valid, invalid, and ignored messages. - Refactor unit tests in `linter.rs` to test the internal validator directly while adding coverage for the public API. - Update CLI integration tests to assert against the new stdout/stderr output strings. - Ensure consistent exit codes and error messaging across all input types (file, hash, message). --- src/command.rs | 63 ++++++++++---------- src/linter.rs | 154 ++++++++++++++++++++++++++++++++++--------------- tests/cli.rs | 6 +- 3 files changed, 143 insertions(+), 80 deletions(-) diff --git a/src/command.rs b/src/command.rs index 28ff2a1..f245a2e 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1,8 +1,7 @@ use crate::cli::Cli; use crate::git_helpers::{get_commit_message_from_hash, get_commit_messages_from_hash_range}; -use crate::linter::lint_commit_message; +use crate::linter::{LintOutcome, lint_commit_message}; use crate::messages::{VALIDATION_FAILED, VALIDATION_SUCCESSFUL}; -use crate::utils::{is_empty, is_ignored}; use anyhow::{Context, Result}; fn read_file(file: &String) -> Result { @@ -11,44 +10,44 @@ fn read_file(file: &String) -> Result { } fn handle_commit_message(msg: &str) { - if is_empty(msg) { - eprintln!( - "{}: Aborting commit due to empty commit message", - VALIDATION_FAILED - ); - std::process::exit(1); - } - - if is_ignored(msg) { - return; - } + match lint_commit_message(msg) { + LintOutcome::Empty => { + eprintln!( + "{}: Aborting commit due to empty commit message\n", + VALIDATION_FAILED + ); + std::process::exit(1); + } + LintOutcome::Ignored => println!("commit message ignored, skipping lint\n"), - let success = lint_commit_message(msg); - if success { - println!("{}", VALIDATION_SUCCESSFUL); - return; + LintOutcome::Valid => { + println!("{}\n", VALIDATION_SUCCESSFUL); + } + LintOutcome::Invalid => { + eprintln!("{}\n", VALIDATION_FAILED); + std::process::exit(1); + } } - - eprintln!("{}", VALIDATION_FAILED); - std::process::exit(1); } fn handle_multiple_commit_messages(messages: &[String]) { let mut has_failure = false; for msg in messages { - if is_empty(msg) { - continue; - } - - if is_ignored(msg) { - continue; - } - - let success = lint_commit_message(msg); - if !success { - has_failure = true; - // TODO: Log proper error for individual message + match lint_commit_message(msg) { + LintOutcome::Empty => { + eprintln!( + "{}: Aborting commit due to empty commit message\n", + VALIDATION_FAILED + ); + std::process::exit(1); + } + LintOutcome::Ignored => println!("commit message ignored, skipping lint\n"), + LintOutcome::Valid => println!("lint success\n"), + LintOutcome::Invalid => { + has_failure = true; + eprintln!("Lint failed \n") + } } } diff --git a/src/linter.rs b/src/linter.rs index 4ee94c8..5602e86 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -1,4 +1,5 @@ use crate::constants::COMMIT_TYPES; +use crate::utils::{is_empty, is_ignored}; use regex::Regex; use std::sync::LazyLock; @@ -13,133 +14,196 @@ static LINT_REGEX: LazyLock = LazyLock::new(|| { Regex::new(&pattern).unwrap() }); -pub fn lint_commit_message(message: &str) -> bool { +#[derive(Debug, PartialEq, Eq)] +pub enum LintOutcome { + Valid, + Invalid, + Ignored, + Empty, +} + +/// Evaluates a commit message, matches the conventional commit format +/// and returns its linting outcome. +pub fn lint_commit_message(message: &str) -> LintOutcome { + println!( + "========= Linting Message:: {} ===========", + message.lines().next().unwrap_or("") + ); + if is_empty(message) { + LintOutcome::Empty + } else if is_ignored(message) { + LintOutcome::Ignored + } else if validate_message(message) { + LintOutcome::Valid + } else { + LintOutcome::Invalid + } +} + +fn validate_message(message: &str) -> bool { LINT_REGEX.is_match(message) } #[cfg(test)] mod tests { use super::*; - - fn assert_lint(message: &str, expected: bool) { - let got = lint_commit_message(message); - assert_eq!( - got, expected, - "lint_commit_message({:?}) returned {}, expected {}", - message, got, expected - ); - } - #[test] fn accepts_basic_conventional_commit() { - assert_lint("feat: add new feature", true); + assert!(validate_message("feat: add new feature")); } #[test] fn accepts_every_known_commit_type() { for kind in COMMIT_TYPES { - assert_lint(&format!("{}: do the thing", kind), true); + assert!(validate_message(&format!("{}: do the thing", kind))); } } #[test] fn accepts_commit_with_scope() { - assert_lint("feat(parser): add new feature", true); - assert_lint( - "build(deps-dev): bump @babel/traverse from 7.22.17 to 7.24.0", - true, - ); + assert!(validate_message("feat(parser): add new feature")); + assert!(validate_message( + "build(deps-dev): bump @babel/traverse from 7.22.17 to 7.24.0" + )); } #[test] fn accepts_breaking_change_marker() { - assert_lint("feat!: breaking feature", true); - assert_lint("feat(api)!: breaking feature", true); + assert!(validate_message("feat!: breaking feature")); + assert!(validate_message("feat(api)!: breaking feature")); } #[test] fn accepts_body_separated_by_blank_line() { - assert_lint("feat: add new feature\n\nthis is body", true); - assert_lint("feat: add new feature\n\nthis is body\n\ntest", true); + assert!(validate_message("feat: add new feature\n\nthis is body")); + assert!(validate_message( + "feat: add new feature\n\nthis is body\n\ntest" + )); } #[test] fn accepts_trailing_newline() { - assert_lint("feat: add new feature\n", true); + assert!(validate_message("feat: add new feature\n")); } #[test] fn rejects_empty_message() { - assert_lint("", false); + assert!(!validate_message("")); } #[test] fn rejects_missing_colon() { - assert_lint("feat add new feature", false); + assert!(!validate_message("feat add new feature")); } #[test] fn rejects_missing_type() { - assert_lint(": add new feature", false); - assert_lint("(invalid): add new feature", false); + assert!(!validate_message(": add new feature")); + assert!(!validate_message("(invalid): add new feature")); } #[test] fn rejects_unknown_type() { - assert_lint("invalid: add new feature", false); - assert_lint("foo(bar): add new feature", false); + assert!(!validate_message("invalid: add new feature")); + assert!(!validate_message("foo(bar): add new feature")); } #[test] fn rejects_space_between_type_and_scope() { - assert_lint("feat (test): add new feature", false); + assert!(!validate_message("feat (test): add new feature")); } #[test] fn rejects_empty_or_whitespace_scope() { - assert_lint("feat(): add new feature", false); - assert_lint("feat( ): add new feature", false); - assert_lint("feat(hello world): add new feature", false); + assert!(!validate_message("feat(): add new feature")); + assert!(!validate_message("feat( ): add new feature")); + assert!(!validate_message("feat(hello world): add new feature")); } #[test] fn rejects_space_between_scope_and_colon() { - assert_lint("feat(test) : add new feature", false); + assert!(!validate_message("feat(test) : add new feature")); } #[test] fn rejects_description_without_leading_space() { - assert_lint("feat:add new feature", false); + assert!(!validate_message("feat:add new feature")); } #[test] fn rejects_description_with_extra_leading_space() { - assert_lint("feat: add new feature", false); + assert!(!validate_message("feat: add new feature")); } #[test] fn rejects_line_break_inside_description() { - assert_lint("feat: add new feature\nhello baby", false); + assert!(!validate_message("feat: add new feature\nhello baby")); } #[test] fn rejects_missing_description() { - assert_lint("feat(test):", false); - assert_lint("feat(test): ", false); + assert!(!validate_message("feat(test):")); + assert!(!validate_message("feat(test): ")); } #[test] fn rejects_description_with_trailing_period() { - assert_lint("feat(test): add new feature.", false); + assert!(!validate_message("feat(test): add new feature.")); } #[test] fn rejects_ignore_style_messages() { // The linter itself does not know about ignore patterns; those are - // short-circuited one layer up in `command::handle_commit_message` via - // `utils::is_ignored`. The regex below should therefore reject these. - assert_lint("Merge pull request #123", false); - assert_lint("Bump urllib3 from 1.26.5 to 1.26.17", false); - assert_lint("Initial commit", false); + // short-circuited one layer up in `lint_commit_message` via + // `is_ignored`. The regex below should therefore reject these. + assert!(!validate_message("Merge pull request #123")); + assert!(!validate_message("Bump urllib3 from 1.26.5 to 1.26.17")); + assert!(!validate_message("Initial commit")); + } + + // ---- lint_commit_message tests ------------------------------------------ + + #[test] + fn returns_outcome_empty() { + assert_eq!(lint_commit_message(""), LintOutcome::Empty); + assert_eq!(lint_commit_message(" "), LintOutcome::Empty); + assert_eq!(lint_commit_message("\n\n"), LintOutcome::Empty); + } + + #[test] + fn returns_outcome_ignored() { + assert_eq!( + lint_commit_message("Merge branch 'main' into develop"), + LintOutcome::Ignored + ); + assert_eq!(lint_commit_message("Initial commit"), LintOutcome::Ignored); + assert_eq!( + lint_commit_message("Revert \"feat: add something\""), + LintOutcome::Ignored + ); + } + + #[test] + fn returns_outcome_valid() { + assert_eq!( + lint_commit_message("feat: add new feature"), + LintOutcome::Valid + ); + assert_eq!( + lint_commit_message("fix(parser): handle empty input"), + LintOutcome::Valid + ); + } + + #[test] + fn returns_outcome_invalid() { + assert_eq!( + lint_commit_message("not a conventional commit"), + LintOutcome::Invalid + ); + assert_eq!( + lint_commit_message("feat: trailing period."), + LintOutcome::Invalid + ); } } diff --git a/tests/cli.rs b/tests/cli.rs index b116a3c..6faffcc 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -95,7 +95,7 @@ fn merge_commit_is_ignored() { .arg("Merge pull request #123") .assert() .success() - .stdout(predicate::str::is_empty()) + .stdout(predicate::str::contains("commit message ignored")) .stderr(predicate::str::is_empty()); } @@ -105,7 +105,7 @@ fn dependabot_bump_is_ignored() { .arg("Bump urllib3 from 1.26.5 to 1.26.17") .assert() .success() - .stdout(predicate::str::is_empty()); + .stdout(predicate::str::contains("commit message ignored")); } #[test] @@ -246,7 +246,7 @@ fn hash_ignored_commit_silently_succeeds() { .arg(&hash) .assert() .success() - .stdout(predicate::str::is_empty()) + .stdout(predicate::str::contains("commit message ignored")) .stderr(predicate::str::is_empty()); } From 16c5e498c96e54c7003505aa53637a18c18c1dbe Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Mon, 1 Jun 2026 16:59:21 +0545 Subject: [PATCH 11/21] docs: update readme for new options --- README.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/README.md b/README.md index be8fceb..c9339bf 100644 --- a/README.md +++ b/README.md @@ -6,10 +6,41 @@ A Conventional Commitlint binary tool (x for executable). ## Development + +## Usage: +```shell +A Conventional Commitlint binary tool + +Usage: cocox [OPTIONS] |--hash |--from-hash > + +Arguments: + [MESSAGE] + +Options: + --file + --hash + --from-hash + --to-hash [default: HEAD] + -h, --help Print help + -V, --version Print version +``` + + Run cocox ```shell cargo run -- "my commit message" cargo run -- --file "/foo/bar" + + +# using commit hash +cargo run -- --hash xxxx-xxxx + +# using range of hashes: +cargo run -- --from-hash xxxx-xxxx --to-hash xxxx-xxxx + +# --to-hash points to HEAD by default +cargo run -- --from-hash xxxx-xxxx + ``` From 85edbf92a48c35d3968014d652fd2c9bb8683cda Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 2 Jun 2026 03:27:45 +0545 Subject: [PATCH 12/21] perf: use RegexSet to match IGNORED_REGEXES in single search --- src/utils.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index cf5a003..c4c15e8 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,17 +1,15 @@ use crate::constants::IGNORE_COMMIT_PATTERNS; -use regex::Regex; +use regex::RegexSet; use std::sync::LazyLock; -static IGNORE_REGEXES: LazyLock> = LazyLock::new(|| { - IGNORE_COMMIT_PATTERNS - .iter() - .map(|&pat| Regex::new(pat).unwrap()) - .collect() -}); +static IGNORE_SET: LazyLock = + LazyLock::new(|| RegexSet::new(IGNORE_COMMIT_PATTERNS).unwrap()); pub fn is_ignored(message: &str) -> bool { - let message = message.lines().next().unwrap(); - IGNORE_REGEXES.iter().any(|re| re.is_match(message)) + message + .lines() + .next() + .is_some_and(|line| IGNORE_SET.is_match(line)) } pub fn is_empty(msg: &str) -> bool { From 4434feb5c1f871a0bc0a6c2f38ae3dd5d136dd61 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 2 Jun 2026 03:39:08 +0545 Subject: [PATCH 13/21] refactor: split project into library and binary crates Moves core logic from `src/main.rs` into a new `src/lib.rs` and updates the binary to act as a thin wrapper. --- src/lib.rs | 7 +++++++ src/main.rs | 11 ++--------- tests/git_helpers.rs | 4 +--- 3 files changed, 10 insertions(+), 12 deletions(-) create mode 100644 src/lib.rs diff --git a/src/lib.rs b/src/lib.rs new file mode 100644 index 0000000..21d205e --- /dev/null +++ b/src/lib.rs @@ -0,0 +1,7 @@ +pub mod cli; +pub mod command; +pub mod constants; +pub mod git_helpers; +pub mod linter; +pub mod messages; +pub mod utils; diff --git a/src/main.rs b/src/main.rs index f5d8e21..3cd1a39 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,14 +1,7 @@ -mod cli; -mod command; -mod constants; -mod git_helpers; -mod linter; -mod messages; -mod utils; - use anyhow::Result; use clap::Parser; -use cli::Cli; +use cocox::cli::Cli; +use cocox::command; fn main() -> Result<()> { let args = Cli::parse(); diff --git a/tests/git_helpers.rs b/tests/git_helpers.rs index 94474f5..071a67a 100644 --- a/tests/git_helpers.rs +++ b/tests/git_helpers.rs @@ -1,9 +1,7 @@ mod common; -#[path = "../src/git_helpers.rs"] -mod git_helpers; +use cocox::git_helpers::{get_commit_message_from_hash, get_commit_messages_from_hash_range}; use common::TestRepo; -use git_helpers::{get_commit_message_from_hash, get_commit_messages_from_hash_range}; use serial_test::serial; // ---- get_commit_message_from_hash ---------------------------------------- From 755778fd6e47c9e8e2a6fde02963494dd3b77ddc Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 2 Jun 2026 05:06:39 +0545 Subject: [PATCH 14/21] docs: add descriptive rustdoc for git_commit_messsages_from_hash_range --- src/git_helpers.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/git_helpers.rs b/src/git_helpers.rs index 1fe42dc..71d8522 100644 --- a/src/git_helpers.rs +++ b/src/git_helpers.rs @@ -15,6 +15,8 @@ pub fn get_commit_message_from_hash(commit_hash: &str) -> Result { Ok(message) } +/// Returns commit messages for the range `from_hash..=to_hash`, inclusive on both ends, +/// in chronological (oldest-first) order. Merge commits are excluded via `--no-merges`. pub fn get_commit_messages_from_hash_range(from_hash: &str, to_hash: &str) -> Result> { let range = format!("{}..{}", from_hash, to_hash); From 5564bd37c56d6875d9d53c52107bb36e82157513 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 2 Jun 2026 05:36:22 +0545 Subject: [PATCH 15/21] refactor: use git log ^.. to - check if has parent, if yes run git log ^.. - if not, run git log . --- src/git_helpers.rs | 30 ++++++++++++++++++++---------- tests/git_helpers.rs | 25 ++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/src/git_helpers.rs b/src/git_helpers.rs index 71d8522..a8a1258 100644 --- a/src/git_helpers.rs +++ b/src/git_helpers.rs @@ -18,12 +18,11 @@ pub fn get_commit_message_from_hash(commit_hash: &str) -> Result { /// Returns commit messages for the range `from_hash..=to_hash`, inclusive on both ends, /// in chronological (oldest-first) order. Merge commits are excluded via `--no-merges`. pub fn get_commit_messages_from_hash_range(from_hash: &str, to_hash: &str) -> Result> { - let range = format!("{}..{}", from_hash, to_hash); - - // git's A..B range is exclusive of A (returns commits reachable from B - // but not from A). To make the range inclusive we fetch A's message - // separately and prepend it below. - let from_hash_message = get_commit_message_from_hash(from_hash)?; + let range = if is_orphan(from_hash)? { + to_hash.to_string() + } else { + format!("{}^..{}", from_hash, to_hash) + }; let output = Command::new("git") .args([ @@ -49,15 +48,26 @@ pub fn get_commit_messages_from_hash_range(from_hash: &str, to_hash: &str) -> Re ); } - let mut messages: Vec = String::from_utf8_lossy(&output.stdout) + let messages: Vec = String::from_utf8_lossy(&output.stdout) .split('\0') .map(|s| s.trim()) // trim removes the trailing newline git adds to %B .filter(|s| !s.is_empty()) .map(|s| s.to_owned()) .collect(); - // prepend the first message of the range - messages.insert(0, from_hash_message); - Ok(messages) } + +pub fn is_orphan(hash: &str) -> Result { + let output = Command::new("git") + .args(["rev-list", "--parents", "-n", "1", hash]) + .output() + .with_context(|| format!("failed to check parent for hash {}", hash))?; + + if !output.status.success() { + anyhow::bail!("failed to check parent for hash {}", hash); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + Ok(stdout.split_whitespace().count() == 1) +} diff --git a/tests/git_helpers.rs b/tests/git_helpers.rs index 071a67a..9b66f22 100644 --- a/tests/git_helpers.rs +++ b/tests/git_helpers.rs @@ -1,6 +1,8 @@ mod common; -use cocox::git_helpers::{get_commit_message_from_hash, get_commit_messages_from_hash_range}; +use cocox::git_helpers::{ + get_commit_message_from_hash, get_commit_messages_from_hash_range, is_orphan, +}; use common::TestRepo; use serial_test::serial; @@ -118,3 +120,24 @@ fn range_errors_on_malformed_from_hash() { "expected error for malformed from_hash" ); } + +// ---- is_orphan -------------------------------------------------------- + +#[test] +#[serial] +fn is_orphan_returns_true_for_root_commit() { + let repo = TestRepo::new(); + let hash = repo.commit("initial commit"); + + assert!(is_orphan(&hash).expect("should check root commit")); +} + +#[test] +#[serial] +fn is_orphan_returns_false_for_child_commit() { + let repo = TestRepo::new(); + repo.commit("initial commit"); + let hash = repo.commit("second commit"); + + assert!(!is_orphan(&hash).expect("should check child commit")); +} From bb1a5c2fd5a4c4b090090e5e5579d6b90eabca84 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Sun, 7 Jun 2026 00:50:08 +0545 Subject: [PATCH 16/21] docs: move usage section down --- README.md | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index c9339bf..35a69d4 100644 --- a/README.md +++ b/README.md @@ -6,28 +6,7 @@ A Conventional Commitlint binary tool (x for executable). ## Development - -## Usage: -```shell -A Conventional Commitlint binary tool - -Usage: cocox [OPTIONS] |--hash |--from-hash > - -Arguments: - [MESSAGE] - -Options: - --file - --hash - --from-hash - --to-hash [default: HEAD] - -h, --help Print help - -V, --version Print version -``` - - Run cocox - ```shell cargo run -- "my commit message" @@ -44,3 +23,21 @@ cargo run -- --from-hash xxxx-xxxx --to-hash xxxx-xxxx cargo run -- --from-hash xxxx-xxxx ``` + +## Usage: +```shell +A Conventional Commitlint binary tool + +Usage: cocox [OPTIONS] |--hash |--from-hash > + +Arguments: + [MESSAGE] + +Options: + --file + --hash + --from-hash + --to-hash [default: HEAD] + -h, --help Print help + -V, --version Print version +``` \ No newline at end of file From f6b7648ebe9045680ed9cadbbe8b9418ffa542bd Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 9 Jun 2026 16:13:50 +0545 Subject: [PATCH 17/21] remove log redirection to stdout and stderr --- src/command.rs | 16 ++------ src/linter.rs | 4 -- tests/cli.rs | 104 +++++++++++++++++++++++++------------------------ 3 files changed, 57 insertions(+), 67 deletions(-) diff --git a/src/command.rs b/src/command.rs index f245a2e..4c1863a 100644 --- a/src/command.rs +++ b/src/command.rs @@ -12,14 +12,9 @@ fn read_file(file: &String) -> Result { fn handle_commit_message(msg: &str) { match lint_commit_message(msg) { LintOutcome::Empty => { - eprintln!( - "{}: Aborting commit due to empty commit message\n", - VALIDATION_FAILED - ); std::process::exit(1); } - LintOutcome::Ignored => println!("commit message ignored, skipping lint\n"), - + LintOutcome::Ignored => (), LintOutcome::Valid => { println!("{}\n", VALIDATION_SUCCESSFUL); } @@ -36,17 +31,12 @@ fn handle_multiple_commit_messages(messages: &[String]) { for msg in messages { match lint_commit_message(msg) { LintOutcome::Empty => { - eprintln!( - "{}: Aborting commit due to empty commit message\n", - VALIDATION_FAILED - ); std::process::exit(1); } - LintOutcome::Ignored => println!("commit message ignored, skipping lint\n"), - LintOutcome::Valid => println!("lint success\n"), + LintOutcome::Ignored => (), + LintOutcome::Valid => (), LintOutcome::Invalid => { has_failure = true; - eprintln!("Lint failed \n") } } } diff --git a/src/linter.rs b/src/linter.rs index 5602e86..3d24471 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -25,10 +25,6 @@ pub enum LintOutcome { /// Evaluates a commit message, matches the conventional commit format /// and returns its linting outcome. pub fn lint_commit_message(message: &str) -> LintOutcome { - println!( - "========= Linting Message:: {} ===========", - message.lines().next().unwrap_or("") - ); if is_empty(message) { LintOutcome::Empty } else if is_ignored(message) { diff --git a/tests/cli.rs b/tests/cli.rs index 6faffcc..9dbf9cd 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -55,6 +55,17 @@ fn invalid_message_fails() { .stderr(predicate::str::contains("Commit validation: failed!")); } +#[test] +fn invalid_message_no_space_after_colon_fails() { + cocox().arg("feat:add feature").assert().failure(); +} + +#[test] +fn invalid_message_description_trailing_period_fails() { + // Parity with Python: descriptions should not end in a period + cocox().arg("feat: add feature.").assert().failure(); +} + #[test] fn unknown_type_fails() { cocox().arg("wip: something").assert().failure().code(1); @@ -62,26 +73,12 @@ fn unknown_type_fails() { #[test] fn empty_message_aborts() { - cocox() - .arg("") - .assert() - .failure() - .code(1) - .stderr(predicate::str::contains( - "Aborting commit due to empty commit message", - )); + cocox().arg("").assert().failure().code(1); } #[test] fn whitespace_only_message_aborts() { - cocox() - .arg(" \n\t ") - .assert() - .failure() - .code(1) - .stderr(predicate::str::contains( - "Aborting commit due to empty commit message", - )); + cocox().arg(" \n\t ").assert().failure().code(1); } // --- ignored messages ------------------------------------------------------ @@ -91,12 +88,7 @@ fn whitespace_only_message_aborts() { #[test] fn merge_commit_is_ignored() { - cocox() - .arg("Merge pull request #123") - .assert() - .success() - .stdout(predicate::str::contains("commit message ignored")) - .stderr(predicate::str::is_empty()); + cocox().arg("Merge pull request #123").assert().success(); } #[test] @@ -104,8 +96,7 @@ fn dependabot_bump_is_ignored() { cocox() .arg("Bump urllib3 from 1.26.5 to 1.26.17") .assert() - .success() - .stdout(predicate::str::contains("commit message ignored")); + .success(); } #[test] @@ -126,6 +117,14 @@ fn file_with_valid_message_succeeds() { .stdout(predicate::str::contains("Commit validation: successful!")); } +#[test] +fn file_with_git_comments_succeeds() { + // Python parity: test__main__valid_commit_message_and_comments_with_file + let content = "feat: add new feature\n\n# This is a git comment\n# It should be ignored"; + let file = write_temp(content); + cocox().arg("--file").arg(file.path()).assert().success(); +} + #[test] fn file_with_invalid_message_fails() { let file = write_temp("bad commit message"); @@ -146,10 +145,7 @@ fn file_with_empty_contents_aborts() { .arg(file.path()) .assert() .failure() - .code(1) - .stderr(predicate::str::contains( - "Aborting commit due to empty commit message", - )); + .code(1); } #[test] @@ -160,10 +156,7 @@ fn file_with_whitespace_only_content_aborts() { .arg(file.path()) .assert() .failure() - .code(1) - .stderr(predicate::str::contains( - "Aborting commit due to empty commit message", - )); + .code(1); } #[test] @@ -241,13 +234,7 @@ fn hash_ignored_commit_silently_succeeds() { let repo = TestRepo::new(); let hash = repo.commit("Merge pull request #123"); - cocox() - .arg("--hash") - .arg(&hash) - .assert() - .success() - .stdout(predicate::str::contains("commit message ignored")) - .stderr(predicate::str::is_empty()); + cocox().arg("--hash").arg(&hash).assert().success(); } // ---- hash range ---------------------------------------------------------- @@ -412,7 +399,9 @@ fn no_args_fails_with_clap_error() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("required")); + .stderr(predicate::str::contains( + "the following required arguments were not provided:", + )); } #[test] @@ -423,7 +412,9 @@ fn to_hash_only_fails() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("required")); + .stderr(predicate::str::contains( + "the following required arguments were not provided:", + )); } #[test] @@ -436,7 +427,9 @@ fn message_and_file_together_fail() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("cannot be used")); + .stderr(predicate::str::contains( + "the argument '[MESSAGE]' cannot be used with '--file '", + )); } #[test] @@ -450,7 +443,9 @@ fn file_and_hash_together_fail() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("cannot be used")); + .stderr(predicate::str::contains( + "error: the argument '--file ' cannot be used with '--hash '", + )); } #[test] @@ -464,7 +459,9 @@ fn file_and_from_hash_together_fail() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("cannot be used")); + .stderr(predicate::str::contains( + "the argument '--file ' cannot be used with '--from-hash '", + )); } #[test] @@ -476,7 +473,9 @@ fn message_and_hash_together_fail() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("cannot be used")); + .stderr(predicate::str::contains( + "the argument '[MESSAGE]' cannot be used with '--hash '", + )); } #[test] @@ -488,7 +487,7 @@ fn message_and_from_hash_together_fail() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("cannot be used")); + .stderr(predicate::str::contains("")); } #[test] @@ -501,9 +500,10 @@ fn hash_and_from_hash_together_fail() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("cannot be used")); + .stderr(predicate::str::contains( + "the argument '--hash ' cannot be used with '--from-hash '", + )); } - #[test] fn to_hash_with_hash_together_fail() { // --to-hash requires --from-hash specifically, not just any input arg @@ -515,7 +515,9 @@ fn to_hash_with_hash_together_fail() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("cannot be used")); + .stderr(predicate::str::contains( + "error: the argument '--hash ' cannot be used with '--to-hash '", + )); } #[test] @@ -528,7 +530,9 @@ fn to_hash_with_message_fails() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("cannot be used")); + .stderr(predicate::str::contains( + "the argument '[MESSAGE]' cannot be used with '--to-hash '", + )); } #[test] From b547a4d776326f21c9c185cd3d7d95c65252e1f5 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 9 Jun 2026 16:59:32 +0545 Subject: [PATCH 18/21] test: (#16) test all capitalization varients of 'initial commit' message --- src/utils.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/utils.rs b/src/utils.rs index c4c15e8..6f5d4aa 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -79,6 +79,8 @@ mod tests { fn ignores_initial_commit() { assert!(is_ignored("Initial commit")); assert!(is_ignored("initial Commit")); + assert!(is_ignored("Initial Commit")); + assert!(is_ignored("initial commit")); } #[test] From 880478ce8e24bf3feacdb89df5a6949a04450aaf Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 9 Jun 2026 17:13:09 +0545 Subject: [PATCH 19/21] refactor: remove --no-merges flag from git log though standard merge commits are to be ignored for linting, they must be extracted when using hash in order to validate custom commit message as invalid. --- src/git_helpers.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/git_helpers.rs b/src/git_helpers.rs index a8a1258..6efaa97 100644 --- a/src/git_helpers.rs +++ b/src/git_helpers.rs @@ -27,7 +27,6 @@ pub fn get_commit_messages_from_hash_range(from_hash: &str, to_hash: &str) -> Re let output = Command::new("git") .args([ "log", - "--no-merges", "--pretty=format:%B%x00", // null byte for delimiter "--reverse", &range, From 42d20a4de5a77b16c556233b2f9ea7490baf3ce0 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Tue, 9 Jun 2026 17:19:47 +0545 Subject: [PATCH 20/21] style: replace string literals with message constants --- tests/cli.rs | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/tests/cli.rs b/tests/cli.rs index 9dbf9cd..f19cf2b 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -6,6 +6,7 @@ use predicates::prelude::*; use serial_test::serial; use std::io::Write; use tempfile::NamedTempFile; +use cocox::messages::{VALIDATION_FAILED, VALIDATION_SUCCESSFUL}; fn cocox() -> Command { Command::cargo_bin("cocox").expect("cocox binary should be built") @@ -26,7 +27,7 @@ fn valid_message_succeeds() { .arg("feat: add new feature") .assert() .success() - .stdout(predicate::str::contains("Commit validation: successful!")); + .stdout(predicate::str::contains(VALIDATION_SUCCESSFUL)); } #[test] @@ -52,7 +53,7 @@ fn invalid_message_fails() { .assert() .failure() .code(1) - .stderr(predicate::str::contains("Commit validation: failed!")); + .stderr(predicate::str::contains(VALIDATION_FAILED)); } #[test] @@ -114,7 +115,7 @@ fn file_with_valid_message_succeeds() { .arg(file.path()) .assert() .success() - .stdout(predicate::str::contains("Commit validation: successful!")); + .stdout(predicate::str::contains(VALIDATION_SUCCESSFUL)); } #[test] @@ -134,7 +135,7 @@ fn file_with_invalid_message_fails() { .assert() .failure() .code(1) - .stderr(predicate::str::contains("Commit validation: failed!")); + .stderr(predicate::str::contains(VALIDATION_FAILED)); } #[test] @@ -184,7 +185,7 @@ fn hash_head_valid_commit_succeeds() { .arg("HEAD") .assert() .success() - .stdout(predicate::str::contains("Commit validation: successful!")); + .stdout(predicate::str::contains(VALIDATION_SUCCESSFUL)); } #[test] @@ -210,7 +211,7 @@ fn hash_exact_valid_commit_succeeds() { .arg(&hash) .assert() .success() - .stdout(predicate::str::contains("Commit validation: successful!")); + .stdout(predicate::str::contains(VALIDATION_SUCCESSFUL)); } #[test] @@ -225,7 +226,7 @@ fn hash_invalid_commit_message_fails() { .assert() .failure() .code(1) - .stderr(predicate::str::contains("Commit validation: failed!")); + .stderr(predicate::str::contains(VALIDATION_FAILED)); } #[test] @@ -257,7 +258,7 @@ fn hash_range_valid_commits_succeeds() { .arg(c) .assert() .success() - .stdout(predicate::str::contains("Commit validation: successful!")); + .stdout(predicate::str::contains(VALIDATION_SUCCESSFUL)); } #[test] @@ -277,7 +278,7 @@ fn hash_range_invalid_commit_in_middle_fails() { .assert() .failure() .code(1) - .stderr(predicate::str::contains("Commit validation: failed!")); + .stderr(predicate::str::contains(VALIDATION_FAILED)); } #[test] @@ -296,7 +297,7 @@ fn hash_range_invalid_from_commit_fails() { .assert() .failure() .code(1) - .stderr(predicate::str::contains("Commit validation: failed!")); + .stderr(predicate::str::contains(VALIDATION_FAILED)); } #[test] @@ -315,7 +316,7 @@ fn hash_range_invalid_to_commit_fails() { .assert() .failure() .code(1) - .stderr(predicate::str::contains("Commit validation: failed!")); + .stderr(predicate::str::contains(VALIDATION_FAILED)); } #[test] @@ -334,7 +335,7 @@ fn hash_range_ignored_commits_are_skipped() { .arg(&c) .assert() .success() - .stdout(predicate::str::contains("Commit validation: successful!")); + .stdout(predicate::str::contains(VALIDATION_SUCCESSFUL)); } #[test] @@ -351,7 +352,7 @@ fn hash_range_single_commit_succeeds() { .arg(&a) .assert() .success() - .stdout(predicate::str::contains("Commit validation: successful!")); + .stdout(predicate::str::contains(VALIDATION_SUCCESSFUL)); } #[test] @@ -369,7 +370,7 @@ fn hash_range_single_invalid_commit_fails() { .assert() .failure() .code(1) - .stderr(predicate::str::contains("Commit validation: failed!")); + .stderr(predicate::str::contains(VALIDATION_FAILED)); } #[test] @@ -388,7 +389,7 @@ fn from_hash_only_valid_commits_succeeds() { .arg(a) .assert() .success() - .stdout(predicate::str::contains("Commit validation: successful!")); + .stdout(predicate::str::contains(VALIDATION_SUCCESSFUL)); } // --- clap argument constraints -------------------------------------------- From 72c81042a1d82d73263a4135735348b30e52bc31 Mon Sep 17 00:00:00 2001 From: Bipin Thapa Date: Fri, 12 Jun 2026 21:05:40 +0545 Subject: [PATCH 21/21] docs: improve docstring style --- tests/common/git_repo.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/common/git_repo.rs b/tests/common/git_repo.rs index 022fe69..acb6a5f 100644 --- a/tests/common/git_repo.rs +++ b/tests/common/git_repo.rs @@ -13,8 +13,10 @@ impl TestRepo { /// Initialize a temporary test repo with git config. /// /// ## Important: Process-Global State - /// This function changes the current working directory of the **entire process** /// to the temporary repository. Because Cargo runs tests in parallel threads within - /// the same process, **any test using `TestRepo` must be marked with `#[serial]`** /// from the `serial_test` crate to prevent race conditions and cross-test interference. + /// This function changes the current working directory of the **entire process + /// to the temporary repository. Because Cargo runs tests in parallel threads within + /// the same process, **any test using `TestRepo` must be marked with `#[serial]` + /// from the `serial_test` crate to prevent race conditions and cross-test interference. /// /// The working directory will automatically reset back to the original project /// directory when this `TestRepo` goes out of scope (via `Drop`).