test: add more tests to forge-github crate

Cargo.toml

@@ -90,7 +90,7 @@ inotify = "0.10"
 # Actors
 actix = "0.13"
 actix-rt = "2.9"
-tokio = { version = "1.37" }
+tokio = { version = "1.37", features = ["rt", "macros"] }
 
 # Testing
 assert2 = "0.3"

crates/config/src/server.rs

@@ -1,5 +1,6 @@
 //
 use actix::prelude::*;
+use derive_more::Constructor;
 
 use std::{
     collections::HashMap,
@@ -93,7 +94,16 @@ impl Webhook {
 }
 
 /// The URL for the webhook where forges should send their updates
-#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize, derive_more::AsRef)]
+#[derive(
+    Clone,
+    Debug,
+    PartialEq,
+    Eq,
+    serde::Serialize,
+    serde::Deserialize,
+    derive_more::AsRef,
+    Constructor,
+)]
 pub struct WebhookUrl(String);
 
 /// The directory to store server data, such as cloned repos

crates/forge-forgejo/src/tests.rs

@@ -1,3 +1,4 @@
+//
 use git_next_config as config;
 use git_next_git as git;
 

crates/forge-forgejo/src/webhook/register.rs

@@ -59,6 +59,8 @@ pub async fn register(
     match result {
         Ok(response) => {
             let Some(hook) = response.response_body() else {
+                #[cfg(not(tarpaulin_include))]
+                // request response is Json so response_body never returns None
                 return Err(git::forge::webhook::Error::NetworkResponseEmpty);
             };
             info!(webhook_id = %hook.id, "Webhook registered");

crates/forge-forgejo/src/webhook/unregister.rs

@@ -26,5 +26,9 @@ pub async fn unregister(
         network::NetRequestLogging::None,
     );
     let result = net.delete(request).await;
-    Ok(result.map(|_| ())?)
+    if let Err(e) = result {
+        tracing::warn!("Failed to unregister webhook");
+        return Err(git::forge::webhook::Error::FailedToRegister(e.to_string()));
+    }
+    Ok(())
 }

crates/forge-github/Cargo.toml

@@ -7,7 +7,7 @@ edition = { workspace = true }
 git-next-config = { workspace = true }
 git-next-git = { workspace = true }
 
-# own version
+# own version for UserAgent requests to github.com
 clap = { workspace = true }
 
 # logging
@@ -45,6 +45,10 @@ derive_more = { workspace = true }
 # # Actors
 tokio = { workspace = true }
 
+[dev-dependencies]
+# Testing
+assert2 = { workspace = true }
+rand = { workspace = true }
 
 [lints.clippy]
 nursery = { level = "warn", priority = -1 }

crates/forge-github/src/commit.rs

@@ -2,7 +2,7 @@
 use crate::{self as github, GithubState};
 use git::forge::commit::Status;
 use git_next_git as git;
-use github::GitHubStatus;
+use github::GithubStatus;
 use kxio::network;
 
 /// Checks the results of any (e.g. CI) status checks for the commit.
@@ -10,12 +10,13 @@ use kxio::network;
 /// GitHub: https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#list-commit-statuses-for-a-reference
 pub async fn status(github: &github::Github, commit: &git::Commit) -> git::forge::commit::Status {
     let repo_details = &github.repo_details;
+    let hostname = repo_details.forge.hostname();
     let repo_path = &repo_details.repo_path;
     let api_token = &repo_details.forge.token();
     use secrecy::ExposeSecret;
     let token = api_token.expose_secret();
     let url = network::NetUrl::new(format!(
-        "https://api.github.com/repos/{repo_path}/commits/{commit}/statuses"
+        "https://api.{hostname}/repos/{repo_path}/commits/{commit}/statuses"
     ));
 
     let headers = network::NetRequestHeaders::new()
@@ -31,14 +32,11 @@ pub async fn status(github: &github::Github, commit: &git::Commit) -> git::forge
         None,
         network::NetRequestLogging::None,
     );
-    let result = github.net.get::<Vec<GitHubStatus>>(request).await;
+    let result = github.net.get::<Vec<GithubStatus>>(request).await;
     match result {
-        Ok(response) => response.response_body().map_or_else(
+        Ok(response) => response
-            || {
+            .response_body()
-                tracing::warn!("No status found for commit");
+            .and_then(|statuses| {
-                git::forge::commit::Status::Pending // assume issue is transient and allow retry
-            },
-            |statuses| {
                 statuses
                     .into_iter()
                     .map(|status| match status.state {
@@ -55,12 +53,11 @@ pub async fn status(github: &github::Github, commit: &git::Commit) -> git::forge
                         (_, Status::Pending) => Status::Pending,
                         (Status::Pending, _) => Status::Pending,
                     })
-                    .unwrap_or_else(|| {
+            })
-                        tracing::warn!("No status checks configured for 'next' branch",);
+            .unwrap_or_else(|| {
-                        Status::Pass
+                tracing::warn!("No status checks configured for 'next' branch",);
-                    })
+                Status::Pass
-            },
+            }),
-        ),
         Err(e) => {
             tracing::warn!(?e, "Failed to get commit status");
             Status::Pending // assume issue is transient and allow retry

crates/forge-github/src/lib.rs

@@ -44,9 +44,9 @@ impl git::ForgeLike for Github {
 
     async fn list_webhooks(
         &self,
-        _webhook_url: &config::server::WebhookUrl,
+        webhook_url: &config::server::WebhookUrl,
     ) -> git::forge::webhook::Result<Vec<config::WebhookId>> {
-        github::webhook::list(self, _webhook_url).await
+        github::webhook::list(self, webhook_url).await
     }
 
     async fn unregister_webhook(
@@ -65,12 +65,12 @@ impl git::ForgeLike for Github {
     }
 }
 
-#[derive(Debug, serde::Deserialize)]
+#[derive(Debug, serde::Deserialize, serde::Serialize)]
-struct GitHubStatus {
+struct GithubStatus {
     pub state: GithubState,
     // other fields that we ignore
 }
-#[derive(Debug, serde::Deserialize)]
+#[derive(Clone, Debug, serde::Deserialize, serde::Serialize)]
 enum GithubState {
     #[serde(rename = "success")]
     Success,
@@ -86,8 +86,16 @@ enum GithubState {
 
 #[derive(Debug, serde::Deserialize)]
 struct GithubHook {
-    pub id: u64,
+    id: u64,
-    pub config: Config,
+    config: Config,
+}
+impl GithubHook {
+    pub fn id(&self) -> config::WebhookId {
+        config::WebhookId::new(format!("{}", self.id))
+    }
+    pub fn url(&self) -> config::server::WebhookUrl {
+        config::server::WebhookUrl::new(self.config.url.clone())
+    }
 }
 #[derive(Debug, serde::Deserialize)]
 struct Config {

crates/forge-github/src/tests/mod.rs

@@ -1,73 +1,703 @@
 //
-
-use git::ForgeLike;
 use git_next_config as config;
 use git_next_git as git;
 
-use std::collections::HashMap;
+mod github {
+    use super::*;
 
-type TestResult = Result<(), Box<dyn std::error::Error>>;
+    use assert2::let_assert;
+    use std::collections::BTreeMap;
 
-#[test]
+    use crate::Github;
-fn accepts_valid_webhook_signature() -> TestResult {
+    use config::{
-    //given
+        webhook::message::Body, ForgeAlias, ForgeConfig, ForgeType, GitDir, RepoAlias,
-    // we registered a webhook with this secret:
+        RepoBranches, ServerRepoConfig, WebhookAuth, WebhookMessage,
-    let webhook_auth = config::WebhookAuth::new("01HZ598CS1K9E0C193ND175XHJ")?;
+    };
-    // then recorded the following test message from github:
+    use git::ForgeLike as _;
-    let headers = HashMap::from([(
-        "x-hub-signature-256".to_string(),
-        "sha256=6c801b0730b1ce06bf38f901de40206d3b0e93ef7b9bf09a5cf28ad9c4221bab".to_string(),
-    )]);
-    let payload = config::webhook::message::Body::new(include_str!("payload.json").to_string());
-    // this reproduces that message:
-    let message = message(headers, payload);
 
-    //when
+    #[test]
-    // now, we attempt to recreate the signature in the header given the same message:
+    fn should_return_name() {
-    let result = forge().is_message_authorised(&message, &webhook_auth);
+        let forge = given::a_github_forge(&given::repo_details(), given::a_network());
+        assert_eq!(forge.name(), "github");
+    }
 
-    //then
+    mod is_message_authorised {
-    // if we succeed: then result will be true:
-    assert!(result);
 
-    Ok(())
+        use super::*;
-}
+
-
+        #[test]
-fn message(
+        fn should_return_true_with_valid_header() {
-    headers: HashMap<String, String>,
+            let forge = given::a_github_forge(&given::repo_details(), given::a_network());
-    payload: config::webhook::message::Body,
+            let auth = given::a_webhook_auth();
-) -> config::WebhookMessage {
+            let message = given::a_webhook_message(given::Header::Valid(
-    config::WebhookMessage::new(
+                auth.clone(),
-        config::ForgeAlias::new("".to_string()),
+                given::a_webhook_message_body(),
-        config::RepoAlias::new(""),
+            ));
-        headers,
+            assert!(forge.is_message_authorised(&message, &auth));
-        payload,
+        }
-    )
+        #[test]
-}
+        fn should_return_false_with_missing_header() {
-
+            let forge = given::a_github_forge(&given::repo_details(), given::a_network());
-fn forge() -> crate::Github {
+            let auth = given::a_webhook_auth();
-    crate::Github::new(
+            let message = given::a_webhook_message(given::Header::Missing);
-        git::RepoDetails::new(
+            assert!(!forge.is_message_authorised(&message, &auth));
-            git::Generation::new(),
+        }
-            &config::RepoAlias::new(""),
+        #[test]
-            &config::ServerRepoConfig::new(
+        fn should_return_false_with_invalid_header() {
-                "a".to_string(),
+            let forge = given::a_github_forge(&given::repo_details(), given::a_network());
-                "b".to_string(),
+            let auth = given::a_webhook_auth();
-                None,
+            let message = given::a_webhook_message(given::Header::Invalid);
-                None,
+            assert!(!forge.is_message_authorised(&message, &auth));
-                None,
+        }
-                None,
+    }
-            ),
+
-            &config::ForgeAlias::new("c".to_string()),
+    mod parse_webhook_body {
-            &config::ForgeConfig::new(
+
-                config::ForgeType::GitHub,
+        use serde_json::json;
-                "d".to_string(),
+
-                "e".to_string(),
+        use super::*;
-                "f".to_string(),
+
-                std::collections::BTreeMap::default(),
+        #[test]
-            ),
+        fn should_parse_valid_body() {
-            config::GitDir::default(),
+            let forge = given::a_github_forge(&given::repo_details(), given::a_network());
-        ),
+            let repo_branches = given::repo_branches();
-        kxio::network::Network::new_mock(),
+            let next = repo_branches.next();
-    )
+            let sha = given::a_name();
+            let message = given::a_name();
+            let body = Body::new(
+                json!({"ref":format!("refs/heads/{next}"),"after":sha,"head_commit":{"message":message}})
+                    .to_string(),
+            );
+            let_assert!(Ok(push) = forge.parse_webhook_body(&body));
+            assert_eq!(push.sha(), sha);
+            assert_eq!(push.message(), message);
+            assert_eq!(
+                push.branch(&repo_branches),
+                Some(config::webhook::push::Branch::Next)
+            );
+        }
+
+        #[test]
+        fn should_error_invalid_body() {
+            let forge = given::a_github_forge(&given::repo_details(), given::a_network());
+            let body = Body::new(r#"{"type":"invalid"}"#.to_string());
+            let_assert!(Err(_) = forge.parse_webhook_body(&body));
+        }
+    }
+
+    mod commit_status {
+        use git_next_git::forge::commit::Status;
+        use kxio::network::StatusCode;
+
+        use crate::GithubState;
+
+        use super::*;
+
+        macro_rules! status_tests{
+            ($($name:ident: $value:expr, )*) => {
+                $(
+                #[tokio::test]
+                async fn $name() {
+                    let (states, expected) = $value;
+                    let repo_details = given::repo_details();
+                    let commit = given::a_commit();
+                    let mut net = given::a_network();
+                    given::commit_states(&states, &mut net, &repo_details, &commit);
+                    let forge = given::a_github_forge(&repo_details, net);
+                    assert_eq!(forge.commit_status(&commit).await, expected);
+                }
+            )*
+            }
+        }
+
+        status_tests!(
+            pass_when_success: ([GithubState::Success], Status::Pass),
+            pending_when_pending: ([GithubState::Pending], Status::Pending),
+            fail_when_failure: ([GithubState::Failure], Status::Fail),
+            fail_when_error: ([GithubState::Error], Status::Fail),
+            pending_when_blank: ([GithubState::Blank], Status::Pending),
+            pass_wneh_no_checks: ([], Status::Pass),
+            pass_when_all_success: ([GithubState::Success, GithubState::Success], Status::Pass),
+            fail_when_only_alpha_fails: ([GithubState::Failure, GithubState::Success], Status::Fail),
+            fail_when_only_beta_fails: ([GithubState::Success, GithubState::Failure], Status::Fail),
+            pending_when_all_pending: ([GithubState::Pending, GithubState::Pending], Status::Pending),
+            pending_when_only_alpha_pending: ([GithubState::Pending, GithubState::Success], Status::Pending),
+            pending_when_only_beta_pending: ([GithubState::Success, GithubState::Pending], Status::Pending),
+
+        );
+
+        #[tokio::test]
+        async fn should_return_pending_for_no_statuses() {
+            let repo_details = given::repo_details();
+            let commit = given::a_commit();
+            let mut net = given::a_network();
+            net.add_get_response(
+                &given::a_commit_status_url(&repo_details, &commit),
+                StatusCode::OK,
+                "",
+            );
+            let forge = given::a_github_forge(&repo_details, net);
+            assert_eq!(forge.commit_status(&commit).await, Status::Pending);
+        }
+        #[tokio::test]
+        async fn should_return_pending_for_network_error() {
+            let repo_details = given::repo_details();
+            let commit = given::a_commit();
+            let mut net = given::a_network();
+            net.add_get_error(
+                &given::a_commit_status_url(&repo_details, &commit),
+                "boom today",
+            );
+            let forge = given::a_github_forge(&repo_details, net);
+            assert_eq!(forge.commit_status(&commit).await, Status::Pending);
+        }
+    }
+
+    mod list_webhooks {
+        use git_next_config::WebhookId;
+        use git_next_git::ForgeLike as _;
+
+        use crate::tests::github::with;
+
+        use super::*;
+
+        #[tokio::test]
+        async fn should_return_a_list_of_matching_webhooks() {
+            let repo_details = given::repo_details();
+            let webhook_url = given::any_webhook_url();
+            let hostname = repo_details.forge.hostname();
+            let repo_path = &repo_details.repo_path;
+            use secrecy::ExposeSecret;
+            let token = repo_details.forge.token().expose_secret();
+            let hook_id_1 = given::a_github_webhook_id();
+            let hook_id_2 = given::a_github_webhook_id();
+            let hook_id_3 = given::a_github_webhook_id();
+            let mut net = given::a_network();
+
+            let mut args = with::WebhookArgs {
+                net: &mut net,
+                hostname,
+                repo_path,
+                token,
+            };
+            // page 1 with three items
+            with::get_webhooks_by_page(
+                1,
+                &[
+                    with::ReturnedWebhook::new(hook_id_1, &webhook_url),
+                    with::ReturnedWebhook::new(hook_id_2, &webhook_url),
+                    with::ReturnedWebhook::new(hook_id_3, &given::any_webhook_url()),
+                ],
+                &mut args,
+            );
+            // page 2 with no items - stops pagination
+            with::get_webhooks_by_page(2, &[], &mut args);
+
+            let forge = given::a_github_forge(&repo_details, net);
+
+            let_assert!(Ok(result) = forge.list_webhooks(&webhook_url).await);
+            assert_eq!(
+                result,
+                vec![
+                    WebhookId::new(format!("{hook_id_1}")),
+                    WebhookId::new(format!("{hook_id_2}"))
+                ]
+            );
+        }
+
+        #[tokio::test]
+        async fn should_return_any_network_error() {
+            let repo_details = given::repo_details();
+            let webhook_url = given::a_webhook_url(&given::a_forge_alias(), &given::a_repo_alias());
+            let hostname = repo_details.forge.hostname();
+            let repo_path = &repo_details.repo_path;
+            let mut net = given::a_network();
+
+            net.add_get_error(
+                format!("https://api.{hostname}/repos/{repo_path}/hooks?page=1").as_str(),
+                "error_message",
+            );
+
+            let forge = given::a_github_forge(&repo_details, net);
+
+            let_assert!(Err(_) = forge.list_webhooks(&webhook_url).await);
+        }
+    }
+
+    mod unregister_webhook {
+        use super::*;
+        use git_next_git::ForgeLike;
+        use kxio::network::StatusCode;
+
+        #[tokio::test]
+        async fn should_delete_webhook() {
+            let repo_details = given::repo_details();
+            let hostname = repo_details.forge.hostname();
+            let repo_path = &repo_details.repo_path;
+            let webhook_id = given::a_webhook_id();
+            let mut net = given::a_network();
+
+            net.add_delete_response(
+                format!("https://api.{hostname}/repos/{repo_path}/hooks/{webhook_id}").as_str(),
+                StatusCode::OK,
+                "",
+            );
+
+            let forge = given::a_github_forge(&repo_details, net);
+
+            let_assert!(Ok(_) = forge.unregister_webhook(&webhook_id).await);
+        }
+        #[tokio::test]
+        async fn should_return_error_on_network_error() {
+            let repo_details = given::repo_details();
+            assert!(
+                repo_details.repo_config.is_some(),
+                "repo_details needs to have repo_config for this test"
+            );
+            let hostname = repo_details.forge.hostname();
+            let repo_path = &repo_details.repo_path;
+
+            let mut net = given::a_network();
+
+            // unregister the webhook will return empty response
+            net.add_delete_error(
+                format!("https://api.{hostname}/repos/{repo_path}/hooks").as_str(),
+                "error",
+            );
+
+            let forge = given::a_github_forge(&repo_details, net);
+
+            let webhook_id = given::a_webhook_id();
+            let_assert!(Err(err) = forge.unregister_webhook(&webhook_id).await);
+            assert!(
+                matches!(err, git::forge::webhook::Error::FailedToRegister(_)),
+                "{err:?}"
+            );
+        }
+    }
+
+    mod register_webhook {
+        use crate::tests::github::with;
+
+        use super::*;
+
+        use git_next_config::WebhookId;
+        use kxio::network::StatusCode;
+        use serde_json::json;
+
+        #[tokio::test]
+        async fn should_register_a_new_webhook() {
+            let repo_details = given::repo_details();
+            assert!(
+                repo_details.repo_config.is_some(),
+                "repo_details needs to have repo_config for this test"
+            );
+            let webhook_url =
+                given::a_webhook_url(repo_details.forge.forge_alias(), &repo_details.repo_alias);
+            let hostname = repo_details.forge.hostname();
+            let repo_path = &repo_details.repo_path;
+            use secrecy::ExposeSecret;
+            let token = repo_details.forge.token().expose_secret();
+
+            let mut net = given::a_network();
+            let mut args = with::WebhookArgs {
+                net: &mut net,
+                hostname,
+                repo_path,
+                token,
+            };
+
+            // there are no existing matching webhooks
+            with::get_webhooks_by_page(1, &[], &mut args);
+            // register the webhook will succeed
+            let webhook_id = given::a_github_webhook_id();
+            net.add_post_response(
+                format!("https://api.{hostname}/repos/{repo_path}/hooks").as_str(),
+                StatusCode::OK,
+                json!({"id": webhook_id, "config":{"url": webhook_url}})
+                    .to_string()
+                    .as_str(),
+            );
+
+            let forge = given::a_github_forge(&repo_details, net);
+
+            let_assert!(Ok(registered_webhook) = forge.register_webhook(&webhook_url).await);
+            assert_eq!(
+                registered_webhook.id(),
+                &WebhookId::new(format!("{webhook_id}"))
+            );
+        }
+
+        #[tokio::test]
+        async fn should_abort_if_repo_config_missing() {
+            let mut repo_details = given::repo_details();
+            repo_details.repo_config.take();
+            assert!(
+                repo_details.repo_config.is_none(),
+                "repo_details needs to NOT have repo_config for this test"
+            );
+            let webhook_url =
+                given::a_webhook_url(repo_details.forge.forge_alias(), &repo_details.repo_alias);
+            let net = given::a_network();
+            let forge = given::a_github_forge(&repo_details, net);
+            let_assert!(Err(err) = forge.register_webhook(&webhook_url).await);
+            assert!(
+                matches!(err, git::forge::webhook::Error::NoRepoConfig),
+                "{err:?}"
+            );
+        }
+
+        #[tokio::test]
+        async fn should_unregister_existing_webhooks_before_registering() {
+            let repo_details = given::repo_details();
+            assert!(
+                repo_details.repo_config.is_some(),
+                "repo_details needs to have repo_config for this test"
+            );
+            let webhook_url =
+                given::a_webhook_url(repo_details.forge.forge_alias(), &repo_details.repo_alias);
+            let hostname = repo_details.forge.hostname();
+            let repo_path = &repo_details.repo_path;
+            use secrecy::ExposeSecret;
+            let token = repo_details.forge.token().expose_secret();
+
+            let mut net = given::a_network();
+            let mut args = with::WebhookArgs {
+                net: &mut net,
+                hostname,
+                repo_path,
+                token,
+            };
+            let hook1 = with::ReturnedWebhook::new(given::a_github_webhook_id(), &webhook_url);
+            let hook2 = with::ReturnedWebhook::new(given::a_github_webhook_id(), &webhook_url);
+            let hook3 =
+                with::ReturnedWebhook::new(given::a_github_webhook_id(), &given::any_webhook_url());
+            let hooks = [hook1, hook2, hook3];
+
+            // there are three existing webhooks, two are matching webhooks
+            with::get_webhooks_by_page(1, &hooks, &mut args);
+            with::get_webhooks_by_page(2, &[], &mut args);
+            // should unregister 1 and 2, but not 3
+            with::unregister_webhook(&hooks[0], &mut args);
+            with::unregister_webhook(&hooks[1], &mut args);
+            // register the webhook will succeed
+            let webhook_id = given::a_github_webhook_id();
+            net.add_post_response(
+                format!("https://api.{hostname}/repos/{repo_path}/hooks").as_str(),
+                StatusCode::OK,
+                json!({"id": webhook_id, "config":{"url": webhook_url}})
+                    .to_string()
+                    .as_str(),
+            );
+
+            let forge = given::a_github_forge(&repo_details, net);
+
+            let_assert!(Ok(registered_webhook) = forge.register_webhook(&webhook_url).await);
+            assert_eq!(
+                registered_webhook.id(),
+                &WebhookId::new(format!("{webhook_id}"))
+            );
+        }
+
+        #[tokio::test]
+        async fn should_return_error_if_empty_network_response() {
+            let repo_details = given::repo_details();
+            assert!(
+                repo_details.repo_config.is_some(),
+                "repo_details needs to have repo_config for this test"
+            );
+            let webhook_url =
+                given::a_webhook_url(repo_details.forge.forge_alias(), &repo_details.repo_alias);
+            let hostname = repo_details.forge.hostname();
+            let repo_path = &repo_details.repo_path;
+            use secrecy::ExposeSecret;
+            let token = repo_details.forge.token().expose_secret();
+
+            let mut net = given::a_network();
+            let mut args = with::WebhookArgs {
+                net: &mut net,
+                hostname,
+                repo_path,
+                token,
+            };
+
+            // there are no existing matching webhooks
+            with::get_webhooks_by_page(1, &[], &mut args);
+            // register the webhook will return empty response
+            net.add_post_response(
+                format!("https://api.{hostname}/repos/{repo_path}/hooks").as_str(),
+                StatusCode::OK,
+                json!({}).to_string().as_str(), // empty response
+            );
+
+            let forge = given::a_github_forge(&repo_details, net);
+
+            let_assert!(Err(err) = forge.register_webhook(&webhook_url).await);
+            assert!(
+                matches!(err, git::forge::webhook::Error::FailedToRegister(_)),
+                "{err:?}"
+            );
+        }
+
+        #[tokio::test]
+        async fn should_return_error_on_network_error() {
+            let repo_details = given::repo_details();
+            assert!(
+                repo_details.repo_config.is_some(),
+                "repo_details needs to have repo_config for this test"
+            );
+            let webhook_url =
+                given::a_webhook_url(repo_details.forge.forge_alias(), &repo_details.repo_alias);
+            let hostname = repo_details.forge.hostname();
+            let repo_path = &repo_details.repo_path;
+            use secrecy::ExposeSecret;
+            let token = repo_details.forge.token().expose_secret();
+
+            let mut net = given::a_network();
+            let mut args = with::WebhookArgs {
+                net: &mut net,
+                hostname,
+                repo_path,
+                token,
+            };
+
+            // there are no existing matching webhooks
+            with::get_webhooks_by_page(1, &[], &mut args);
+            // register the webhook will return empty response
+            net.add_post_error(
+                format!("https://api.{hostname}/repos/{repo_path}/hooks").as_str(),
+                "error",
+            );
+
+            let forge = given::a_github_forge(&repo_details, net);
+
+            let_assert!(Err(err) = forge.register_webhook(&webhook_url).await);
+            assert!(
+                matches!(err, git::forge::webhook::Error::FailedToRegister(_)),
+                "{err:?}"
+            );
+        }
+    }
+
+    mod with {
+        use serde::Serialize;
+        use serde_json::json;
+
+        use git_next_config::{server::WebhookUrl, Hostname, RepoPath};
+        use kxio::network::{self, StatusCode};
+
+        pub fn get_webhooks_by_page(
+            page: u8,
+            response: &[ReturnedWebhook],
+            args: &mut WebhookArgs,
+        ) {
+            let hostname = args.hostname;
+            let repo_path = args.repo_path;
+            args.net.add_get_response(
+                format!("https://api.{hostname}/repos/{repo_path}/hooks?page={page}").as_str(),
+                StatusCode::OK,
+                json!(response).to_string().as_str(),
+            );
+        }
+
+        pub fn unregister_webhook(hook1: &ReturnedWebhook, args: &mut WebhookArgs) {
+            let webhook_id = hook1.id;
+            let hostname = args.hostname;
+            let repo_path = args.repo_path;
+            args.net.add_delete_response(
+                format!("https://api.{hostname}/repos/{repo_path}/hooks/{webhook_id}").as_str(),
+                StatusCode::OK,
+                "",
+            );
+        }
+        pub struct WebhookArgs<'a> {
+            pub net: &'a mut network::MockNetwork,
+            pub hostname: &'a Hostname,
+            pub repo_path: &'a RepoPath,
+            pub token: &'a str,
+        }
+        #[derive(Debug, Serialize)]
+        pub struct ReturnedWebhook {
+            pub id: i64,
+            pub config: Config,
+        }
+        impl ReturnedWebhook {
+            pub fn new(id: i64, url: &WebhookUrl) -> Self {
+                Self {
+                    id,
+                    config: Config { url: url.clone() },
+                }
+            }
+        }
+        #[derive(Debug, Serialize)]
+        pub struct Config {
+            pub url: WebhookUrl,
+        }
+    }
+
+    mod given {
+        use std::collections::HashMap;
+
+        use git_next_config::{server::Webhook, WebhookId};
+        use kxio::network::{MockNetwork, StatusCode};
+        use rand::RngCore;
+        use serde_json::json;
+
+        use crate::{GithubState, GithubStatus};
+
+        use super::*;
+
+        pub fn commit_states(
+            states: &[GithubState],
+            net: &mut MockNetwork,
+            repo_details: &git::RepoDetails,
+            commit: &git::Commit,
+        ) {
+            let response = json!(states
+                .iter()
+                .map(|state| GithubStatus {
+                    state: state.to_owned()
+                })
+                .collect::<Vec<_>>());
+            net.add_get_response(
+                a_commit_status_url(repo_details, commit).as_str(),
+                StatusCode::OK,
+                response.to_string().as_str(),
+            )
+        }
+
+        pub fn a_commit_status_url(
+            repo_details: &git::RepoDetails,
+            commit: &git::Commit,
+        ) -> String {
+            let hostname = repo_details.forge.hostname();
+            let repo_path = &repo_details.repo_path;
+            format!("https://api.{hostname}/repos/{repo_path}/commits/{commit}/statuses")
+        }
+
+        pub fn a_webhook_auth() -> WebhookAuth {
+            WebhookAuth::generate()
+        }
+
+        pub enum Header {
+            Valid(WebhookAuth, Body),
+            Missing,
+            Invalid,
+        }
+        pub fn a_webhook_message(header: Header) -> WebhookMessage {
+            let body = match &header {
+                Header::Valid(_, body) => body.clone(),
+                _ => given::a_webhook_message_body(),
+            };
+            WebhookMessage::new(
+                given::a_forge_alias(),
+                given::a_repo_alias(),
+                given::webhook_headers(header),
+                body,
+            )
+        }
+        pub fn webhook_headers(header: Header) -> HashMap<String, String> {
+            let mut headers = HashMap::new();
+            match header {
+                Header::Valid(auth, body) => {
+                    if let Some(sig) = crate::webhook::sign_body(&auth, &body) {
+                        headers.insert("x-hub-signature-256".to_string(), format!("sha256={sig}"));
+                    }
+                }
+                Header::Missing => { /* don't add any header */ }
+                Header::Invalid => {
+                    headers.insert(
+                        "x-hub-signature-256".to_string(),
+                        format!("{}", WebhookAuth::generate()),
+                    );
+                }
+            }
+            headers
+        }
+        pub fn a_webhook_message_body() -> Body {
+            Body::new(a_name())
+        }
+        pub fn a_commit() -> git::Commit {
+            git::Commit::new(
+                git::commit::Sha::new(a_name()),
+                git::commit::Message::new(a_name()),
+            )
+        }
+
+        pub fn repo_branches() -> RepoBranches {
+            RepoBranches::new(a_name(), a_name(), a_name())
+        }
+
+        pub fn a_github_forge(
+            repo_details: &git::RepoDetails,
+            net: impl Into<kxio::network::Network>,
+        ) -> Github {
+            Github::new(repo_details.clone(), net.into())
+        }
+        pub fn repo_details() -> git::RepoDetails {
+            git::RepoDetails::new(
+                git::Generation::new(),
+                &a_repo_alias(),
+                &ServerRepoConfig::new(
+                    format!("{}/{}", a_name(), a_name()), // repo path: owner/repo
+                    a_name(),
+                    None,
+                    Some(a_name()),
+                    Some(a_name()),
+                    Some(a_name()),
+                ),
+                &a_forge_alias(),
+                &ForgeConfig::new(
+                    ForgeType::ForgeJo,
+                    a_name(),
+                    a_name(),
+                    a_name(),
+                    BTreeMap::default(),
+                ),
+                GitDir::default(),
+            )
+        }
+
+        pub fn a_forge_alias() -> ForgeAlias {
+            ForgeAlias::new(a_name())
+        }
+
+        pub fn a_repo_alias() -> RepoAlias {
+            RepoAlias::new(a_name())
+        }
+        pub fn a_network() -> kxio::network::MockNetwork {
+            kxio::network::MockNetwork::new()
+        }
+
+        pub fn a_webhook_url(
+            forge_alias: &ForgeAlias,
+            repo_alias: &RepoAlias,
+        ) -> git_next_config::server::WebhookUrl {
+            Webhook::new(a_name()).url(forge_alias, repo_alias)
+        }
+        pub fn any_webhook_url() -> git_next_config::server::WebhookUrl {
+            given::a_webhook_url(&given::a_forge_alias(), &given::a_repo_alias())
+        }
+
+        pub fn a_name() -> String {
+            use rand::Rng;
+            use std::iter;
+
+            fn generate(len: usize) -> String {
+                const CHARSET: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+                let mut rng = rand::thread_rng();
+                let one_char = || CHARSET[rng.gen_range(0..CHARSET.len())] as char;
+                iter::repeat_with(one_char).take(len).collect()
+            }
+            generate(5)
+        }
+
+        pub fn a_webhook_id() -> WebhookId {
+            WebhookId::new(given::a_name())
+        }
+
+        pub fn a_github_webhook_id() -> i64 {
+            rand::thread_rng().next_u32().into()
+        }
+    }
 }

crates/forge-github/src/webhook/authorised.rs

@@ -2,25 +2,32 @@
 use git_next_config as config;
 
 pub fn is_authorised(msg: &config::WebhookMessage, webhook_auth: &config::WebhookAuth) -> bool {
-    let Some(github_signature) = msg
+    msg.header("x-hub-signature-256")
-        .header("x-hub-signature-256")
         .map(|x| x.trim_matches('"').to_string())
         .and_then(|sha| sha.strip_prefix("sha256=").map(|k| k.to_string()))
-    else {
+        .and_then(|github_signature| hex::decode(github_signature).ok())
-        tracing::warn!("no signature header found");
+        .and_then(|gh_sig| {
-        return false;
+            let payload = &msg.body().as_str();
-    };
+
-    let Ok(gh_sig) = hex::decode(github_signature) else {
+            use hmac::Mac;
-        eprintln!("can't decode github signature");
+            type HmacSha256 = hmac::Hmac<sha2::Sha256>;
-        return false;
+            let mut hmac = HmacSha256::new_from_slice(webhook_auth.to_string().as_bytes()).ok()?;
-    };
+            hmac::Mac::update(&mut hmac, payload.as_ref());
-    let payload = msg.body().as_str();
+            Some(hmac::Mac::verify_slice(hmac, gh_sig.as_ref()).is_ok())
+        })
+        .unwrap_or_default()
+}
+
+#[cfg(test)]
+pub fn sign_body(
+    webhook_auth: &config::WebhookAuth,
+    body: &config::webhook::message::Body,
+) -> std::option::Option<String> {
+    let payload = body.as_str();
     use hmac::Mac;
     type HmacSha256 = hmac::Hmac<sha2::Sha256>;
-    let Ok(mut hmac) = HmacSha256::new_from_slice(webhook_auth.to_string().as_bytes()) else {
+    let mut hmac = HmacSha256::new_from_slice(webhook_auth.to_string().as_bytes()).ok()?;
-        tracing::error!("failed to parse webhook auth token");
-        return false;
-    };
     hmac::Mac::update(&mut hmac, payload.as_ref());
-    hmac::Mac::verify_slice(hmac, gh_sig.as_ref()).is_ok()
+    let f = hmac::Mac::finalize(hmac);
+    Some(hex::encode(f.into_bytes()))
 }

crates/forge-github/src/webhook/list.rs

@@ -9,36 +9,45 @@ pub async fn list(
     github: &github::Github,
     webhook_url: &config::server::WebhookUrl,
 ) -> git::forge::webhook::Result<Vec<config::WebhookId>> {
-    let net = &github.net;
+    let mut ids: Vec<config::WebhookId> = vec![];
     let repo_details = &github.repo_details;
-    let request = network::NetRequest::new(
+    let hostname = &repo_details.forge.hostname();
-        network::RequestMethod::Delete,
+    let net = &github.net;
-        network::NetUrl::new(format!(
+    let mut page = 1;
-            "https://api.github.com/repos/{}/hooks/",
+    loop {
-            repo_details.repo_path,
+        let request = network::NetRequest::new(
-        )),
+            network::RequestMethod::Get,
-        github::webhook::headers(repo_details.forge.token()),
+            network::NetUrl::new(format!(
-        network::RequestBody::None,
+                "https://api.{hostname}/repos/{}/hooks?page={page}",
-        network::ResponseType::None,
+                repo_details.repo_path,
-        None,
+            )),
-        network::NetRequestLogging::None,
+            github::webhook::headers(repo_details.forge.token()),
-    );
+            network::RequestBody::None,
-    match net.post_json::<Vec<github::GithubHook>>(request).await {
+            network::ResponseType::Json,
-        Err(e) => {
+            None,
-            tracing::warn!("Failed to list webhooks");
+            network::NetRequestLogging::None,
-            Err(git::forge::webhook::Error::FailedToList(e.to_string()))
+        );
-        }
+        let result = net.get::<Vec<github::GithubHook>>(request).await;
-        Ok(response) => response.response_body().map_or_else(
+        match result {
-            || Ok(vec![]),
+            Ok(response) => {
-            |hooks| {
+                let Some(list) = response.response_body() else {
-                Ok(hooks
+                    #[cfg(not(tarpaulin_include))]
-                    .into_iter()
+                    // request response is Json so response_body never returns None
-                    .filter(|hook| &hook.config.url == webhook_url.as_ref())
+                    return Err(git::forge::webhook::Error::NetworkResponseEmpty);
-                    .map(|hook| hook.id)
+                };
-                    .map(|id| format!("{id}"))
+                if list.is_empty() {
-                    .map(config::WebhookId::new)
+                    return Ok(ids);
-                    .collect::<Vec<_>>())
+                }
-            },
+                for hook in list {
-        ),
+                    if hook.url().as_ref().starts_with(webhook_url.as_ref()) {
+                        ids.push(hook.id());
+                    }
+                }
+                page += 1;
+            }
+            Err(e) => {
+                return Err(git::forge::webhook::Error::Network(e));
+            }
+        };
     }
 }

crates/forge-github/src/webhook/mod.rs

@@ -14,6 +14,9 @@ pub use parse::parse_body;
 pub use register::register;
 pub use unregister::unregister;
 
+#[cfg(test)]
+pub use authorised::sign_body;
+
 pub fn headers(token: &config::ApiToken) -> kxio::network::NetRequestHeaders {
     use secrecy::ExposeSecret;
     kxio::network::NetRequestHeaders::default()

crates/forge-github/src/webhook/register.rs

@@ -10,13 +10,19 @@ pub async fn register(
     github: &github::Github,
     webhook_url: &config::server::WebhookUrl,
 ) -> git::forge::webhook::Result<config::RegisteredWebhook> {
-    let net = &github.net;
     let repo_details = &github.repo_details;
+    if repo_details.repo_config.is_none() {
+        return Err(git::forge::webhook::Error::NoRepoConfig);
+    };
+
+    let net = &github.net;
+
+    let hostname = repo_details.forge.hostname();
     let authorisation = config::WebhookAuth::generate();
     let request = network::NetRequest::new(
         network::RequestMethod::Post,
         network::NetUrl::new(format!(
-            "https://api.github.com/repos/{}/hooks",
+            "https://api.{hostname}/repos/{}/hooks",
             repo_details.repo_path
         )),
         github::webhook::headers(repo_details.forge.token()),
@@ -39,6 +45,8 @@ pub async fn register(
     match result {
         Ok(response) => {
             let Some(hook) = response.response_body() else {
+                #[cfg(not(tarpaulin_include))]
+                // request response is Json so response_body never returns None
                 return Err(git::forge::webhook::Error::NetworkResponseEmpty);
             };
             tracing::info!(webhook_id = %hook.id, "Webhook registered");

crates/forge-github/src/webhook/unregister.rs

@@ -12,10 +12,11 @@ pub async fn unregister(
 ) -> git::forge::webhook::Result<()> {
     let net = &github.net;
     let repo_details = &github.repo_details;
+    let hostname = repo_details.forge.hostname();
     let request = network::NetRequest::new(
         network::RequestMethod::Delete,
         network::NetUrl::new(format!(
-            "https://api.github.com/repos/{}/hooks/{}",
+            "https://api.{hostname}/repos/{}/hooks/{}",
             repo_details.repo_path, webhook_id
         )),
         github::webhook::headers(repo_details.forge.token()),
@@ -24,9 +25,8 @@ pub async fn unregister(
         None,
         network::NetRequestLogging::None,
     );
-    if let Err(e) = net.post_json::<github::GithubHook>(request).await {
+    net.delete(request)
-        tracing::warn!("Failed to register webhook");
+        .await
-        return Err(git::forge::webhook::Error::FailedToRegister(e.to_string()));
+        .map_err(|e| git::forge::webhook::Error::FailedToRegister(e.to_string()))
-    }
+        .map(|_| ())
-    Ok(())
 }

justfile

@@ -22,6 +22,10 @@ coverage-update:
   cargo tarpaulin --lib --out html
   echo "Now:\n\topen tarpaulin-report.html"
 
+coverage crate test:
+  cargo tarpaulin -p {{crate}} --lib --out html -- {{test}}
+  # cargo tarpaulin --skip-clean -p {{crate}} --lib --out html -- {{test}}
+  
 grcov-coverage:
   #!/usr/bin/env bash
   set -e