Skip to content

Commit

Permalink
Include all commits in check output (#25)
Browse files Browse the repository at this point in the history 
Signed-off-by: Sergio Castaño Arteaga <[email protected]>
  • Loading branch information
@tegioz
tegioz authored Sep 4, 2024
1 parent 220dbde commit e3eb201
Show file tree
Hide file tree
Showing 5 changed files with 321 additions and 281 deletions.
145 changes: 17 additions & 128 deletions dco2/src/dco/check/filters.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,188 +4,77 @@ use super::{CommitCheckOutput, CommitError};

/// Template filter to check if any of the commits contain any of the
/// provided errors.
pub(crate) fn contains_error(
commits_with_errors: &[CommitCheckOutput],
errors: &[CommitError],
) -> askama::Result<bool> {
Ok(commits_with_errors.iter().any(|c| c.errors.iter().any(|e| errors.contains(e))))
}

/// Template filter to check if all the commits contain the same errors.
pub(crate) fn have_same_errors(commits_with_errors: &[CommitCheckOutput]) -> askama::Result<bool> {
for i in 1..commits_with_errors.len() {
if commits_with_errors[i].errors != commits_with_errors[0].errors {
return Ok(false);
}
}
Ok(true)
pub(crate) fn contains_error(commits: &[CommitCheckOutput], errors: &[CommitError]) -> askama::Result<bool> {
Ok(commits.iter().any(|c| c.errors.iter().any(|e| errors.contains(e))))
}

#[cfg(test)]
mod tests {
use crate::dco::check::{
filters::{contains_error, have_same_errors},
CommitCheckOutput, CommitError,
};
use crate::dco::check::{filters::contains_error, CommitCheckOutput, CommitError};

#[test]
fn contains_error_one_commit_error_found() {
let commits_with_errors = vec![CommitCheckOutput {
let commits = vec![CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail],
success_reason: None,
}];

assert!(contains_error(&commits_with_errors, &[CommitError::InvalidAuthorEmail]).unwrap());
assert!(contains_error(&commits, &[CommitError::InvalidAuthorEmail]).unwrap());
}

#[test]
fn contains_error_one_commit_error_not_found() {
let commits_with_errors = vec![CommitCheckOutput {
let commits = vec![CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail],
success_reason: None,
}];

assert!(!contains_error(&commits_with_errors, &[CommitError::InvalidCommitterEmail]).unwrap());
assert!(!contains_error(&commits, &[CommitError::InvalidCommitterEmail]).unwrap());
}

#[test]
fn contains_error_two_commits_error_found() {
let commits_with_errors = vec![
let commits = vec![
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail],
success_reason: None,
},
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidCommitterEmail],
success_reason: None,
},
];

assert!(contains_error(
&commits_with_errors,
&commits,
&[CommitError::InvalidAuthorEmail, CommitError::SignOffNotFound]
)
.unwrap());
}

#[test]
fn contains_error_two_commits_error_not_found() {
let commits_with_errors = vec![
let commits = vec![
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail],
success_reason: None,
},
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidCommitterEmail],
success_reason: None,
},
];

assert!(!contains_error(
&commits_with_errors,
&commits,
&[CommitError::SignOffNotFound, CommitError::SignOffMismatch]
)
.unwrap());
}

#[test]
fn have_same_errors_one_commit_always_true() {
let commits_with_errors = vec![CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail],
}];

assert!(have_same_errors(&commits_with_errors).unwrap());
}

#[test]
fn have_same_errors_two_commits_one_error_matches() {
let commits_with_errors = vec![
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail],
},
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail],
},
];

assert!(have_same_errors(&commits_with_errors).unwrap());
}

#[test]
fn have_same_errors_two_commits_two_errors_match() {
let commits_with_errors = vec![
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail, CommitError::SignOffNotFound],
},
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail, CommitError::SignOffNotFound],
},
];

assert!(have_same_errors(&commits_with_errors).unwrap());
}

#[test]
fn have_same_errors_two_commits_two_errors_do_not_match() {
let commits_with_errors = vec![
CommitCheckOutput {
commit: Default::default(),
errors: vec![
CommitError::InvalidAuthorEmail,
CommitError::InvalidCommitterEmail,
],
},
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail, CommitError::SignOffNotFound],
},
];

assert!(!have_same_errors(&commits_with_errors).unwrap());
}

#[test]
fn have_same_errors_three_commits_two_errors_match() {
let commits_with_errors = vec![
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail, CommitError::SignOffNotFound],
},
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail, CommitError::SignOffNotFound],
},
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail, CommitError::SignOffNotFound],
},
];

