From 55f46ef531b43d355c42c717655164581adf9095 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Tue, 27 Oct 2020 22:19:34 -0700 Subject: [PATCH] Fix nits in depchecker (#1865) --- depcheck/depcheck.go | 13 +++++-------- depcheck/depcheck_test.go | 4 ++-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/depcheck/depcheck.go b/depcheck/depcheck.go index aaa88762f..8febf632f 100644 --- a/depcheck/depcheck.go +++ b/depcheck/depcheck.go @@ -101,8 +101,7 @@ func buildGraph(importpaths ...string) (graph, error) { // CheckNoDependency checks that the given import paths (ip) does not // depend (transitively) on certain banned imports. -func CheckNoDependency(t *testing.T, ip string, banned []string) error { - t.Helper() +func CheckNoDependency(ip string, banned []string) error { g, err := buildGraph(ip) if err != nil { return fmt.Errorf("buildGraph(queue) = %v", err) @@ -122,7 +121,7 @@ func AssertNoDependency(t *testing.T, banned map[string][]string) { t.Helper() for ip, banned := range banned { t.Run(ip, func(t *testing.T) { - if err := CheckNoDependency(t, ip, banned); err != nil { + if err := CheckNoDependency(ip, banned); err != nil { t.Error("CheckNoDependency() =", err) } }) @@ -134,16 +133,14 @@ func AssertNoDependency(t *testing.T, banned map[string][]string) { // Note: while perhaps counterintuitive we allow the value to be a superset // of the actual imports to that folks can use a constant that holds blessed // import paths. -func CheckOnlyDependencies(t *testing.T, ip string, allowed map[string]struct{}) error { - t.Helper() - +func CheckOnlyDependencies(ip string, allowed map[string]struct{}) error { g, err := buildGraph(ip) if err != nil { return fmt.Errorf("buildGraph(queue) = %v", err) } for _, name := range g.order() { if _, ok := allowed[name]; !ok { - return fmt.Errorf("Dependency %s of %s is not explicitly allowed\n%s", name, ip, + return fmt.Errorf("dependency %s of %s is not explicitly allowed\n%s", name, ip, strings.Join(g.path(name), "\n")) } } @@ -164,7 +161,7 @@ func AssertOnlyDependencies(t *testing.T, allowed map[string][]string) { allowed[x] = struct{}{} } t.Run(ip, func(t *testing.T) { - if err := CheckOnlyDependencies(t, ip, allowed); err != nil { + if err := CheckOnlyDependencies(ip, allowed); err != nil { t.Error("CheckOnlyDependencies() =", err) } }) diff --git a/depcheck/depcheck_test.go b/depcheck/depcheck_test.go index 5954f1166..e54ca2c4d 100644 --- a/depcheck/depcheck_test.go +++ b/depcheck/depcheck_test.go @@ -48,7 +48,7 @@ func TestExample(t *testing.T) { }) // Sample failure case, duck clearly relies on corev1 for all assortment of things. - if err := depcheck.CheckNoDependency(t, "knative.dev/pkg/apis/duck", []string{"k8s.io/api/core/v1"}); err == nil { + if err := depcheck.CheckNoDependency("knative.dev/pkg/apis/duck", []string{"k8s.io/api/core/v1"}); err == nil { t.Error("CheckNoDependency() = nil, wanted error") } else if !strings.Contains(err.Error(), "knative.dev/pkg/tracker") { t.Errorf("CheckNoDependency() = %v, expected to contain: %v", err, "knative.dev/pkg/tracker") @@ -71,7 +71,7 @@ func TestExample(t *testing.T) { }) // Sample failure case, doesn't include transitive dependencies! - if err := depcheck.CheckOnlyDependencies(t, "knative.dev/pkg/depcheck", map[string]struct{}{ + if err := depcheck.CheckOnlyDependencies("knative.dev/pkg/depcheck", map[string]struct{}{ "fmt": {}, "sort": {}, "strings": {},