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 <[email protected]>
ossf · Sep 20, 2023 · 7a2a8df · 7a2a8df
1 parent 339594f
commit 7a2a8df
Show file tree

Hide file tree

Showing 2 changed files with 146 additions and 22 deletions.
diff --git 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
@@ -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{