internal/buildbinary: collect all binaries with their errors

Rather than failing to collect any binaries even if only one binary
creation fails, collect all binaries and remember the ones that error.
This will increase coverage for the compare mode.

Also does some minor refactoring.

Change-Id: I22e44b752404f3acacebdd7d485caee8879dc6f9
Reviewed-on: https://go-review.googlesource.com/c/pkgsite-metrics/+/520715
Reviewed-by: Maceo Thompson <maceothompson@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/cmd/govulncheck_compare/govulncheck_compare.go b/cmd/govulncheck_compare/govulncheck_compare.go
index 7660590..4d51e2f 100644
--- a/cmd/govulncheck_compare/govulncheck_compare.go
+++ b/cmd/govulncheck_compare/govulncheck_compare.go
@@ -58,10 +58,16 @@
 	defer removeBinaries(binaries)
 
 	for _, binary := range binaries {
-		pair := govulncheck.ComparePair{
+		pair := &govulncheck.ComparePair{
 			BinaryResults: govulncheck.SandboxResponse{Stats: govulncheck.ScanStats{BuildTime: binary.BuildTime}},
 			SourceResults: govulncheck.SandboxResponse{Stats: govulncheck.ScanStats{}},
 		}
+		response.FindingsForMod[binary.ImportPath] = pair
+
+		if binary.Error != nil {
+			pair.Error = binary.Error
+			continue // there was an error in building the binary
+		}
 
 		pair.SourceResults.Findings, err = govulncheck.RunGovulncheckCmd(govulncheckPath, govulncheck.FlagSource, binary.ImportPath, modulePath, vulndbPath, &pair.SourceResults.Stats)
 		if err != nil {
@@ -74,8 +80,6 @@
 			fail(err)
 			return
 		}
-
-		response.FindingsForMod[binary.ImportPath] = &pair
 	}
 
 	b, err := json.MarshalIndent(response, "", "\t")
diff --git a/internal/buildbinary/bin.go b/internal/buildbinary/bin.go
index b21d9af..17ea3d0 100644
--- a/internal/buildbinary/bin.go
+++ b/internal/buildbinary/bin.go
@@ -18,6 +18,7 @@
 	BinaryPath string
 	ImportPath string
 	BuildTime  time.Duration
+	Error      error
 }
 
 // FindAndBuildBinaries finds and builds all possible binaries from a given module.
@@ -30,14 +31,15 @@
 
 	for i, target := range buildTargets {
 		path, buildTime, err := runBuild(modulePath, target, i)
-		if err != nil {
-			return nil, err
-		}
-		binaries = append(binaries, &BinaryInfo{
+		b := &BinaryInfo{
 			BinaryPath: path,
 			ImportPath: target,
 			BuildTime:  buildTime,
-		})
+		}
+		if err != nil {
+			b.Error = err
+		}
+		binaries = append(binaries, b)
 	}
 	return binaries, nil
 }
diff --git a/internal/buildbinary/bin_test.go b/internal/buildbinary/bin_test.go
index 1249755..b79f739 100644
--- a/internal/buildbinary/bin_test.go
+++ b/internal/buildbinary/bin_test.go
@@ -40,13 +40,17 @@
 			want:    []string{"example.com/test", "example.com/test/multipleBinModule", "example.com/test/p1"},
 			wantErr: false,
 		},
+		{
+			name:    "error test",
+			dir:     "non-existing-module",
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := findBinaries(tt.dir)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("findBinaries() error = %v, wantErr %v", err, tt.wantErr)
-				return
+				t.Fatalf("got error=%v, wantErr=%v", err, tt.wantErr)
 			}
 
 			if diff := cmp.Diff(tt.want, got, cmpopts.SortSlices(less)); diff != "" {
@@ -86,8 +90,7 @@
 			got, _, err := runBuild(tt.modulePath, tt.importPath, 1)
 			defer os.Remove(got)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("runBuild() error = %v, wantErr %v", err, tt.wantErr)
-				return
+				t.Fatalf("got error=%v; wantErr=%v", err, tt.wantErr)
 			}
 
 			if diff := cmp.Diff(tt.want, got); diff != "" {
@@ -95,7 +98,7 @@
 			}
 			_, err = os.Stat(got)
 			if err != nil && os.IsNotExist(err) {
-				t.Errorf("runBuild did not produce the expected binary")
+				t.Errorf("did not produce the expected binary")
 			}
 		})
 	}
diff --git a/internal/govulncheck/govulncheck.go b/internal/govulncheck/govulncheck.go
index cecd547..2d12126 100644
--- a/internal/govulncheck/govulncheck.go
+++ b/internal/govulncheck/govulncheck.go
@@ -288,6 +288,7 @@
 type ComparePair struct {
 	BinaryResults SandboxResponse
 	SourceResults SandboxResponse
+	Error         error
 }
 
 func UnmarshalCompareResponse(output []byte) (*CompareResponse, error) {
diff --git a/internal/worker/govulncheck_scan.go b/internal/worker/govulncheck_scan.go
index c22bca2..dd16d14 100644
--- a/internal/worker/govulncheck_scan.go
+++ b/internal/worker/govulncheck_scan.go
@@ -230,6 +230,13 @@
 	}
 	log.Infof(ctx, "scanner.runGovulncheckCompare found %d compilable binaries in %s:", len(response.FindingsForMod), sreq.Path())
 	for pkg, results := range response.FindingsForMod {
+		if results.Error != nil {
+			// Just log error if binary failed to build. Otherwise, we'd have
+			// to create bq rows for both binary and source compare modes.
+			log.Errorf(ctx, err, "building binary failed: %s %s", pkg, sreq.Path())
+			continue
+		}
+
 		binRow := createComparisonRow(pkg, &results.BinaryResults, baseRow, ModeBinary)
 		srcRow := createComparisonRow(pkg, &results.SourceResults, baseRow, ModeGovulncheck)