Disambiguate expected and unexpected errors for the presubmit service. PiperOrigin-RevId: 256018619
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)
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) } }