Merge pull request #844 from grepory/dep-ensure-errors

Better error reporting from dep ensure when packages have errors

cmd/dep/ensure.go

@@ -704,27 +704,28 @@ func getProjectConstraint(arg string, sm gps.SourceManager) (gps.ProjectConstrai
 }
 
 func checkErrors(m map[string]pkgtree.PackageOrErr) error {
-	noGoErrors, pkgErrors := 0, 0
+	var (
+		buildErrors []string
+		noGoErrors  int
+	)
+
 	for _, poe := range m {
 		if poe.Err != nil {
 			switch poe.Err.(type) {
 			case *build.NoGoError:
 				noGoErrors++
 			default:
-				pkgErrors++
+				buildErrors = append(buildErrors, poe.Err.Error())
 			}
 		}
 	}
+
 	if len(m) == 0 || len(m) == noGoErrors {
 		return errors.New("all dirs lacked any go code")
 	}
 
-	if len(m) == pkgErrors {
-		return errors.New("all dirs had go code with errors")
-	}
-
-	if len(m) == pkgErrors+noGoErrors {
-		return errors.Errorf("%d dirs had errors and %d had no go code", pkgErrors, noGoErrors)
+	if len(buildErrors) > 0 {
+		return errors.Errorf("Found %d errors:\n\n%s", len(buildErrors), strings.Join(buildErrors, "\n"))
 	}
 
 	return nil

cmd/dep/ensure_test.go

@@ -1,13 +1,16 @@
-// Copyright 2016 The Go Authors. All rights reserved.
+// Copyright 2017 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 package main
 
 import (
+	"errors"
+	"go/build"
 	"testing"
 
 	"github.com/golang/dep/internal/gps"
+	"github.com/golang/dep/internal/gps/pkgtree"
 )
 
 func TestInvalidEnsureFlagCombinations(t *testing.T) {
@@ -49,3 +52,62 @@ func TestInvalidEnsureFlagCombinations(t *testing.T) {
 		t.Errorf("no args to plain ensure")
 	}
 }
+
+func TestCheckErrors(t *testing.T) {
+	tt := []struct {
+		name        string
+		hasErrs     bool
+		pkgOrErrMap map[string]pkgtree.PackageOrErr
+	}{
+		{
+			name:    "noErrors",
+			hasErrs: false,
+			pkgOrErrMap: map[string]pkgtree.PackageOrErr{
+				"mypkg": {
+					P: pkgtree.Package{},
+				},
+			},
+		},
+		{
+			name:    "hasErrors",
+			hasErrs: true,
+			pkgOrErrMap: map[string]pkgtree.PackageOrErr{
+				"github.com/me/pkg": {
+					Err: &build.NoGoError{},
+				},
+				"github.com/someone/pkg": {
+					Err: errors.New("code is busted"),
+				},
+			},
+		},
+		{
+			name:    "onlyGoErrors",
+			hasErrs: false,
+			pkgOrErrMap: map[string]pkgtree.PackageOrErr{
+				"github.com/me/pkg": {
+					Err: &build.NoGoError{},
+				},
+				"github.com/someone/pkg": {
+					P: pkgtree.Package{},
+				},
+			},
+		},
+		{
+			name:    "allGoErrors",
+			hasErrs: true,
+			pkgOrErrMap: map[string]pkgtree.PackageOrErr{
+				"github.com/me/pkg": {
+					Err: &build.NoGoError{},
+				},
+			},
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			if hasErrs := checkErrors(tc.pkgOrErrMap) != nil; hasErrs != tc.hasErrs {
+				t.Fail()
+			}
+		})
+	}
+}

cmd/dep/testdata/harness_tests/README.md

@@ -36,7 +36,7 @@ will be included automatically.
 
 The json file needs to be accompanied by `initial` and `final` directories. The
 `initial` is copied verbatim into the test project before the `dep` commands are
-run, are the `manifest` and `lock` files in `final`, if present, are used to
+run, and the `manifest` and `lock` files in `final`, if present, are used to
 compare against the test project results after the commands. The `stdout.txt` file
 is optional, and if present will be compared with command output.
 

cmd/dep/testdata/harness_tests/ensure/pkg-errors/case2/initial/bar/bar.go

@@ -0,0 +1,3 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.

cmd/dep/testdata/harness_tests/ensure/pkg-errors/case2/testcase.json

@@ -0,0 +1,7 @@
+{
+    "commands": [
+        ["ensure"]
+    ],
+    "error-expected": "Found 1 errors",
+    "vendor-final": []
+}