Skip to content

Commit

Permalink
feat(verify): add the ability to forbid merge commit via configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
oknozor committed Mar 8, 2022
1 parent 2c10235 commit d932d91
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 27 deletions.
26 changes: 20 additions & 6 deletions src/bin/cog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ enum Cli {
/// Check commit history, starting from the latest tag to HEAD
#[clap(short = 'l', long)]
from_latest_tag: bool,
/// Ignore merge commits messages
#[clap(short, long)]
ignore_merge_commits: bool,
},

/// Create a new conventional commit
Expand Down Expand Up @@ -77,6 +80,9 @@ enum Cli {
Verify {
/// The commit message
message: String,
/// Ignore merge commits messages
#[clap(short, long)]
ignore_merge_commits: bool,
},

/// Display a changelog for the given commit oid range
Expand All @@ -96,15 +102,15 @@ enum Cli {
template: Option<String>,

/// Url to use during template generation
#[clap(name = "remote", long, short, requires_all(& ["owner", "repository"]))]
#[clap(name = "remote", long, short, requires_all(&["owner", "repository"]))]
remote: Option<String>,

/// Repository owner to use during template generation
#[clap(name = "owner", long, short, requires_all(& ["remote", "repository"]))]
owner: Option<String>,

/// Name of the repository used during template generation
#[clap(name = "repository", long, requires_all(&["owner", "remote"]))]
#[clap(name = "repository", long, requires_all(& ["owner", "remote"]))]
repository: Option<String>,
},

Expand Down Expand Up @@ -209,16 +215,24 @@ fn main() -> Result<()> {

cocogitto.create_version(increment, pre.as_deref(), hook_profile.as_deref())?
}
Cli::Verify { message } => {
Cli::Verify {
message,
ignore_merge_commits,
} => {
let ignore_merge_commits = ignore_merge_commits || SETTINGS.ignore_merge_commits;
let author = CocoGitto::get()
.map(|cogito| cogito.get_committer().unwrap())
.ok();

commit::verify(author, &message)?;
commit::verify(author, &message, ignore_merge_commits)?;
}
Cli::Check { from_latest_tag } => {
Cli::Check {
from_latest_tag,
ignore_merge_commits,
} => {
let cocogitto = CocoGitto::get()?;
cocogitto.check(from_latest_tag)?;
let ignore_merge_commits = ignore_merge_commits || SETTINGS.ignore_merge_commits;
cocogitto.check(from_latest_tag, ignore_merge_commits)?;
}
Cli::Edit { from_latest_tag } => {
let cocogitto = CocoGitto::get()?;
Expand Down
19 changes: 14 additions & 5 deletions src/conventional/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,23 @@ impl Ord for Commit {
}
}

pub fn verify(author: Option<String>, message: &str) -> Result<(), ConventionalCommitError> {
pub fn verify(
author: Option<String>,
message: &str,
ignore_merge_commit: bool,
) -> Result<(), ConventionalCommitError> {
// Strip away comments from git message before parsing
let msg: String = message
.lines()
.filter(|line| !line.trim_start().starts_with('#'))
.collect::<Vec<&str>>()
.join("\n");

if msg.starts_with("Merge ") && ignore_merge_commit {
println!("{}", "Merge commit was ignored".yellow());
return Ok(());
}

let commit = conventional_commit_parser::parse(msg.as_str());

match commit {
Expand Down Expand Up @@ -306,7 +315,7 @@ mod test {
let message = "feat(database): add postgresql driver";

// Act
let result = verify(Some("toml".into()), message);
let result = verify(Some("toml".into()), message, false);

// Assert
assert_that!(result).is_ok();
Expand All @@ -324,7 +333,7 @@ mod test {
);

// Act
let result = verify(Some("toml".into()), message);
let result = verify(Some("toml".into()), message, false);

// Assert
assert_that!(result).is_ok();
Expand All @@ -336,7 +345,7 @@ mod test {
let message = "feat add postgresql driver";

// Act
let result = verify(Some("toml".into()), message);
let result = verify(Some("toml".into()), message, false);

// Assert
assert_that!(result).is_err();
Expand All @@ -348,7 +357,7 @@ mod test {
let message = "post: add postgresql driver";

// Act
let result = verify(Some("toml".into()), message);
let result = verify(Some("toml".into()), message, false);

// Assert
assert_that!(result).is_err();
Expand Down
32 changes: 23 additions & 9 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,12 @@ impl CocoGitto {
.collect();

rebase.commit(None, &original_commit.committer(), Some(&new_message))?;
match verify(self.repository.get_author().ok(), &new_message) {
let ignore_merge_commit = SETTINGS.ignore_merge_commits;
match verify(
self.repository.get_author().ok(),
&new_message,
ignore_merge_commit,
) {
Ok(_) => println!(
"Changed commit message to:\"{}\"",
&new_message.trim_end()
Expand All @@ -250,21 +255,30 @@ impl CocoGitto {
Ok(())
}

pub fn check(&self, check_from_latest_tag: bool) -> Result<()> {
pub fn check(&self, check_from_latest_tag: bool, ignore_merge_commits: bool) -> Result<()> {
let commit_range = if check_from_latest_tag {
self.repository
.get_commit_range(&RevspecPattern::default())?
} else {
self.repository.all_commits()?
};

let errors: Vec<_> = commit_range
.commits
.iter()
.filter(|commit| !commit.message().unwrap_or("").starts_with("Merge "))
.map(Commit::from_git_commit)
.filter_map(Result::err)
.collect();
let errors: Vec<_> = if ignore_merge_commits {
commit_range
.commits
.iter()
.filter(|commit| !commit.message().unwrap_or("").starts_with("Merge "))
.map(Commit::from_git_commit)
.filter_map(Result::err)
.collect()
} else {
commit_range
.commits
.iter()
.map(Commit::from_git_commit)
.filter_map(Result::err)
.collect()
};

if errors.is_empty() {
let msg = "No errored commits".green();
Expand Down
2 changes: 2 additions & 0 deletions src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ pub enum HookType {
#[derive(Debug, Deserialize, Serialize, Eq, PartialEq, Default)]
#[serde(deny_unknown_fields)]
pub struct Settings {
#[serde(default)]
pub ignore_merge_commits: bool,
#[serde(default)]
pub branch_whitelist: Vec<String>,
pub tag_prefix: Option<String>,
Expand Down
59 changes: 59 additions & 0 deletions tests/cog_tests/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::helpers::*;

use anyhow::Result;
use assert_cmd::prelude::*;
use cmd_lib::run_cmd;
use indoc::indoc;
use sealed_test::prelude::*;

Expand Down Expand Up @@ -89,3 +90,61 @@ fn verify_with_unknown_commit_type_fails() -> Result<()> {

Ok(())
}

#[test]
fn should_not_ignore_merge_commit_by_default() -> Result<()> {
// Arrange
let message = "Merge toto into titi";

// Act
Command::cargo_bin("cog")?
.arg("verify")
.arg(message)
// Assert
.assert()
.failure();

Ok(())
}

#[test]
fn should_ignore_merge_commit_with_ignore_flag() -> Result<()> {
// Arrange
let message = "Merge toto into titi";

// Act
Command::cargo_bin("cog")?
.arg("verify")
.arg("--ignore-merge-commits")
.arg(message)
// Assert
.assert()
.success();

Ok(())
}

#[sealed_test]
fn should_ignore_merge_commit_via_config() -> Result<()> {
// Arrange
git_init()?;
let settings = r#"ignore_merge_commits = true"#;

run_cmd!(
echo $settings > cog.toml;
git add .;
git commit -m "feat: cog.toml config"
)?;

let message = "Merge toto into titi";

// Act
Command::cargo_bin("cog")?
.arg("verify")
.arg(message)
// Assert
.assert()
.success();

Ok(())
}
45 changes: 38 additions & 7 deletions tests/lib_tests/cocogitto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,45 @@ fn open_repo_err() -> Result<()> {
#[sealed_test]
fn check_commit_history_ok() -> Result<()> {
// Arrange
git_init_and_set_current_path("commit_history_ok")?;
create_empty_config()?;
git_init()?;
git_commit("feat: a valid commit")?;
git_commit("chore(test): another valid commit")?;
let cocogitto = CocoGitto::get()?;

// Act
let check = cocogitto.check(false);
let check = cocogitto.check(false, false);

// Assert
assert_that!(check).is_ok();
Ok(())
}

#[sealed_test]
fn check_commit_history_err_with_merge_commit() -> Result<()> {
// Arrange
git_init()?;
git_commit("feat: a valid commit")?;
git_commit("Merge feature one into main")?;
let cocogitto = CocoGitto::get()?;

// Act
let check = cocogitto.check(false, false);

// Assert
assert_that!(check).is_err();
Ok(())
}

#[sealed_test]
fn check_commit_history_ok_with_merge_commit_ignored() -> Result<()> {
// Arrange
git_init()?;
git_commit("feat: a valid commit")?;
git_commit("Merge feature one into main")?;
let cocogitto = CocoGitto::get()?;

// Act
let check = cocogitto.check(false, true);

// Assert
assert_that!(check).is_ok();
Expand All @@ -59,7 +90,7 @@ fn check_commit_history_err() -> Result<()> {
let cocogitto = CocoGitto::get()?;

// Act
let check = cocogitto.check(false);
let check = cocogitto.check(false, false);

// Assert
assert_that!(check).is_err();
Expand All @@ -78,7 +109,7 @@ fn check_commit_ok_from_latest_tag() -> Result<()> {
let cocogitto = CocoGitto::get()?;

// Act
let check = cocogitto.check(true);
let check = cocogitto.check(true, false);

// Assert
assert_that!(check).is_ok();
Expand All @@ -96,7 +127,7 @@ fn check_commit_err_from_latest_tag() -> Result<()> {
let cocogitto = CocoGitto::get()?;

// Act
let check = cocogitto.check(true);
let check = cocogitto.check(true, false);

// Assert
assert_that!(check).is_err();
Expand All @@ -113,7 +144,7 @@ fn long_commit_summary_does_not_panic() -> Result<()> {
git_add("Hello", "file")?;
cocogitto.conventional_commit("feat", None, message, None, None, false)?;

let check = cocogitto.check(false);
let check = cocogitto.check(false, false);

assert_that!(check.is_ok());
Ok(())
Expand Down

0 comments on commit d932d91

Please sign in to comment.