From 06103d8ec1583d002d604318c39dad014cae96ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 21 Jan 2024 12:30:35 +0100 Subject: [PATCH] Add branch protection tests --- src/github/api/read.rs | 1 + src/github/mod.rs | 2 +- src/github/tests/mod.rs | 264 ++++++++++++++++++++++++++++++++- src/github/tests/test_utils.rs | 92 +++++++++++- 4 files changed, 349 insertions(+), 10 deletions(-) diff --git a/src/github/api/read.rs b/src/github/api/read.rs index 91f29e8..ef6cbc8 100644 --- a/src/github/api/read.rs +++ b/src/github/api/read.rs @@ -48,6 +48,7 @@ pub(crate) trait GithubRead { fn repo_collaborators(&self, org: &str, repo: &str) -> anyhow::Result>; /// Get branch_protections + /// Returns a map branch pattern -> (protection ID, protection data) fn branch_protections( &self, org: &str, diff --git a/src/github/mod.rs b/src/github/mod.rs index d7bd2bd..73017dd 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -606,7 +606,7 @@ pub fn convert_permission(p: &rust_team_data::v1::RepoPermission) -> RepoPermiss } } -fn construct_branch_protection( +pub fn construct_branch_protection( expected_repo: &rust_team_data::v1::Repo, branch_protection: &rust_team_data::v1::BranchProtection, ) -> api::BranchProtection { diff --git a/src/github/tests/mod.rs b/src/github/tests/mod.rs index 7038787..ce13577 100644 --- a/src/github/tests/mod.rs +++ b/src/github/tests/mod.rs @@ -1,5 +1,5 @@ -use crate::github::tests::test_utils::{DataModel, RepoData, TeamData}; -use rust_team_data::v1::RepoPermission; +use crate::github::tests::test_utils::{BranchProtectionBuilder, DataModel, RepoData, TeamData}; +use rust_team_data::v1::{BranchProtectionMode, RepoPermission}; mod test_utils; @@ -307,7 +307,13 @@ fn repo_create() { RepoData::new("repo1") .description("foo".to_string()) .member("user1", RepoPermission::Write) - .team("team1", RepoPermission::Triage), + .team("team1", RepoPermission::Triage) + .branch_protections(vec![BranchProtectionBuilder::pr_required( + "main", + &["test"], + 1, + ) + .build()]), ); let diff = model.diff_repos(gh); insta::assert_debug_snapshot!(diff, @r#" @@ -342,7 +348,22 @@ fn repo_create() { ), }, ], - branch_protections: [], + branch_protections: [ + ( + "main", + BranchProtection { + pattern: "main", + is_admin_enforced: true, + dismisses_stale_reviews: false, + required_approving_review_count: 1, + required_status_check_contexts: [ + "test", + ], + push_allowances: [], + requires_approving_reviews: true, + }, + ), + ], app_installations: [], }, ), @@ -726,3 +747,238 @@ fn repo_archive_repo() { ] "#); } + +#[test] +fn repo_add_branch_protection() { + let mut model = DataModel::default(); + model.create_repo(RepoData::new("repo1").team("team1", RepoPermission::Write)); + + let gh = model.gh_model(); + model.get_repo("repo1").branch_protections.extend([ + BranchProtectionBuilder::pr_required("master", &["test", "test 2"], 0).build(), + BranchProtectionBuilder::pr_not_required("beta").build(), + ]); + + let diff = model.diff_repos(gh); + insta::assert_debug_snapshot!(diff, @r#" + [ + Update( + UpdateRepoDiff { + org: "rust-lang", + name: "repo1", + repo_node_id: "0", + repo_id: 0, + settings_diff: ( + RepoSettings { + description: Some( + "", + ), + homepage: None, + archived: false, + auto_merge_enabled: false, + }, + RepoSettings { + description: Some( + "", + ), + homepage: None, + archived: false, + auto_merge_enabled: false, + }, + ), + permission_diffs: [], + branch_protection_diffs: [ + BranchProtectionDiff { + pattern: "master", + operation: Create( + BranchProtection { + pattern: "master", + is_admin_enforced: true, + dismisses_stale_reviews: false, + required_approving_review_count: 0, + required_status_check_contexts: [ + "test", + "test 2", + ], + push_allowances: [], + requires_approving_reviews: true, + }, + ), + }, + BranchProtectionDiff { + pattern: "beta", + operation: Create( + BranchProtection { + pattern: "beta", + is_admin_enforced: true, + dismisses_stale_reviews: false, + required_approving_review_count: 0, + required_status_check_contexts: [], + push_allowances: [], + requires_approving_reviews: false, + }, + ), + }, + ], + app_installation_diffs: [], + }, + ), + ] + "#); +} + +#[test] +fn repo_update_branch_protection() { + let mut model = DataModel::default(); + model.create_repo( + RepoData::new("repo1") + .team("team1", RepoPermission::Write) + .branch_protections(vec![BranchProtectionBuilder::pr_required( + "master", + &["test"], + 1, + ) + .build()]), + ); + + let gh = model.gh_model(); + let protection = model + .get_repo("repo1") + .branch_protections + .last_mut() + .unwrap(); + match &mut protection.mode { + BranchProtectionMode::PrRequired { + ci_checks, + required_approvals, + } => { + ci_checks.push("Test".to_string()); + *required_approvals = 0; + } + BranchProtectionMode::PrNotRequired => unreachable!(), + } + protection.dismiss_stale_review = true; + + let diff = model.diff_repos(gh); + insta::assert_debug_snapshot!(diff, @r#" + [ + Update( + UpdateRepoDiff { + org: "rust-lang", + name: "repo1", + repo_node_id: "0", + repo_id: 0, + settings_diff: ( + RepoSettings { + description: Some( + "", + ), + homepage: None, + archived: false, + auto_merge_enabled: false, + }, + RepoSettings { + description: Some( + "", + ), + homepage: None, + archived: false, + auto_merge_enabled: false, + }, + ), + permission_diffs: [], + branch_protection_diffs: [ + BranchProtectionDiff { + pattern: "master", + operation: Update( + "0", + BranchProtection { + pattern: "master", + is_admin_enforced: true, + dismisses_stale_reviews: false, + required_approving_review_count: 1, + required_status_check_contexts: [ + "test", + ], + push_allowances: [], + requires_approving_reviews: true, + }, + BranchProtection { + pattern: "master", + is_admin_enforced: true, + dismisses_stale_reviews: true, + required_approving_review_count: 0, + required_status_check_contexts: [ + "test", + "Test", + ], + push_allowances: [], + requires_approving_reviews: true, + }, + ), + }, + ], + app_installation_diffs: [], + }, + ), + ] + "#); +} + +#[test] +fn repo_remove_branch_protection() { + let mut model = DataModel::default(); + model.create_repo( + RepoData::new("repo1") + .team("team1", RepoPermission::Write) + .branch_protections(vec![ + BranchProtectionBuilder::pr_required("main", &["test"], 1).build(), + BranchProtectionBuilder::pr_required("stable", &["test"], 0).build(), + ]), + ); + + let gh = model.gh_model(); + model.get_repo("repo1").branch_protections.pop().unwrap(); + + let diff = model.diff_repos(gh); + insta::assert_debug_snapshot!(diff, @r#" + [ + Update( + UpdateRepoDiff { + org: "rust-lang", + name: "repo1", + repo_node_id: "0", + repo_id: 0, + settings_diff: ( + RepoSettings { + description: Some( + "", + ), + homepage: None, + archived: false, + auto_merge_enabled: false, + }, + RepoSettings { + description: Some( + "", + ), + homepage: None, + archived: false, + auto_merge_enabled: false, + }, + ), + permission_diffs: [], + branch_protection_diffs: [ + BranchProtectionDiff { + pattern: "stable", + operation: Delete( + "1", + ), + }, + ], + app_installation_diffs: [], + }, + ), + ] + "#); +} diff --git a/src/github/tests/test_utils.rs b/src/github/tests/test_utils.rs index 304955a..656b397 100644 --- a/src/github/tests/test_utils.rs +++ b/src/github/tests/test_utils.rs @@ -2,13 +2,17 @@ use std::collections::{HashMap, HashSet}; use derive_builder::Builder; use rust_team_data::v1; -use rust_team_data::v1::{Bot, GitHubTeam, Person, RepoPermission, TeamGitHub, TeamKind}; +use rust_team_data::v1::{ + Bot, BranchProtectionMode, GitHubTeam, MergeBot, Person, RepoPermission, TeamGitHub, TeamKind, +}; use crate::github::api::{ BranchProtection, GithubRead, OrgAppInstallation, Repo, RepoAppInstallation, RepoTeam, RepoUser, Team, TeamMember, TeamPrivacy, TeamRole, }; -use crate::github::{api, convert_permission, RepoDiff, SyncGitHub, TeamDiff}; +use crate::github::{ + api, construct_branch_protection, convert_permission, RepoDiff, SyncGitHub, TeamDiff, +}; const DEFAULT_ORG: &str = "rust-lang"; @@ -105,6 +109,7 @@ impl DataModel { let mut repos = HashMap::default(); let mut repo_members: HashMap = HashMap::default(); + let mut branch_protections = HashMap::new(); for repo in &self.repos { repos.insert( @@ -144,6 +149,16 @@ impl DataModel { }) .collect(); repo_members.insert(repo.name.clone(), RepoMembers { teams, members }); + + let repo_v1: v1::Repo = repo.clone().into(); + let mut protections = vec![]; + for protection in &repo.branch_protections { + protections.push(( + format!("{}", protections.len()), + construct_branch_protection(&repo_v1, protection), + )); + } + branch_protections.insert(repo.name.clone(), protections); } GithubMock { @@ -154,6 +169,7 @@ impl DataModel { team_invitations: Default::default(), repos, repo_members, + branch_protections, } } @@ -265,6 +281,8 @@ pub struct RepoData { pub archived: bool, #[builder(default)] pub allow_auto_merge: bool, + #[builder(default)] + pub branch_protections: Vec, } impl RepoData { @@ -298,6 +316,7 @@ impl From for v1::Repo { members, archived, allow_auto_merge, + branch_protections, } = value; Self { org: DEFAULT_ORG.to_string(), @@ -307,7 +326,7 @@ impl From for v1::Repo { bots, teams: teams.clone(), members: members.clone(), - branch_protections: vec![], + branch_protections, archived, private: false, auto_merge_enabled: allow_auto_merge, @@ -337,6 +356,58 @@ impl RepoDataBuilder { } } +#[derive(Clone)] +pub struct BranchProtectionBuilder { + pub pattern: String, + pub dismiss_stale_review: bool, + pub mode: BranchProtectionMode, + pub allowed_merge_teams: Vec, + pub merge_bots: Vec, +} + +impl BranchProtectionBuilder { + pub fn pr_required(pattern: &str, ci_checks: &[&str], required_approvals: u32) -> Self { + Self::create( + pattern, + BranchProtectionMode::PrRequired { + ci_checks: ci_checks.iter().map(|s| s.to_string()).collect(), + required_approvals, + }, + ) + } + + pub fn pr_not_required(pattern: &str) -> Self { + Self::create(pattern, BranchProtectionMode::PrNotRequired) + } + + pub fn build(self) -> v1::BranchProtection { + let BranchProtectionBuilder { + pattern, + dismiss_stale_review, + mode, + allowed_merge_teams, + merge_bots, + } = self; + v1::BranchProtection { + pattern, + dismiss_stale_review, + mode, + allowed_merge_teams, + merge_bots, + } + } + + fn create(pattern: &str, mode: BranchProtectionMode) -> Self { + Self { + pattern: pattern.to_string(), + mode, + dismiss_stale_review: false, + allowed_merge_teams: vec![], + merge_bots: vec![], + } + } +} + /// Represents the state of GitHub repositories, teams and users. #[derive(Default)] pub struct GithubMock { @@ -353,6 +424,8 @@ pub struct GithubMock { repos: HashMap, // Repo name -> (teams, members) repo_members: HashMap, + // Repo name -> Vec<(protection ID, branch protection)> + branch_protections: HashMap>, } impl GithubMock { @@ -461,10 +534,19 @@ impl GithubRead for GithubMock { fn branch_protections( &self, org: &str, - _repo: &str, + repo: &str, ) -> anyhow::Result> { assert_eq!(org, DEFAULT_ORG); - Ok(HashMap::default()) + + let Some(protections) = self.branch_protections.get(repo) else { + return Ok(Default::default()); + }; + let mut result = HashMap::default(); + for (id, protection) in protections { + result.insert(protection.pattern.clone(), (id.clone(), protection.clone())); + } + + Ok(result) } }