Extract check-commits handler trigger logic and add tests for it

Author: Urgau

src/github.rs

@@ -287,6 +287,7 @@ pub struct Label {
 /// and some don't. GitHub does include a few fields here, but they aren't
 /// needed at this time (merged_at, diff_url, html_url, patch_url, url).
 #[derive(Debug, serde::Deserialize)]
+#[cfg_attr(test, derive(Default))]
 pub struct PullRequestDetails {
     /// This is a slot to hold the diff for a PR.
     ///

src/handlers/check_commits.rs

@@ -39,25 +39,42 @@ struct CheckCommitsState {
     last_labels: Vec<String>,
 }
 
-pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> anyhow::Result<()> {
-    let Event::Issue(event) = event else {
-        return Ok(());
-    };
+fn should_handle_event(event: &IssuesEvent) -> bool {
+    // Reject non-PR
+    if !event.issue.is_pr() {
+        return false;
+    }
+
+    // Reject rollups and draft pr
+    if event.issue.title.starts_with("Rollup of") || event.issue.draft {
+        return false;
+    }
 
-    let should_check = matches!(
+    // Check opened, reopened, synchronized and ready-for-review PRs
+    if matches!(
         event.action,
         IssuesAction::Opened
             | IssuesAction::Reopened
             | IssuesAction::Synchronize
             | IssuesAction::ReadyForReview
-    ) || event.has_base_changed();
+    ) {
+        return true;
+    }
 
-    if !should_check || !event.issue.is_pr() {
-        return Ok(());
+    // Also check PRs that changed their base branch (master -> beta)
+    if event.has_base_changed() {
+        return true;
     }
 
-    // Don't ping on rollups or draft PRs.
-    if event.issue.title.starts_with("Rollup of") || event.issue.draft {
+    false
+}
+
+pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> anyhow::Result<()> {
+    let Event::Issue(event) = event else {
+        return Ok(());
+    };
+
+    if !should_handle_event(event) {
         return Ok(());
     }
 
@@ -350,3 +367,139 @@ r#":warning: **Warning** :warning:
      - 123456789"#
     );
 }
+
+#[cfg(test)]
+fn make_opened_issues_event() -> IssuesEvent {
+    IssuesEvent {
+        action: IssuesAction::Opened,
+        issue: crate::github::Issue {
+            number: 123,
+            body: "My PR body".to_string(),
+            created_at: Default::default(),
+            updated_at: Default::default(),
+            merge_commit_sha: Default::default(),
+            title: "Some title".to_string(),
+            html_url: Default::default(),
+            user: crate::github::User {
+                login: "user".to_string(),
+                id: 654123,
+            },
+            labels: Default::default(),
+            assignees: Default::default(),
+            pull_request: Some(Default::default()),
+            merged: false,
+            draft: false,
+            comments: Default::default(),
+            comments_url: Default::default(),
+            repository: Default::default(),
+            base: Some(crate::github::CommitBase {
+                sha: "fake-sha".to_string(),
+                git_ref: "master".to_string(),
+                repo: None,
+            }),
+            head: Some(crate::github::CommitBase {
+                sha: "fake-sha".to_string(),
+                git_ref: "master".to_string(),
+                repo: None,
+            }),
+            state: crate::github::IssueState::Open,
+            milestone: None,
+            mergeable: None,
+            author_association: octocrab::models::AuthorAssociation::Contributor,
+        },
+        changes: None,
+        repository: crate::github::Repository {
+            full_name: "rust-lang/rust".to_string(),
+            default_branch: "master".to_string(),
+            fork: false,
+            parent: None,
+        },
+        sender: crate::github::User {
+            login: "rustbot".to_string(),
+            id: 987654,
+        },
+    }
+}
+
+#[test]
+fn test_pr_closed() {
+    let mut event = make_opened_issues_event();
+    event.action = IssuesAction::Closed;
+    assert!(!should_handle_event(&event));
+}
+
+#[test]
+fn test_pr_opened() {
+    let event = make_opened_issues_event();
+    assert!(should_handle_event(&event));
+}
+
+#[test]
+fn test_not_pr() {
+    let mut event = make_opened_issues_event();
+    event.issue.pull_request = None;
+    assert!(!should_handle_event(&event));
+}
+
+#[test]
+fn test_pr_rollup() {
+    let mut event = make_opened_issues_event();
+    event.issue.title = "Rollup of 6 pull requests".to_string();
+    assert!(!should_handle_event(&event));
+}
+
+#[test]
+fn test_pr_draft() {
+    let mut event = make_opened_issues_event();
+    event.issue.draft = true;
+    assert!(!should_handle_event(&event));
+}
+
+#[test]
+fn test_pr_ready() {
+    let mut event = make_opened_issues_event();
+    event.action = IssuesAction::ReadyForReview;
+    assert!(should_handle_event(&event));
+}
+
+#[test]
+fn test_pr_reopened() {
+    let mut event = make_opened_issues_event();
+    event.action = IssuesAction::Reopened;
+    assert!(should_handle_event(&event));
+}
+
+#[test]
+fn test_pr_synchronized() {
+    let mut event = make_opened_issues_event();
+    event.action = IssuesAction::Synchronize;
+    assert!(should_handle_event(&event));
+}
+
+#[test]
+fn test_pr_edited_title() {
+    let mut event = make_opened_issues_event();
+    event.action = IssuesAction::Edited;
+    event.changes = Some(crate::github::Changes {
+        base: None,
+        body: None,
+        title: Some(crate::github::ChangeInner {
+            from: "Previous title".to_string(),
+        }),
+    });
+    assert!(!should_handle_event(&event));
+}
+
+#[test]
+fn test_pr_edited_base() {
+    let mut event = make_opened_issues_event();
+    event.action = IssuesAction::Edited;
+    event.changes = Some(crate::github::Changes {
+        title: None,
+        body: None,
+        base: Some(crate::github::ChangeInner {
+            from: "master".to_string(),
+        }),
+    });
+    assert!(should_handle_event(&event));
+}