Require test coverage to merge.

.github/workflows/robot/internal/bot/check.go

@@ -18,6 +18,9 @@ package bot
 
 import (
 	"context"
+	"strings"
+
+	"github.com/gravitational/teleport/.github/workflows/robot/internal/github"
 
 	"github.com/gravitational/trace"
 )
@@ -50,14 +53,69 @@ func (b *Bot) Check(ctx context.Context) error {
 			return trace.Wrap(err)
 		}
 
+		// Check if PR has received required approvals.
 		if err := b.c.Review.CheckInternal(b.c.Environment.Author, reviews, docs, code); err != nil {
 			return trace.Wrap(err)
 		}
+
+		// Check if PR has test coverage or has admin approval to bypass.
+		if err := b.checkTests(ctx, b.c.Environment.Author, reviews); err != nil {
+			return trace.Wrap(err)
+		}
+
+		return nil
+	}
+
+	// PRs from external authors require two admin approvals to merge.
+	if err := b.c.Review.CheckAdmin(b.c.Environment.Author, reviews, 2); err != nil {
+		return trace.Wrap(err)
+	}
+
+	return nil
+}
+
+func (b *Bot) checkTests(ctx context.Context, author string, reviews map[string]*github.Review) error {
+	// If an admin has approved, bypass the test coverage check.
+	if err := b.c.Review.CheckAdmin(author, reviews, 1); err == nil {
 		return nil
 	}
 
-	if err := b.c.Review.CheckExternal(b.c.Environment.Author, reviews); err != nil {
+	if err := b.hasTestCoverage(ctx); err != nil {
+		return trace.Wrap(err)
+	}
+
+	return nil
+}
+
+func (b *Bot) hasTestCoverage(ctx context.Context) error {
+	files, err := b.c.GitHub.ListFiles(ctx,
+		b.c.Environment.Organization,
+		b.c.Environment.Repository,
+		b.c.Environment.Number)
+	if err != nil {
 		return trace.Wrap(err)
 	}
+
+	var code bool
+	var tests bool
+
+	for _, file := range files {
+		// Remove after "branch/v7" and "branch/v8" go out of support.
+		if strings.HasPrefix(file, "vendor/") {
+			continue
+		}
+
+		switch {
+		case strings.HasSuffix(file, "_test.go"):
+			tests = true
+		case strings.HasSuffix(file, ".go"):
+			code = true
+		}
+	}
+
+	// Fail if code was added without test coverage.
+	if code && !tests {
+		return trace.BadParameter("missing test coverage, add test coverage or request admin override")
+	}
 	return nil
 }

.github/workflows/robot/internal/bot/check_test.go

@@ -0,0 +1,129 @@
+/*
+Copyright 2022 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package bot
+
+import (
+	"context"
+	"testing"
+
+	"github.com/gravitational/teleport/.github/workflows/robot/internal/env"
+	"github.com/gravitational/teleport/.github/workflows/robot/internal/github"
+	"github.com/gravitational/teleport/.github/workflows/robot/internal/review"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestCheck(t *testing.T) {
+	r, err := review.New(&review.Config{
+		// Code.
+		CodeReviewers: map[string]review.Reviewer{
+			"1": review.Reviewer{Team: "Core", Owner: true},
+			"2": review.Reviewer{Team: "Core", Owner: true},
+			"3": review.Reviewer{Team: "Core", Owner: false},
+			"4": review.Reviewer{Team: "Core", Owner: false},
+			"5": review.Reviewer{Team: "Core", Owner: false},
+		},
+		CodeReviewersOmit: map[string]bool{},
+		DocsReviewers:     map[string]review.Reviewer{},
+		DocsReviewersOmit: map[string]bool{},
+		// Admins.
+		Admins: []string{
+			"6",
+			"7",
+		},
+	})
+	require.NoError(t, err)
+
+	tests := []struct {
+		desc    string
+		author  string
+		files   []string
+		reviews map[string]*github.Review
+		result  bool
+	}{
+		{
+			desc:   "no-approvals-fails",
+			author: "3",
+			files: []string{
+				"file.go",
+			},
+			reviews: map[string]*github.Review{},
+			result:  false,
+		},
+		{
+			desc:   "approvals-without-tests-fails",
+			author: "3",
+			files: []string{
+				"file.go",
+			},
+			reviews: map[string]*github.Review{
+				"1": &github.Review{Author: "1", State: "APPROVED"},
+				"4": &github.Review{Author: "4", State: "APPROVED"},
+			},
+			result: false,
+		},
+		{
+			desc:   "approvals-without-tests-requires-admin-success",
+			author: "3",
+			files: []string{
+				"file.go",
+			},
+			reviews: map[string]*github.Review{
+				"1": &github.Review{Author: "1", State: "APPROVED"},
+				"4": &github.Review{Author: "4", State: "APPROVED"},
+				"6": &github.Review{Author: "6", State: "APPROVED"},
+			},
+			result: true,
+		},
+		{
+			desc:   "approvals-and-tests-success",
+			author: "3",
+			files: []string{
+				"file.go",
+				"file_test.go",
+			},
+			reviews: map[string]*github.Review{
+				"1": &github.Review{Author: "1", State: "APPROVED"},
+				"4": &github.Review{Author: "4", State: "APPROVED"},
+			},
+			result: true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			b := &Bot{
+				c: &Config{
+					Environment: &env.Environment{
+						Organization: "foo",
+						Repository:   "bar",
+						Number:       0,
+					},
+					GitHub: &fakeGithub{
+						test.files,
+					},
+					Review: r,
+				},
+			}
+			err := b.checkTests(context.Background(), test.author, test.reviews)
+			if test.result {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+			}
+		})
+	}
+}

.github/workflows/robot/internal/review/review.go

@@ -195,15 +195,16 @@ func (r *Assignments) getCodeReviewerSets(author string) ([]string, []string) {
 }
 
 // CheckExternal requires two admins have approved.
-func (r *Assignments) CheckExternal(author string, reviews map[string]*github.Review) error {
-	log.Printf("Check: Found external author %v.", author)
+func (r *Assignments) CheckAdmin(author string, reviews map[string]*github.Review, count int) error {
+	log.Printf("Check: Checking for %v admin approvals for %v.", count, author)
 
 	reviewers := r.getAdminReviewers(author)
 
-	if checkN(reviewers, reviews) > 1 {
+	n := checkN(reviewers, reviews)
+	if n >= count {
 		return nil
 	}
-	return trace.BadParameter("at least two approvals required from %v", reviewers)
+	return trace.BadParameter("at least %v approvals required from %v, only have %v", count, reviewers, n)
 }
 
 // CheckInternal will verify if required reviewers have approved. Checks if

.github/workflows/robot/internal/review/review_test.go

@@ -359,8 +359,8 @@ func TestGetDocsReviewers(t *testing.T) {
 	}
 }
 
-// TestCheckExternal checks external reviews.
-func TestCheckExternal(t *testing.T) {
+// TestCheckAdmin checks external reviews.
+func TestCheckAdmin(t *testing.T) {
 	r := &Assignments{
 		c: &Config{
 			// Code.
@@ -455,7 +455,7 @@ func TestCheckExternal(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			err := r.CheckExternal(test.author, test.reviews)
+			err := r.CheckAdmin(test.author, test.reviews, 2)
 			if test.result {
 				require.NoError(t, err)
 			} else {