fixes

ossf · Feb 2, 2022 · 17577c9 · 17577c9
1 parent 5891b25
commit 17577c9
Show file tree

Hide file tree

Showing 6 changed files with 257 additions and 268 deletions.
diff --git a/checker/raw_result.go b/checker/raw_result.go
@@ -114,11 +114,6 @@ type Issue struct {
 // DefaultBranchCommit represents a commit
 // to the default branch.
 type DefaultBranchCommit struct {
-	// Review indicate whether the commit was reviewed,
-	// using scorecard's logic.
-	// Note: this field may disppear if we decide the purely "raw"
-	// results are enough.
-	ApprovedReviews *ApprovedReviews
 	// Fields below are taken directly from cloud
 	// version control systems, e.g. GitHub.
 	SHA           string
@@ -135,43 +130,18 @@ type MergeRequest struct {
 	Reviews []Review
 }
 
-// User represent a user.
-type User struct {
-	Login string
-}
-
-var (
-	// ReviewPlatformGitHub is the name of ReviewPlatform for GitHub.
-	ReviewPlatformGitHub = "GitHub"
-	// ReviewPlatformProw is the name of ReviewPlatform for Prow.
-	ReviewPlatformProw = "Prow"
-	// ReviewPlatformGerrit is the name of ReviewPlatform for Gerrit.
-	ReviewPlatformGerrit = "Gerrit"
-	// ReviewStateApproved means an approved review.
-	ReviewStateApproved = "approved"
-)
-
-// ReviewPlatform represents a review platform.
-type ReviewPlatform struct {
-	Name string
-	// TODO: add fields, e.g. config files, etc.
-}
-
-// ApprovedReviews represents the LGTMs associated with a commit
-// to the default branch.
-type ApprovedReviews struct {
-	Platform ReviewPlatform
-	// Note: this field is only populated for GitHub and Prow.
-	MaintainerReviews []Review
-}
-
-// Review represent a single-maintainer's review.
+// Review represent a review using the built-in review system.
 type Review struct {
 	Reviewer User
 	State    string
 	// TODO(Review): add fields here if needed.
 }
 
+// User represent a user.
+type User struct {
+	Login string
+}
+
 // File represents a file.
 type File struct {
 	Path    string

diff --git a/checks/code_review_test.go b/checks/code_review_test.go
@@ -69,7 +69,7 @@ func TestCodereview(t *testing.T) {
 			},
 		},
 		{
-			name: "Valid PR's and commits as not a bot",
+			name: "Valid GitHub PR",
 			prs: []clients.PullRequest{
 				{
 					Number:   1,
@@ -79,11 +79,39 @@ func TestCodereview(t *testing.T) {
 							State: "APPROVED",
 						},
 					},
+					MergeCommit: clients.Commit{
+						SHA: "sha",
+					},
+				},
+			},
+			commits: []clients.Commit{
+				{
+					SHA: "sha",
+					Committer: clients.User{
+						Login: "user",
+					},
+				},
+			},
+
+			expected: checker.CheckResult{
+				Score: 10,
+				Pass:  true,
+			},
+		},
+		{
+			name: "Valid Prow PR as not a bot",
+			prs: []clients.PullRequest{
+				{
+					Number:   1,
+					MergedAt: time.Now(),
 					Labels: []clients.Label{
 						{
 							Name: "lgtm",
 						},
 					},
+					MergeCommit: clients.Commit{
+						SHA: "sha",
+					},
 				},
 			},
 			commits: []clients.Commit{
@@ -100,23 +128,20 @@ func TestCodereview(t *testing.T) {
 				Pass:  true,
 			},
 		},
-
 		{
-			name: "Valid PR's and commits as bot",
+			name: "Valid Prow PR and commits as bot",
 			prs: []clients.PullRequest{
 				{
 					Number:   1,
 					MergedAt: time.Now(),
-					Reviews: []clients.Review{
-						{
-							State: "APPROVED",
-						},
-					},
 					Labels: []clients.Label{
 						{
 							Name: "lgtm",
 						},
 					},
+					MergeCommit: clients.Commit{
+						SHA: "sha",
+					},
 				},
 			},
 			commits: []clients.Commit{
@@ -133,56 +158,52 @@ func TestCodereview(t *testing.T) {
 				Pass:  true,
 			},
 		},
-
 		{
-			name: "Valid PR's and commits not yet merged",
+			name: "Valid PR's and commits with merged by someone else",
 			prs: []clients.PullRequest{
 				{
 					Number: 1,
-					Reviews: []clients.Review{
-						{
-							State: "APPROVED",
-						},
-					},
 					Labels: []clients.Label{
 						{
 							Name: "lgtm",
 						},
 					},
+					MergeCommit: clients.Commit{
+						SHA: "sha",
+						Committer: clients.User{
+							Login: "bot",
+						},
+					},
 				},
 			},
 			commits: []clients.Commit{
 				{
 					SHA: "sha",
 					Committer: clients.User{
-						Login: "bot",
+						Login: "bob",
 					},
 				},
 			},
 
 			expected: checker.CheckResult{
-				Score: -1,
+				Score: 0,
 			},
 		},
 		{
-			name: "Valid PR's and commits with merged by someone else",
+			name: "2 PRs 2 review on GitHub",
 			prs: []clients.PullRequest{
 				{
-					Number: 1,
+					Number:   1,
+					MergedAt: time.Now(),
 					Reviews: []clients.Review{
 						{
 							State: "APPROVED",
 						},
 					},
-					Labels: []clients.Label{
-						{
-							Name: "lgtm",
-						},
-					},
 					MergeCommit: clients.Commit{
 						SHA: "sha",
 						Committer: clients.User{
-							Login: "bob",
+							Login: "bot",
 						},
 					},
 				},
@@ -191,13 +212,19 @@ func TestCodereview(t *testing.T) {
 				{
 					SHA: "sha",
 					Committer: clients.User{
-						Login: "bot",
+						Login: "bob",
+					},
+				},
+				{
+					SHA: "sha2",
+					Committer: clients.User{
+						Login: "bob",
 					},
 				},
 			},
 
 			expected: checker.CheckResult{
-				Score: -1,
+				Score: 5,
 			},
 		},
 	}