From 7a2a8df41c777b9005540931a9827a64805c7bd8 Mon Sep 17 00:00:00 2001 From: Diogo Teles Sant'Anna Date: Tue, 19 Sep 2023 19:09:47 +0000 Subject: [PATCH] test(branch-protection): increment and adapt testing 1. Adapt previous test cases to consider that now we'll have an aditional Info log telling that the project requires PRs to make changes. 2. Add more cases to test relevant use cases on the tier 2 level of branch protection Signed-off-by: Diogo Teles Sant'Anna --- checks/branch_protection_test.go | 8 +- checks/evaluation/branch_protection_test.go | 160 +++++++++++++++++--- 2 files changed, 146 insertions(+), 22 deletions(-) diff --git a/checks/branch_protection_test.go b/checks/branch_protection_test.go index 929c20768c8..cdec24819d7 100644 --- a/checks/branch_protection_test.go +++ b/checks/branch_protection_test.go @@ -136,7 +136,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { Error: nil, Score: 2, NumberOfWarn: 7, - NumberOfInfo: 2, + NumberOfInfo: 3, NumberOfDebug: 0, }, defaultBranch: main, @@ -175,7 +175,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { Error: nil, Score: 2, NumberOfWarn: 9, - NumberOfInfo: 10, + NumberOfInfo: 12, NumberOfDebug: 0, }, defaultBranch: main, @@ -231,7 +231,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { Error: nil, Score: 8, NumberOfWarn: 4, - NumberOfInfo: 16, + NumberOfInfo: 18, NumberOfDebug: 0, }, defaultBranch: main, @@ -287,7 +287,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { Error: nil, Score: 2, NumberOfWarn: 7, - NumberOfInfo: 2, + NumberOfInfo: 3, NumberOfDebug: 0, }, defaultBranch: main, diff --git a/checks/evaluation/branch_protection_test.go b/checks/evaluation/branch_protection_test.go index 5fd5e541edb..362e4596a88 100644 --- a/checks/evaluation/branch_protection_test.go +++ b/checks/evaluation/branch_protection_test.go @@ -55,7 +55,7 @@ func TestIsBranchProtected(t *testing.T) { Error: nil, Score: 2, NumberOfWarn: 7, - NumberOfInfo: 2, + NumberOfInfo: 3, NumberOfDebug: 0, }, branch: &clients.BranchRef{ @@ -100,7 +100,7 @@ func TestIsBranchProtected(t *testing.T) { Error: nil, Score: 2, NumberOfWarn: 5, - NumberOfInfo: 4, + NumberOfInfo: 5, NumberOfDebug: 0, }, branch: &clients.BranchRef{ @@ -131,7 +131,7 @@ func TestIsBranchProtected(t *testing.T) { Error: nil, Score: 2, NumberOfWarn: 6, - NumberOfInfo: 3, + NumberOfInfo: 4, NumberOfDebug: 0, }, branch: &clients.BranchRef{ @@ -157,28 +157,152 @@ func TestIsBranchProtected(t *testing.T) { }, }, { - name: "Required pull request enabled", + name: "Admin run with all tier 2 requirements except require PRs and reviewers", expected: scut.TestReturn{ Error: nil, - Score: 2, - NumberOfWarn: 6, - NumberOfInfo: 3, + Score: 4, // Should be 4.2 if we allow decimal puctuation + NumberOfWarn: 3, + NumberOfInfo: 6, NumberOfDebug: 0, }, branch: &clients.BranchRef{ Name: &branchVal, Protected: &trueVal, BranchProtectionRule: clients.BranchProtectionRule{ - EnforceAdmins: &falseVal, - RequireLastPushApproval: &falseVal, + EnforceAdmins: &trueVal, + RequireLastPushApproval: &trueVal, RequireLinearHistory: &trueVal, AllowForcePushes: &falseVal, AllowDeletions: &falseVal, CheckRules: clients.StatusChecksRule{ - RequiresStatusChecks: &trueVal, - UpToDateBeforeMerge: &falseVal, + RequiresStatusChecks: &falseVal, + UpToDateBeforeMerge: &trueVal, + Contexts: []string{"foo"}, + }, + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + DismissStaleReviews: &falseVal, + RequireCodeOwnerReviews: &falseVal, + RequiredApprovingReviewCount: nil, + }, + }, + }, + }, + { + name: "Admin run on project requiring pull requests but without approver -- best a single maintainer can do", + expected: scut.TestReturn{ + Error: nil, + Score: 4, // Should be 4.8 if we allow decimal punctuation + NumberOfWarn: 3, + NumberOfInfo: 7, + NumberOfDebug: 0, + }, + branch: &clients.BranchRef{ + Name: &branchVal, + Protected: &trueVal, + BranchProtectionRule: clients.BranchProtectionRule{ + EnforceAdmins: &trueVal, + RequireLastPushApproval: &trueVal, + RequireLinearHistory: &trueVal, + AllowForcePushes: &falseVal, + AllowDeletions: &falseVal, + CheckRules: clients.StatusChecksRule{ + RequiresStatusChecks: &falseVal, + UpToDateBeforeMerge: &trueVal, Contexts: []string{"foo"}, }, + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + DismissStaleReviews: &falseVal, + RequireCodeOwnerReviews: &falseVal, + RequiredApprovingReviewCount: &zeroVal, + }, + }, + }, + }, + { + name: "Admin run on project with all tier 2 requirements", + expected: scut.TestReturn{ + Error: nil, + Score: 6, + NumberOfWarn: 4, + NumberOfInfo: 6, + NumberOfDebug: 0, + }, + branch: &clients.BranchRef{ + Name: &branchVal, + Protected: &trueVal, + BranchProtectionRule: clients.BranchProtectionRule{ + EnforceAdmins: &trueVal, + RequireLastPushApproval: &trueVal, + RequireLinearHistory: &trueVal, + AllowForcePushes: &falseVal, + AllowDeletions: &falseVal, + CheckRules: clients.StatusChecksRule{ + RequiresStatusChecks: &falseVal, + UpToDateBeforeMerge: &trueVal, + Contexts: nil, + }, + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + DismissStaleReviews: &falseVal, + RequireCodeOwnerReviews: &falseVal, + RequiredApprovingReviewCount: &oneVal, + }, + }, + }, + }, + { + name: "Non-admin run on project that doesn't require 1 reviewer", + expected: scut.TestReturn{ + Error: nil, + Score: 3, + NumberOfWarn: 3, + NumberOfInfo: 4, + NumberOfDebug: 2, + }, + branch: &clients.BranchRef{ + Name: &branchVal, + Protected: &trueVal, + BranchProtectionRule: clients.BranchProtectionRule{ + EnforceAdmins: &trueVal, + RequireLastPushApproval: nil, + RequireLinearHistory: &trueVal, + AllowForcePushes: &falseVal, + AllowDeletions: &falseVal, + CheckRules: clients.StatusChecksRule{ + RequiresStatusChecks: &falseVal, + UpToDateBeforeMerge: nil, + Contexts: []string{"foo"}, + }, + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + DismissStaleReviews: &falseVal, + RequireCodeOwnerReviews: &falseVal, + RequiredApprovingReviewCount: nil, + }, + }, + }, + }, + { + name: "Non-admin run on project with all tier 2 requirements", + expected: scut.TestReturn{ + Error: nil, + Score: 6, + NumberOfWarn: 4, + NumberOfInfo: 3, + NumberOfDebug: 2, + }, + branch: &clients.BranchRef{ + Name: &branchVal, + Protected: &trueVal, + BranchProtectionRule: clients.BranchProtectionRule{ + EnforceAdmins: &trueVal, + RequireLastPushApproval: nil, + RequireLinearHistory: &trueVal, + AllowForcePushes: &falseVal, + AllowDeletions: &falseVal, + CheckRules: clients.StatusChecksRule{ + RequiresStatusChecks: &falseVal, + UpToDateBeforeMerge: nil, + Contexts: nil, + }, RequiredPullRequestReviews: clients.PullRequestReviewRule{ DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -193,7 +317,7 @@ func TestIsBranchProtected(t *testing.T) { Error: nil, Score: 3, NumberOfWarn: 5, - NumberOfInfo: 4, + NumberOfInfo: 5, NumberOfDebug: 0, }, branch: &clients.BranchRef{ @@ -224,7 +348,7 @@ func TestIsBranchProtected(t *testing.T) { Error: nil, Score: 2, NumberOfWarn: 6, - NumberOfInfo: 3, + NumberOfInfo: 4, NumberOfDebug: 0, }, branch: &clients.BranchRef{ @@ -255,7 +379,7 @@ func TestIsBranchProtected(t *testing.T) { Error: nil, Score: 1, NumberOfWarn: 7, - NumberOfInfo: 2, + NumberOfInfo: 3, NumberOfDebug: 0, }, branch: &clients.BranchRef{ @@ -287,7 +411,7 @@ func TestIsBranchProtected(t *testing.T) { Error: nil, Score: 1, NumberOfWarn: 7, - NumberOfInfo: 2, + NumberOfInfo: 3, NumberOfDebug: 0, }, branch: &clients.BranchRef{ @@ -318,7 +442,7 @@ func TestIsBranchProtected(t *testing.T) { Error: nil, Score: 8, NumberOfWarn: 2, - NumberOfInfo: 8, + NumberOfInfo: 9, NumberOfDebug: 0, }, branch: &clients.BranchRef{ @@ -349,7 +473,7 @@ func TestIsBranchProtected(t *testing.T) { Error: nil, Score: 8, NumberOfWarn: 1, - NumberOfInfo: 8, + NumberOfInfo: 9, NumberOfDebug: 0, }, branch: &clients.BranchRef{ @@ -381,7 +505,7 @@ func TestIsBranchProtected(t *testing.T) { Error: nil, Score: 5, NumberOfWarn: 3, - NumberOfInfo: 7, + NumberOfInfo: 8, NumberOfDebug: 0, }, branch: &clients.BranchRef{