Repository: knative/pkg
Pull request #1865 (2020-10-28 05:06:35)

Fix nits in depchecker
/assign @mattmoor 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1865#" title="Author self-approved">vagababov</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/pkg/blob/master/OWNERS)~~ [vagababov]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/1865?src=pr&el=h1) Report
> Merging [#1865](https://codecov.io/gh/knative/pkg/pull/1865?src=pr&el=desc) into [master](https://codecov.io/gh/knative/pkg/commit/ed076316cbe1107590822f930f4ed20b109290f1?el=desc) will **increase** coverage by `1.12%`.
> The diff coverage is `33.33%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/pkg/pull/1865/graphs/tree.svg?width=650&height=150&src=pr&token=rlqpDnfxbO)](https://codecov.io/gh/knative/pkg/pull/1865?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1865      +/-   ##
==========================================
+ Coverage   68.25%   69.37%   +1.12%     
==========================================
  Files         223      223              
  Lines        9479     9477       -2     
==========================================
+ Hits         6470     6575     +105     
+ Misses       2682     2572     -110     
- Partials      327      330       +3     
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/1865?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [depcheck/depcheck.go](https://codecov.io/gh/knative/pkg/pull/1865/diff?src=pr&el=tree#diff-ZGVwY2hlY2svZGVwY2hlY2suZ28=) | `82.14% <33.33%> (-0.62%)` | :arrow_down: |
| [test/gcs/mock/mock.go](https://codecov.io/gh/knative/pkg/pull/1865/diff?src=pr&el=tree#diff-dGVzdC9nY3MvbW9jay9tb2NrLmdv) | `92.47% <0.00%> (+2.15%)` | :arrow_up: |
| [leaderelection/config.go](https://codecov.io/gh/knative/pkg/pull/1865/diff?src=pr&el=tree#diff-bGVhZGVyZWxlY3Rpb24vY29uZmlnLmdv) | `77.27% <0.00%> (+4.54%)` | :arrow_up: |
| [leaderelection/context.go](https://codecov.io/gh/knative/pkg/pull/1865/diff?src=pr&el=tree#diff-bGVhZGVyZWxlY3Rpb24vY29udGV4dC5nbw==) | `91.81% <0.00%> (+91.81%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/1865?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/pkg/pull/1865?src=pr&el=footer). Last update [ed07631...aefd008](https://codecov.io/gh/knative/pkg/pull/1865?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
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)
 			}
 		})


Test patch:
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":                        {},