assert!(have_same_errors(&commits_with_errors).unwrap());
}

#[test]
fn have_same_errors_three_commits_one_error_does_not_match() {
let commits_with_errors = vec![
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail],
},
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::SignOffNotFound],
},
CommitCheckOutput {
commit: Default::default(),
errors: vec![CommitError::InvalidAuthorEmail],
},
];

assert!(!have_same_errors(&commits_with_errors).unwrap());
}
}
51 changes: 26 additions & 25 deletions dco2/src/dco/check/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use regex::Regex;
use serde::{Deserialize, Serialize};
use std::sync::LazyLock;
use thiserror::Error;
use tracing::debug;

mod filters;
#[cfg(test)]
Expand All @@ -25,16 +24,17 @@ pub(crate) struct CheckInput {
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Template)]
#[template(path = "output.md")]
pub(crate) struct CheckOutput {
pub commits_with_errors: Vec<CommitCheckOutput>,
pub commits: Vec<CommitCheckOutput>,
pub head_ref: String,
pub total_commits: usize,
pub num_commits_with_errors: usize,
}

/// Commit check output.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub(crate) struct CommitCheckOutput {
pub commit: Commit,
pub errors: Vec<CommitError>,
pub success_reason: Option<CommitSuccessReason>,
}

impl CommitCheckOutput {
Expand All @@ -43,6 +43,7 @@ impl CommitCheckOutput {
Self {
commit,
errors: Vec::new(),
success_reason: None,
}
}
}
Expand All @@ -60,12 +61,20 @@ pub(crate) enum CommitError {
SignOffNotFound,
}

/// Reasons why a commit's check succeeded.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub(crate) enum CommitSuccessReason {
FromBot,
IsMerge,
ValidSignOff,
}

/// Run DCO check.
pub(crate) fn check(input: &CheckInput) -> CheckOutput {
let mut output = CheckOutput {
commits_with_errors: Vec::new(),
commits: Vec::new(),
head_ref: input.head_ref.clone(),
total_commits: input.commits.len(),
num_commits_with_errors: 0,
};

// Check each commit
Expand All @@ -75,7 +84,8 @@ pub(crate) fn check(input: &CheckInput) -> CheckOutput {
// Check if we should skip this commit
let (commit_should_be_skipped, reason) = should_skip_commit(commit);
if commit_should_be_skipped {
debug!("commit ({}) skipped: {:?}", commit_output.commit.sha, reason);
commit_output.success_reason = reason;
output.commits.push(commit_output);
continue;
}

Expand All @@ -99,27 +109,30 @@ pub(crate) fn check(input: &CheckInput) -> CheckOutput {
commit_output.errors.push(CommitError::SignOffMismatch);
}

// Track commit if it has errors
debug_processed_commit(&commit_output, &signoffs);
if !commit_output.errors.is_empty() {
output.commits_with_errors.push(commit_output);
// Track commit
if commit_output.errors.is_empty() {
commit_output.success_reason = Some(CommitSuccessReason::ValidSignOff);
}
output.commits.push(commit_output);
}

// Update output status
output.num_commits_with_errors = output.commits.iter().filter(|c| !c.errors.is_empty()).count();

output
}

/// Check if we should skip this commit.
fn should_skip_commit(commit: &Commit) -> (bool, Option<String>) {
fn should_skip_commit(commit: &Commit) -> (bool, Option<CommitSuccessReason>) {
// Skip merge commits
if commit.is_merge {
return (true, Some("merge commit".to_string()));
return (true, Some(CommitSuccessReason::IsMerge));
}

// Skip bots commits
if let Some(author) = &commit.author {
if author.is_bot {
return (true, Some("author is a bot".to_string()));
return (true, Some(CommitSuccessReason::FromBot));
}
}

Expand Down Expand Up @@ -209,15 +222,3 @@ fn signoffs_match(signoffs: &[SignOff], commit: &Commit) -> bool {

signoffs.iter().any(|s| signoff_matches_author(s) || signoff_matches_committer(s))
}

/// Display some information about a processed commit.
fn debug_processed_commit(commit_output: &CommitCheckOutput, signoffs: &[SignOff]) {
debug!("commit processed: {}", commit_output.commit.sha);
debug!("errors found: {:?}", commit_output.errors);
debug!("author: {:?}", commit_output.commit.author);
debug!("committer: {:?}", commit_output.commit.committer);
debug!("sign-offs:");
for signoff in signoffs {
debug!("sign-off: {:?}", signoff);
}
}
Loading

0 comments on commit e3eb201

Please sign in to comment.