From ac3e1be261e668762f879f9011de65a1d6e0cfa1 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 14 May 2024 07:59:31 +0100 Subject: [PATCH] test: add some tests --- crates/config/Cargo.toml | 6 +- crates/config/src/forge_config.rs | 8 +- crates/config/src/host_name.rs | 7 +- crates/config/src/lib.rs | 3 + crates/config/src/repo_alias.rs | 7 +- crates/config/src/tests.rs | 132 +++++++++++++++++++++ crates/git/src/validate.rs | 2 +- crates/server/src/actors/repo/branch.rs | 21 +--- crates/server/src/actors/repo/mod.rs | 3 + crates/server/src/actors/repo/tests.rs | 31 +++++ crates/server/src/actors/repo/webhook.rs | 15 +-- crates/server/src/actors/webhook/router.rs | 2 +- crates/server/src/config/tests.rs | 12 +- crates/server/src/gitforge/tests/common.rs | 4 +- 14 files changed, 206 insertions(+), 47 deletions(-) create mode 100644 crates/config/src/tests.rs create mode 100644 crates/server/src/actors/repo/tests.rs diff --git a/crates/config/Cargo.toml b/crates/config/Cargo.toml index 9e4800f..5643845 100644 --- a/crates/config/Cargo.toml +++ b/crates/config/Cargo.toml @@ -56,10 +56,10 @@ derive_more = { workspace = true } # actix-rt = { workspace = true } # tokio = { workspace = true } # -# [dev-dependencies] +[dev-dependencies] # # Testing -# assert2 = { workspace = true } -# pretty_assertions = { workspace = true } +assert2 = { workspace = true } +pretty_assertions = { workspace = true } # test-log = { workspace = true } # anyhow = { workspace = true } diff --git a/crates/config/src/forge_config.rs b/crates/config/src/forge_config.rs index 1d2fe19..a8ec57f 100644 --- a/crates/config/src/forge_config.rs +++ b/crates/config/src/forge_config.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::BTreeMap; use crate::{ApiToken, ForgeType, Hostname, RepoAlias, ServerRepoConfig, User}; @@ -11,7 +11,7 @@ pub struct ForgeConfig { pub hostname: String, pub user: String, pub token: String, - pub repos: HashMap, + pub repos: BTreeMap, } impl ForgeConfig { pub const fn forge_type(&self) -> ForgeType { @@ -19,7 +19,7 @@ impl ForgeConfig { } pub fn hostname(&self) -> Hostname { - Hostname(self.hostname.clone()) + Hostname::new(&self.hostname) } pub fn user(&self) -> User { @@ -33,6 +33,6 @@ impl ForgeConfig { pub fn repos(&self) -> impl Iterator { self.repos .iter() - .map(|(name, repo)| (RepoAlias(name.clone()), repo)) + .map(|(name, repo)| (RepoAlias::new(name), repo)) } } diff --git a/crates/config/src/host_name.rs b/crates/config/src/host_name.rs index 6fb9316..a1b7b15 100644 --- a/crates/config/src/host_name.rs +++ b/crates/config/src/host_name.rs @@ -1,3 +1,8 @@ /// The hostname of a forge #[derive(Clone, Debug, PartialEq, Eq, derive_more::Display)] -pub struct Hostname(pub String); +pub struct Hostname(String); +impl Hostname { + pub fn new(str: impl Into) -> Self { + Self(str.into()) + } +} diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index a284394..bda164a 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -15,6 +15,9 @@ mod repo_path; mod server_repo_config; mod user; +#[cfg(test)] +mod tests; + pub use api_token::ApiToken; pub use branch_name::BranchName; pub use forge_config::ForgeConfig; diff --git a/crates/config/src/repo_alias.rs b/crates/config/src/repo_alias.rs index 1e130ad..38091fd 100644 --- a/crates/config/src/repo_alias.rs +++ b/crates/config/src/repo_alias.rs @@ -1,4 +1,9 @@ /// The alias of a repo /// This is the alias for the repo within `git-next-server.toml` #[derive(Clone, Debug, PartialEq, Eq, Hash, derive_more::Display)] -pub struct RepoAlias(pub String); +pub struct RepoAlias(String); +impl RepoAlias { + pub fn new(str: impl Into) -> Self { + Self(str.into()) + } +} diff --git a/crates/config/src/tests.rs b/crates/config/src/tests.rs new file mode 100644 index 0000000..4b3c9ab --- /dev/null +++ b/crates/config/src/tests.rs @@ -0,0 +1,132 @@ +type TestResult = Result<(), Box>; + +mod server_repo_config { + use assert2::let_assert; + + use crate::{RepoBranches, RepoConfig, RepoConfigSource}; + + use super::super::server_repo_config::*; + + #[test] + fn should_not_return_repo_config_when_no_branches() { + let src = ServerRepoConfig { + repo: "".to_string(), + branch: "".to_string(), + gitdir: None, + main: None, + next: None, + dev: None, + }; + + let_assert!(None = src.repo_config()); + } + + #[test] + fn should_return_repo_config_when_branches() { + let src = ServerRepoConfig { + repo: "".to_string(), + branch: "".to_string(), + gitdir: None, + main: Some("main".to_string()), + next: Some("next".to_string()), + dev: Some("dev".to_string()), + }; + + let_assert!(Some(rc) = src.repo_config()); + assert_eq!( + rc, + RepoConfig { + branches: RepoBranches { + main: "main".to_string(), + next: "next".to_string(), + dev: "dev".to_string() + }, + source: RepoConfigSource::Server + } + ); + } +} +mod repo_config { + use crate::{RepoBranches, RepoConfigSource}; + + use super::super::repo_config::*; + use super::*; + + #[test] + fn should_parse_toml() -> TestResult { + let toml = r#" + [branches] + main = "main" + next = "next" + dev = "dev" + "#; + + let rc = RepoConfig::load(toml)?; + + assert_eq!( + rc, + RepoConfig { + branches: RepoBranches { + main: "main".to_string(), + next: "next".to_string(), + dev: "dev".to_string(), + }, + source: RepoConfigSource::Repo // reading from repo is the default + } + ); + + Ok(()) + } +} +mod forge_config { + use std::collections::BTreeMap; + + use crate::{ForgeConfig, ForgeType, RepoAlias, ServerRepoConfig}; + + use super::*; + #[test] + fn should_return_repos() -> TestResult { + let forge_type = ForgeType::MockForge; + let hostname = "localhost".to_string(); + let user = "bob".to_string(); + let token = "alpha".to_string(); + let red = ServerRepoConfig { + repo: "red".to_string(), + branch: "main".to_string(), + main: None, + next: None, + dev: None, + gitdir: None, + }; + let blue = ServerRepoConfig { + repo: "blue".to_string(), + branch: "main".to_string(), + main: None, + next: None, + dev: None, + gitdir: None, + }; + let mut repos = BTreeMap::new(); + repos.insert("red".to_string(), red.clone()); + repos.insert("blue".to_string(), blue.clone()); + let fc = ForgeConfig { + forge_type, + hostname, + user, + token, + repos, + }; + + let returned_repos = fc.repos().collect::>(); + + assert_eq!( + returned_repos, + vec![ + // alphabetical order by key + (RepoAlias::new("blue"), &blue), + (RepoAlias::new("red"), &red), + ] + ); + Ok(()) + } +} diff --git a/crates/git/src/validate.rs b/crates/git/src/validate.rs index fa352b5..f6b93d4 100644 --- a/crates/git/src/validate.rs +++ b/crates/git/src/validate.rs @@ -47,7 +47,7 @@ pub fn find_default_remote( let path = path.strip_suffix(".git").map_or(path, |path| path); info!(%host, %path, "found"); Ok(GitRemote::new( - Hostname(host.to_string()), + Hostname::new(host), RepoPath(path.to_string()), )) } diff --git a/crates/server/src/actors/repo/branch.rs b/crates/server/src/actors/repo/branch.rs index 24baab4..bab06f1 100644 --- a/crates/server/src/actors/repo/branch.rs +++ b/crates/server/src/actors/repo/branch.rs @@ -62,7 +62,7 @@ fn validate_commit_message(message: &git::commit::Message) -> Option { } } -fn find_next_commit_on_dev( +pub fn find_next_commit_on_dev( next: git::Commit, dev_commit_history: Vec, ) -> Option { @@ -100,22 +100,3 @@ pub async fn advance_main( RepoConfigSource::Server => addr.do_send(ValidateRepo { message_token }), } } - -#[cfg(test)] -mod tests { - use super::*; - - #[actix_rt::test] - async fn test_find_next_commit_on_dev() { - let next = git::Commit::new("current-next", "foo"); - let expected = git::Commit::new("dev-next", "next-should-go-here"); - let dev_commit_history = vec![ - git::Commit::new("dev", "future"), - expected.clone(), - next.clone(), - git::Commit::new("current-main", "history"), - ]; - let next_commit = find_next_commit_on_dev(next, dev_commit_history); - assert_eq!(next_commit, Some(expected)); - } -} diff --git a/crates/server/src/actors/repo/mod.rs b/crates/server/src/actors/repo/mod.rs index 31afbee..1a0cbd9 100644 --- a/crates/server/src/actors/repo/mod.rs +++ b/crates/server/src/actors/repo/mod.rs @@ -3,6 +3,9 @@ mod config; pub mod status; pub mod webhook; +#[cfg(test)] +mod tests; + use actix::prelude::*; use git_next_config::{ForgeType, RepoConfig}; use git_next_git::{self as git, Generation, RepoDetails}; diff --git a/crates/server/src/actors/repo/tests.rs b/crates/server/src/actors/repo/tests.rs new file mode 100644 index 0000000..dc75c13 --- /dev/null +++ b/crates/server/src/actors/repo/tests.rs @@ -0,0 +1,31 @@ +// +use super::*; +mod branch { + use super::super::branch::*; + use super::*; + + #[actix_rt::test] + async fn test_find_next_commit_on_dev() { + let next = git::Commit::new("current-next", "foo"); + let expected = git::Commit::new("dev-next", "next-should-go-here"); + let dev_commit_history = vec![ + git::Commit::new("dev", "future"), + expected.clone(), + next.clone(), + git::Commit::new("current-main", "history"), + ]; + + let next_commit = find_next_commit_on_dev(next, dev_commit_history); + + assert_eq!(next_commit, Some(expected), "Found the wrong commit"); + } +} + +mod webhook { + use super::super::webhook::*; + + #[test] + fn should_split_ref() { + assert_eq!(split_ref("refs/heads/next"), ("refs/heads/", "next")); + } +} diff --git a/crates/server/src/actors/repo/webhook.rs b/crates/server/src/actors/repo/webhook.rs index 199a298..552d10b 100644 --- a/crates/server/src/actors/repo/webhook.rs +++ b/crates/server/src/actors/repo/webhook.rs @@ -285,7 +285,7 @@ impl Push { warn!(r#ref = self.reference, "Unexpected ref"); return None; } - let (_, branch) = self.reference.split_at(11); + let (_, branch) = split_ref(&self.reference); let branch = BranchName::new(branch); if branch == repo_branches.main() { return Some(Branch::Main); @@ -299,11 +299,16 @@ impl Push { warn!(%branch, "Unexpected branch"); None } + pub fn commit(&self) -> git::Commit { git::Commit::new(&self.after, &self.head_commit.message) } } +pub fn split_ref(reference: &str) -> (&str, &str) { + reference.split_at(11) +} + #[derive(Debug)] enum Branch { Main, @@ -315,11 +320,3 @@ enum Branch { struct HeadCommit { message: String, } - -#[cfg(test)] -mod tests { - #[test] - fn splt_ref() { - assert_eq!("refs/heads/next".split_at(11), ("refs/heads/", "next")); - } -} diff --git a/crates/server/src/actors/webhook/router.rs b/crates/server/src/actors/webhook/router.rs index 84e662d..381d23f 100644 --- a/crates/server/src/actors/webhook/router.rs +++ b/crates/server/src/actors/webhook/router.rs @@ -28,7 +28,7 @@ impl Handler for WebhookRouter { fn handle(&mut self, msg: WebhookMessage, _ctx: &mut Self::Context) -> Self::Result { let _gaurd = self.span.enter(); - let repo_alias = RepoAlias(msg.path().clone()); + let repo_alias = RepoAlias::new(msg.path()); debug!(repo = %repo_alias, "Router..."); if let Some(recipient) = self.repos.get(&repo_alias) { info!(repo = %repo_alias, "Sending to Recipient"); diff --git a/crates/server/src/config/tests.rs b/crates/server/src/config/tests.rs index aef989e..28dff7a 100644 --- a/crates/server/src/config/tests.rs +++ b/crates/server/src/config/tests.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeMap; + use assert2::let_assert; use git_next_config::{ ForgeType, GitDir, Hostname, RepoBranches, RepoConfig, RepoConfigSource, RepoPath, @@ -68,7 +70,7 @@ fn load_should_parse_server_config() -> Result<()> { hostname: "git.example.net".to_string(), user: "Bob".to_string(), token: "API-Token".to_string(), - repos: HashMap::from([ + repos: BTreeMap::from([ ( "hello".to_string(), ServerRepoConfig { @@ -176,7 +178,7 @@ fn repo_details_find_default_push_remote_finds_correct_remote() -> Result<()> { None, GitDir::new(root), // Server GitDir - should be ignored ); - repo_details.forge.hostname = Hostname("git.kemitix.net".to_string()); + repo_details.forge.hostname = Hostname::new("git.kemitix.net"); repo_details.repo_path = RepoPath("kemitix/git-next".to_string()); let gitdir = &repo_details.gitdir; let repository = Repository::open(gitdir)?; @@ -202,7 +204,7 @@ fn gitdir_validate_should_pass_a_valid_git_repo() -> Result<()> { None, GitDir::new(root), // Server GitDir - should be ignored ); - repo_details.forge.hostname = Hostname("git.kemitix.net".to_string()); + repo_details.forge.hostname = Hostname::new("git.kemitix.net"); repo_details.repo_path = RepoPath("kemitix/git-next".to_string()); let gitdir = &repo_details.gitdir; let repository = Repository::open(gitdir)?; @@ -222,7 +224,7 @@ fn gitdir_validate_should_fail_a_git_repo_with_wrong_remote() -> Result<()> { None, GitDir::new(root), // Server GitDir - should be ignored ); - repo_details.forge.hostname = Hostname("localhost".to_string()); + repo_details.forge.hostname = Hostname::new("localhost"); repo_details.repo_path = RepoPath("hello/world".to_string()); let gitdir = &repo_details.gitdir; let repository = Repository::open(gitdir)?; @@ -233,7 +235,7 @@ fn gitdir_validate_should_fail_a_git_repo_with_wrong_remote() -> Result<()> { #[test] fn git_remote_to_string_is_as_expected() { - let git_remote = GitRemote::new(Hostname("foo".to_string()), RepoPath("bar".to_string())); + let git_remote = GitRemote::new(Hostname::new("foo"), RepoPath("bar".to_string())); let as_string = git_remote.to_string(); assert_eq!(as_string, "foo:bar"); diff --git a/crates/server/src/gitforge/tests/common.rs b/crates/server/src/gitforge/tests/common.rs index b359a97..2385846 100644 --- a/crates/server/src/gitforge/tests/common.rs +++ b/crates/server/src/gitforge/tests/common.rs @@ -24,7 +24,7 @@ pub fn user(n: u32) -> User { } pub fn hostname(n: u32) -> Hostname { - Hostname(format!("hostname-{}", n)) + Hostname::new(format!("hostname-{}", n)) } pub fn forge_name(n: u32) -> ForgeName { @@ -57,7 +57,7 @@ pub fn repo_path(n: u32) -> RepoPath { } pub fn repo_alias(n: u32) -> RepoAlias { - RepoAlias(format!("repo-alias-{}", n)) + RepoAlias::new(format!("repo-alias-{}", n)) } pub fn repo_config(n: u32, source: RepoConfigSource) -> RepoConfig {