Add some tests (#10)

Signed-off-by: Sergio Castaño Arteaga <[email protected]>

Cargo.toml

@@ -27,6 +27,7 @@ figment = { version = "0.10.19", features = ["yaml", "env"] }
 hmac = "0.12.1"
 hex = "0.4.3"
 http = "1.1.0"
+indoc = "2.0.5"
 lambda_http = "0.13.0"
 octorust = "0.8.0-rc.1"
 pem = "3.0.4"

dco2/Cargo.toml

@@ -21,3 +21,6 @@ serde = { workspace = true }
 serde_json = { workspace = true }
 thiserror = { workspace = true }
 tracing = { workspace = true }
+
+[dev-dependencies]
+indoc = { workspace = true }

dco2/src/dco.rs → dco2/src/dco/mod.rs

@@ -14,6 +14,9 @@ use std::sync::LazyLock;
 use thiserror::Error;
 use tracing::debug;
 
+#[cfg(test)]
+mod tests;
+
 /// Sign-off line regular expression.
 static SIGN_OFF: LazyLock<Regex> = LazyLock::new(|| {
     Regex::new(r"(?mi)^Signed-off-by: (.*) <(.*)>\s*$").expect("expr in SIGN_OFF to be valid")
@@ -29,7 +32,6 @@ pub struct CheckInput {
 #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, Template)]
 #[template(path = "output.md")]
 pub struct CheckOutput {
-    pub check_passed: bool,
     pub commits_with_errors: Vec<CommitCheckOutput>,
 }
 
@@ -101,11 +103,16 @@ pub async fn process_event(gh_client: DynGHClient, event: &Event) -> Result<()>
 
     // Run DCO check
     let input = dco::CheckInput { commits };
-    let output = dco::check(&input).context("error running dco check")?;
+    let output = dco::check(&input);
 
     // Create check run
     let check_run = CheckRun {
-        conclusion: (if output.check_passed { "success" } else { "failure" }).to_string(),
+        conclusion: (if output.commits_with_errors.is_empty() {
+            "success"
+        } else {
+            "failure"
+        })
+        .to_string(),
         head_sha: event.pull_request.head.sha.clone(),
         name: "DCO".to_string(),
         started_at,
@@ -118,7 +125,7 @@ pub async fn process_event(gh_client: DynGHClient, event: &Event) -> Result<()>
 }
 
 /// Run DCO check.
-pub fn check(input: &CheckInput) -> Result<CheckOutput> {
+pub fn check(input: &CheckInput) -> CheckOutput {
     let mut output = CheckOutput::default();
 
     // Check each commit
@@ -155,10 +162,7 @@ pub fn check(input: &CheckInput) -> Result<CheckOutput> {
         }
     }
 
-    // The check passes if there are no commits with errors
-    output.check_passed = output.commits_with_errors.is_empty();
-
-    Ok(output)
+    output
 }
 
 /// Check if we should skip this commit.
@@ -222,12 +226,23 @@ fn get_signoffs(commit: &Commit) -> Vec<SignOff> {
 
 /// Check if any of the sign-offs matches the author's or committer's email.
 fn signoffs_match(signoffs: &[SignOff], commit: &Commit) -> bool {
-    let author_email = commit.author.as_ref().map(|a| &a.email);
-    let committer_email = commit.committer.as_ref().map(|c| &c.email);
+    let signoff_matches_author = |s: &SignOff| {
+        if let Some(a) = &commit.author {
+            s.name.to_lowercase() == a.name.to_lowercase() && s.email.to_lowercase() == a.email.to_lowercase()
+        } else {
+            false
+        }
+    };
 
-    signoffs
-        .iter()
-        .any(|s| Some(&s.email) == author_email || Some(&s.email) == committer_email)
+    let signoff_matches_committer = |s: &SignOff| {
+        if let Some(c) = &commit.committer {
+            s.name.to_lowercase() == c.name.to_lowercase() && s.email.to_lowercase() == c.email.to_lowercase()
+        } else {
+            false
+        }
+    };
+
+    signoffs.iter().any(|s| signoff_matches_author(s) || signoff_matches_committer(s))
 }
 
 /// Display some information about a processed commit.

dco2/src/dco/tests.rs

@@ -0,0 +1,208 @@
+use super::{check, CheckInput};
+use crate::github::{Commit, GitUser};
+use indoc::indoc;
+
+#[test]
+fn check_no_signoff_single_commit_is_merge_commit() {
+    let commit1 = Commit {
+        is_merge: true,
+        ..Default::default()
+    };
+
+    let input = CheckInput {
+        commits: vec![commit1],
+    };
+    let output = check(&input);
+
+    assert!(output.commits_with_errors.is_empty());
+}
+
+#[test]
+fn check_no_signoff_single_commit_author_is_bot() {
+    let commit1 = Commit {
+        author: Some(GitUser {
+            is_bot: true,
+            ..Default::default()
+        }),
+        ..Default::default()
+    };
+
+    let input = CheckInput {
+        commits: vec![commit1],
+    };
+    let output = check(&input);
+
+    assert!(output.commits_with_errors.is_empty());
+}
+
+#[test]
+fn check_valid_signoff_single_commit_author_match() {
+    let commit1 = Commit {
+        author: Some(GitUser {
+            name: "user1".to_string(),
+            email: "[email protected]".to_string(),
+            ..Default::default()
+        }),
+        message: indoc! {r"
+            Test commit message
+
+            Signed-off-by: user1 <[email protected]>
+        "}
+        .to_string(),
+        ..Default::default()
+    };
+
+    let input = CheckInput {
+        commits: vec![commit1],
+    };
+    let output = check(&input);
+
+    assert!(output.commits_with_errors.is_empty());
+}
+
+#[test]
+fn check_valid_signoff_single_commit_committer_match() {
+    let commit1 = Commit {
+        committer: Some(GitUser {
+            name: "user1".to_string(),
+            email: "[email protected]".to_string(),
+            ..Default::default()
+        }),
+        message: indoc! {r"
+            Test commit message
+
+            Signed-off-by: user1 <[email protected]>
+        "}
+        .to_string(),
+        ..Default::default()
+    };
+
+    let input = CheckInput {
+        commits: vec![commit1],
+    };
+    let output = check(&input);
+
+    assert!(output.commits_with_errors.is_empty());
+}
+
+#[test]
+fn check_valid_signoff_single_commit_multiple_signoffs() {
+    let commit1 = Commit {
+        author: Some(GitUser {
+            name: "user1".to_string(),
+            email: "[email protected]".to_string(),
+            ..Default::default()
+        }),
+        message: indoc! {r"
+            Test commit message
+
+            Signed-off-by: userx <[email protected]>
+            Signed-off-by: user1 <[email protected]>
+            Signed-off-by: usery <[email protected]>
+        "}
+        .to_string(),
+        ..Default::default()
+    };
+
+    let input = CheckInput {
+        commits: vec![commit1],
+    };
+    let output = check(&input);
+
+    assert!(output.commits_with_errors.is_empty());
+}
+
+#[test]
+fn check_valid_signoff_single_commit_signoff_case_insensitive() {
+    let commit1 = Commit {
+        author: Some(GitUser {
+            name: "user1".to_string(),
+            email: "[email protected]".to_string(),
+            ..Default::default()
+        }),
+        message: indoc! {r"
+            Test commit message
+
+            signed-off-by: USER1 <[email protected]>
+        "}
+        .to_string(),
+        ..Default::default()
+    };
+
+    let input = CheckInput {
+        commits: vec![commit1],
+    };
+    let output = check(&input);
+
+    assert!(output.commits_with_errors.is_empty());
+}
+
+#[test]
+fn check_valid_signoff_single_commit_signoff_trailing_whitespace() {
+    let commit1 = Commit {
+        author: Some(GitUser {
+            name: "user1".to_string(),
+            email: "[email protected]".to_string(),
+            ..Default::default()
+        }),
+        message: "Test\n\nSigned-off-by: user1 <[email protected]>   ".to_string(),
+        ..Default::default()
+    };
+
+    let input = CheckInput {
+        commits: vec![commit1],
+    };
+    let output = check(&input);
+
+    assert!(output.commits_with_errors.is_empty());
+}
+
+#[test]
+fn check_valid_signoff_single_commit_email_contains_subdomain() {
+    let commit1 = Commit {
+        author: Some(GitUser {
+            name: "user1".to_string(),
+            email: "[email protected]".to_string(),
+            ..Default::default()
+        }),
+        message: indoc! {r"
+            Test commit message
+
+            Signed-off-by: user1 <[email protected]>
+        "}
+        .to_string(),
+        ..Default::default()
+    };
+
+    let input = CheckInput {
+        commits: vec![commit1],
+    };
+    let output = check(&input);
+
+    assert!(output.commits_with_errors.is_empty());
+}
+
+#[test]
+fn check_valid_signoff_single_commit_email_contains_plus_alias() {
+    let commit1 = Commit {
+        author: Some(GitUser {
+            name: "user1".to_string(),
+            email: "[email protected]".to_string(),
+            ..Default::default()
+        }),
+        message: indoc! {r"
+            Test commit message
+
+            Signed-off-by: user1 <[email protected]>
+        "}
+        .to_string(),
+        ..Default::default()
+    };
+
+    let input = CheckInput {
+        commits: vec![commit1],
+    };
+    let output = check(&input);
+
+    assert!(output.commits_with_errors.is_empty());
+}

dco2/src/github.rs

@@ -146,7 +146,7 @@ pub struct CheckRun {
 }
 
 /// Commit information.
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
 pub struct Commit {
     pub author: Option<GitUser>,
     pub committer: Option<GitUser>,
@@ -157,7 +157,7 @@ pub struct Commit {
 }
 
 /// Git user information.
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
 pub struct GitUser {
     pub name: String,
     pub email: String,

dco2/templates/output.md

@@ -1,4 +1,4 @@
-{% if check_passed -%}
+{% if commits_with_errors.is_empty() -%}
 All commits are signed off!
 {% else -%}
 There is at least one commit incorrectly signed off. This means that the author of this commit failed to include a Signed-off-by line in the commit message.