commit 03416110c68c443e69bb909c4e09f4c065df5041
author lucassloan <lucassloan@google.com> Mon Jul 01 13:19:57 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Mon Jul 01 13:20:16 2019 -0700
tree 97b34535ce829280abd1707579ee77192bbfd606
parent eb40f70cf45503c1e7df37ce52047b99acf3b5e6

Disambiguate expected and unexpected errors for the presubmit service.

PiperOrigin-RevId: 256018619

ts_auto_deps/updater/updater.go

diff --git a/ts_auto_deps/updater/updater.go b/ts_auto_deps/updater/updater.go
index b5b7720..0c52437 100644
--- a/ts_auto_deps/updater/updater.go
+++ b/ts_auto_deps/updater/updater.go
@@ -108,14 +108,7 @@
 	args = append(args, targets...)
 	out, stderr, err := upd.bazelAnalyze(buildFilePath, args)
 	if err != nil {
-		return nil, &AnalysisFailedError{
-			[]AnalysisFailureCause{
-				AnalysisFailureCause{
-					Message: fmt.Sprintf("running bazel analyze %s failed: %v", args, err),
-					Path:    bld.Path,
-				},
-			},
-		}
+		return nil, err
 	}
 
 	var res arpb.AnalyzeResult
@@ -217,7 +210,25 @@
 		}
 		return nil, fmt.Errorf("reading %q: %s", buildFilePath, err)
 	}
-	return build.ParseBuild(normalizedG3Path, data)
+	bld, err := build.ParseBuild(normalizedG3Path, data)
+	if err != nil {
+		if parseErr, ok := err.(build.ParseError); ok {
+			return nil, &AnalysisFailedError{
+				[]AnalysisFailureCause{
+					AnalysisFailureCause{
+						Message: parseErr.Error(),
+						Path:    parseErr.Filename,
+						Line:    parseErr.Pos.Line,
+					},
+				},
+			}
+
+		}
+
+		// wasn't an error we know how to parse
+		return nil, err
+	}
+	return bld, nil
 }
 
 type srcSet map[string]bool
@@ -433,9 +444,16 @@
 		}
 		hadUnresolved := len(report.UnresolvedImport) > 0
 		if hadUnresolved {
-			return fmt.Errorf("ERROR in %s: unresolved imports %s.\nMaybe you are missing a "+
-				"'// from ...'' comment, or the target BUILD files are incorrect?\n%s\n",
-				fullTarget, report.UnresolvedImport, strings.Join(report.GetFeedback(), "\n"))
+			return &AnalysisFailedError{
+				[]AnalysisFailureCause{
+					AnalysisFailureCause{
+						Message: fmt.Sprintf("ERROR in %s: unresolved imports %s.\nMaybe you are missing a "+
+							"'// from ...'' comment, or the target BUILD files are incorrect?\n%s\n",
+							fullTarget, report.UnresolvedImport, strings.Join(report.GetFeedback(), "\n")),
+						Path: bld.Path,
+					},
+				},
+			}
 		}
 		for _, d := range report.UnnecessaryDependency {
 			platform.Infof("Removing dependency on %s from %s\n", d, fullTarget)

ts_auto_deps/updater/updater_test.go

diff --git a/ts_auto_deps/updater/updater_test.go b/ts_auto_deps/updater/updater_test.go
index 65dd3bb..b62ef20 100644
--- a/ts_auto_deps/updater/updater_test.go
+++ b/ts_auto_deps/updater/updater_test.go
@@ -2,11 +2,11 @@
 
 import (
 	"context"
-	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/bazelbuild/buildtools/build"
@@ -238,12 +238,11 @@
 		t.Fatal(err)
 	}
 
-	expectedErr := fmt.Errorf("ERROR in %s: unresolved imports %s.\nMaybe you are missing a "+
-		"'// from ...'' comment, or the target BUILD files are incorrect?\n\n", "//foo:bar", []string{"unresolved/import"})
+	expectedErr := "'// from ...'' comment, or the target BUILD files are incorrect?"
 
 	err = updateDeps(bld, []*arpb.DependencyReport{report})
-	if !reflect.DeepEqual(err, expectedErr) {
-		t.Errorf("returned error %s: expected %s", err, expectedErr)
+	if !strings.Contains(err.Error(), expectedErr) {
+		t.Errorf("returned error %s: expected it to contain %s", err, expectedErr)
 	}
 }