Fix Golang lint errors.

This should fix most of the lint errors in the repo. I'm ignoring a few
files that don't get touched often or don't have obvious fixes (mainly
around imports).

cmd/state_comparer/main.go

@@ -98,7 +98,7 @@ func gatherOptions() options {
 var tgDupRegEx = regexp.MustCompile(` \<TESTGRID:\d+\>`)
 var dupRegEx = regexp.MustCompile(` \[\d+\]`)
 
-func rowsAndColumns(ctx context.Context, grid *statepb.Grid, numColumnsRecent int32) (map[string]int, map[string]int, map[string]int) {
+func rowsAndColumns(grid *statepb.Grid, numColumnsRecent int32) (map[string]int, map[string]int, map[string]int) {
 	rows := make(map[string]int)
 	issues := make(map[string]int)
 	for _, row := range grid.GetRows() {
@@ -214,10 +214,10 @@ func reportDiff(first, second map[string]int, identifier string, diffRatioOK flo
 	return
 }
 
-func compare(ctx context.Context, first, second *statepb.Grid, diffRatioOK float64, numColumnsRecent int32) (diffed bool, rowDiffReasons, colDiffReasons diffReasons) {
+func compare(first, second *statepb.Grid, diffRatioOK float64, numColumnsRecent int32) (diffed bool, rowDiffReasons, colDiffReasons diffReasons) {
 	logrus.Tracef("*****************************")
-	firstRows, firstColumns, _ := rowsAndColumns(ctx, first, numColumnsRecent)
-	secondRows, secondColumns, _ := rowsAndColumns(ctx, second, numColumnsRecent)
+	firstRows, firstColumns, _ := rowsAndColumns(first, numColumnsRecent)
+	secondRows, secondColumns, _ := rowsAndColumns(second, numColumnsRecent)
 	// both grids have no results, ignore
 	if (len(firstRows) == 0 && len(secondRows) == 0) || (len(firstColumns) == 0 && len(secondColumns) == 0) {
 		return
@@ -336,7 +336,7 @@ func main() {
 			continue
 		}
 
-		if diffed, rowReasons, colReasons := compare(ctx, firstGrid, secondGrid, opt.diffRatioOK, tg.GetNumColumnsRecent()); diffed {
+		if diffed, rowReasons, colReasons := compare(firstGrid, secondGrid, opt.diffRatioOK, tg.GetNumColumnsRecent()); diffed {
 			msg := fmt.Sprintf("%q vs. %q", firstP, secondP)
 			if opt.testGroupURL != "" {
 				parts := strings.Split(firstP, "/")

cmd/state_comparer/main_test.go

@@ -21,7 +21,6 @@ limitations under the License.
 package main
 
 import (
-	"context"
 	"strings"
 	"testing"
 
@@ -354,8 +353,7 @@ func TestCompare(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			ctx := context.Background()
-			if diffed, _, _ := compare(ctx, tc.first, tc.second, tc.diffRatioOK, 5); diffed != tc.diffed {
+			if diffed, _, _ := compare(tc.first, tc.second, tc.diffRatioOK, 5); diffed != tc.diffed {
 				t.Errorf("compare(%s, %s) not as expected; got %t, want %t", tc.first, tc.second, diffed, tc.diffed)
 			}
 		})

config/BUILD.bazel

@@ -59,6 +59,7 @@ go_test(
         "//util/gcs/fake:go_default_library",
         "@com_github_golang_protobuf//proto:go_default_library",
         "@com_github_google_go_cmp//cmp:go_default_library",
+        "@com_github_google_go_cmp//cmp/cmpopts:go_default_library",
         "@com_github_hashicorp_go_multierror//:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",
         "@com_google_cloud_go_storage//:go_default_library",

config/config.go

@@ -68,6 +68,13 @@ type ValidationError struct {
 	Message string
 }
 
+// MissingConfigError is an error for missing configuration.
+type MissingConfigError struct{}
+
+func (e MissingConfigError) Error() string {
+	return "got an empty config.Configuration"
+}
+
 func (e ValidationError) Error() string {
 	return fmt.Sprintf("configuration error for (%s) %s: %s", e.Entity, e.Name, e.Message)
 }
@@ -103,7 +110,7 @@ func validateUnique(items []string, entity string) error {
 func validateAllUnique(c *configpb.Configuration) error {
 	var mErr error
 	if c == nil {
-		return multierror.Append(mErr, errors.New("got an empty config.Configuration"))
+		return multierror.Append(mErr, MissingConfigError{})
 	}
 	var tgNames []string
 	for _, tg := range c.GetTestGroups() {
@@ -163,7 +170,7 @@ func validateAllUnique(c *configpb.Configuration) error {
 func validateReferencesExist(c *configpb.Configuration) error {
 	var mErr error
 	if c == nil {
-		return multierror.Append(mErr, errors.New("got an empty config.Configuration"))
+		return multierror.Append(mErr, MissingConfigError{})
 	}
 
 	tgNames := map[string]bool{}
@@ -378,7 +385,7 @@ func validateTestGroup(tg *configpb.TestGroup) error {
 		} else if cv != "" && (p != "" || l != "") || p != "" && (cv != "" || l != "") {
 			mErr = multierror.Append(
 				mErr,
-				fmt.Errorf("Column Header %d must only set one value, got configuration_value: %q, property: %q, label: %q", idx, cv, p, l),
+				fmt.Errorf("column header %d must only set one value, got configuration_value: %q, property: %q, label: %q", idx, cv, p, l),
 			)
 		}
 
@@ -473,7 +480,7 @@ func validateDashboardTab(dt *configpb.DashboardTab) error {
 func validateEntityConfigs(c *configpb.Configuration) error {
 	var mErr error
 	if c == nil {
-		return multierror.Append(mErr, errors.New("got an empty config.Configuration"))
+		return multierror.Append(mErr, MissingConfigError{})
 	}
 
 	// At the moment, don't need to further validate Dashboards or DashboardGroups.
@@ -498,7 +505,7 @@ func validateEntityConfigs(c *configpb.Configuration) error {
 func Validate(c *configpb.Configuration) error {
 	var mErr error
 	if c == nil {
-		return multierror.Append(mErr, errors.New("got an empty config.Configuration"))
+		return multierror.Append(mErr, MissingConfigError{})
 	}
 
 	// TestGrid requires at least 1 TestGroup and 1 Dashboard in order to do anything.
@@ -553,7 +560,7 @@ func Unmarshal(r io.Reader) (*configpb.Configuration, error) {
 // Returns an error if config is invalid or writing failed.
 func MarshalText(c *configpb.Configuration, w io.Writer) error {
 	if c == nil {
-		return errors.New("got an empty config.Configuration")
+		return MissingConfigError{}
 	}
 	if err := Validate(c); err != nil {
 		return err
@@ -565,7 +572,7 @@ func MarshalText(c *configpb.Configuration, w io.Writer) error {
 // Returns an error if config is invalid or encoding failed.
 func MarshalBytes(c *configpb.Configuration) ([]byte, error) {
 	if c == nil {
-		return nil, errors.New("got an empty config.Configuration")
+		return nil, MissingConfigError{}
 	}
 	if err := Validate(c); err != nil {
 		return nil, err

config/config_test.go

@@ -17,12 +17,12 @@ limitations under the License.
 package config
 
 import (
-	"errors"
-	"reflect"
 	"strings"
 	"testing"
 
 	configpb "github.com/GoogleCloudPlatform/testgrid/pb/config"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	multierror "github.com/hashicorp/go-multierror"
 )
 
@@ -97,8 +97,8 @@ func TestValidateUnique(t *testing.T) {
 				}
 
 				if mErr, ok := err.(*multierror.Error); ok {
-					if !reflect.DeepEqual(test.expectedErrs, mErr.Errors) {
-						t.Fatalf("Expected %v, but got: %v", test.expectedErrs, mErr.Errors)
+					if diff := cmp.Diff(test.expectedErrs, mErr.Errors, cmpopts.EquateErrors()); diff != "" {
+						t.Fatalf("Errors differed (-want, +got): %s", diff)
 					}
 				} else {
 					t.Fatalf("Expected %v, but got: %v", test.expectedErrs, err)
@@ -224,7 +224,7 @@ func TestValidateReferencesExist(t *testing.T) {
 		{
 			name:         "reject nil config.Configuration",
 			input:        nil,
-			expectedErrs: []error{errors.New("got an empty config.Configuration")},
+			expectedErrs: []error{MissingConfigError{}},
 		},
 		{
 			name: "Dashboard Tabs must reference an existing Test Group",
@@ -353,8 +353,8 @@ func TestValidateReferencesExist(t *testing.T) {
 				}
 
 				if mErr, ok := err.(*multierror.Error); ok {
-					if !reflect.DeepEqual(test.expectedErrs, mErr.Errors) {
-						t.Fatalf("Expected %v, but got: %v", test.expectedErrs, mErr.Errors)
+					if diff := cmp.Diff(test.expectedErrs, mErr.Errors, cmpopts.EquateErrors()); diff != "" {
+						t.Fatalf("Errors differed (-want, +got): %s", diff)
 					}
 				} else {
 					t.Fatalf("Expected %v, but got: %v", test.expectedErrs, err)
@@ -1261,7 +1261,7 @@ func TestUpdate_Validate(t *testing.T) {
 		{
 			name:         "Nil input; returns error",
 			input:        nil,
-			expectedErrs: []error{errors.New("got an empty config.Configuration")},
+			expectedErrs: []error{MissingConfigError{}},
 		},
 		{
 			name: "Dashboard Only; returns error",
@@ -1525,8 +1525,8 @@ func TestUpdate_Validate(t *testing.T) {
 				}
 
 				if mErr, ok := err.(*multierror.Error); ok {
-					if !reflect.DeepEqual(test.expectedErrs, mErr.Errors) {
-						t.Fatalf("Expected %v, but got: %v", test.expectedErrs, mErr.Errors)
+					if diff := cmp.Diff(test.expectedErrs, mErr.Errors); diff != "" {
+						t.Fatalf("Errors differed (-want, +got): %s", diff)
 					}
 				} else {
 					t.Fatalf("Expected %v, but got: %v", test.expectedErrs, err)

config/converge.go

@@ -38,7 +38,7 @@ var insert void
 // The output protobuf will pass config.Validate if all its inputs pass config.Validate
 func Converge(shards map[string]*configpb.Configuration) (*configpb.Configuration, error) {
 	if len(shards) == 0 {
-		return nil, fmt.Errorf("No configurations to converge")
+		return nil, fmt.Errorf("no configurations to converge")
 	}
 
 	result := configpb.Configuration{}
@@ -111,7 +111,7 @@ func negotiateConversions(prefix string, original, new map[string]void) map[stri
 		if _, exists := original[key]; exists {
 			candidate := fmt.Sprintf("%s-%s", prefix, key)
 			attempt := 1
-			for true {
+			for {
 				_, existInOld := original[candidate]
 				_, existInNew := new[candidate]
 				if !existInOld && !existInNew {

config/fields_test.go

@@ -140,10 +140,10 @@ func TestFields(t *testing.T) {
 									BetaAutobugComponent: 1,
 									HotlistIdsFromSource: []*configpb.HotlistIdFromSource{
 										{
-											HotlistIdSource: &configpb.HotlistIdFromSource_Label{"foo"},
+											HotlistIdSource: &configpb.HotlistIdFromSource_Label{Label: "foo"},
 										},
 										{
-											HotlistIdSource: &configpb.HotlistIdFromSource_Value{1},
+											HotlistIdSource: &configpb.HotlistIdFromSource_Value{Value: 1},
 										},
 									},
 								},
@@ -157,10 +157,10 @@ func TestFields(t *testing.T) {
 							BetaAutobugComponent: 1,
 							HotlistIdsFromSource: []*configpb.HotlistIdFromSource{
 								{
-									HotlistIdSource: &configpb.HotlistIdFromSource_Value{2},
+									HotlistIdSource: &configpb.HotlistIdFromSource_Value{Value: 2},
 								},
 								{
-									HotlistIdSource: &configpb.HotlistIdFromSource_Value{3},
+									HotlistIdSource: &configpb.HotlistIdFromSource_Value{Value: 3},
 								},
 							},
 						},

config/yamlcfg/BUILD.bazel

@@ -19,6 +19,7 @@ go_test(
     deps = [
         "//pb/config:go_default_library",
         "@com_github_google_go_cmp//cmp:go_default_library",
+        "@org_golang_google_protobuf//testing/protocmp:go_default_library",
     ],
 )
 

config/yamlcfg/yaml2proto.go

@@ -88,31 +88,29 @@ func pathDefault(path string, defaultFiles map[string]DefaultConfiguration, defa
 // 		 after applying defaults from defaultPath.
 // Optionally, defaultPath points to default setting YAML
 // Returns a configuration proto containing the data from all of those sources
-func ReadConfig(paths []string, defaultpath string, strict bool) (config.Configuration, error) {
-
-	var result config.Configuration
-
+func ReadConfig(paths []string, defaultpath string, strict bool) (*config.Configuration, error) {
 	// Get the overall default file, if specified.
 	var defaults DefaultConfiguration
 	if defaultpath != "" {
 		b, err := ioutil.ReadFile(defaultpath)
 		if err != nil {
-			return result, fmt.Errorf("failed to read default at %s: %v", defaultpath, err)
+			return nil, fmt.Errorf("failed to read default at %s: %v", defaultpath, err)
 		}
 		defaults, err = LoadDefaults(b)
 		if err != nil {
-			return result, fmt.Errorf("failed to deserialize default at %s: %v", defaultpath, err)
+			return nil, fmt.Errorf("failed to deserialize default at %s: %v", defaultpath, err)
 		}
 	}
 
 	// Find all default files, map their directory to their contents.
 	defaultFiles, err := seekDefaults(paths)
 	if err != nil {
-		return result, err
+		return nil, err
 	}
 
 	// Gather configuration from each YAML file, applying the config's default.yaml if
 	// one exists in its directory, or the overall default otherwise.
+	var result *config.Configuration
 	err = SeekYAMLFiles(paths, func(path string, info os.FileInfo) error {
 		if filepath.Base(path) == "default.yaml" || filepath.Base(path) == "default.yml" {
 			return nil
@@ -123,30 +121,29 @@ func ReadConfig(paths []string, defaultpath string, strict bool) (config.Configu
 			return fmt.Errorf("failed to read %s: %v", path, err)
 		}
 		localDefaults := pathDefault(path, defaultFiles, defaults)
-		if err = Update(&result, b, &localDefaults, strict); err != nil {
+		if result, err = Update(result, b, &localDefaults, strict); err != nil {
 			return fmt.Errorf("failed to merge %s into config: %v", path, err)
 		}
 		return nil
 	})
 	if err != nil {
-		return result, fmt.Errorf("SeekYAMLFiles(%v), gathering config: %v", paths, err)
+		return nil, fmt.Errorf("SeekYAMLFiles(%v), gathering config: %v", paths, err)
 	}
 
 	return result, err
 }
 
 // Update reads the config in yamlData and updates the config in c.
 // If reconcile is non-nil, it will pad out new entries with those default settings
-func Update(cfg *config.Configuration, yamlData []byte, reconcile *DefaultConfiguration, strict bool) error {
-
+func Update(cfg *config.Configuration, yamlData []byte, reconcile *DefaultConfiguration, strict bool) (*config.Configuration, error) {
 	newConfig := &config.Configuration{}
 	if strict {
 		if err := yaml.UnmarshalStrict(yamlData, newConfig); err != nil {
-			return err
+			return nil, err
 		}
 	} else {
 		if err := yaml.Unmarshal(yamlData, newConfig); err != nil {
-			return err
+			return nil, err
 		}
 	}
 
@@ -174,7 +171,7 @@ func Update(cfg *config.Configuration, yamlData []byte, reconcile *DefaultConfig
 		cfg.DashboardGroups = append(cfg.DashboardGroups, dashboardGroup)
 	}
 
-	return nil
+	return cfg, nil
 }
 
 // MarshalYAML returns a YAML file representing the parsed configuration.