Repository: kubernetes-sigs/krew
Pull request #362 (2019-10-29 23:05:42)

Remove trueVerifier from main logic
This verifier implementation is only used in tests, so it should be defined there.

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/362#" title="Author self-approved">corneliusweig</a>*

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

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/kubernetes-sigs/krew/blob/master/OWNERS)~~ [corneliusweig]

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-io: # [Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/362?src=pr&el=h1) Report
> Merging [#362](https://codecov.io/gh/kubernetes-sigs/krew/pull/362?src=pr&el=desc) into [master](https://codecov.io/gh/kubernetes-sigs/krew/commit/d67e08799abe4fe2b8d3210cddc8bb2277fe9e85?src=pr&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/kubernetes-sigs/krew/pull/362/graphs/tree.svg?width=650&token=J3nFtARI2m&height=150&src=pr)](https://codecov.io/gh/kubernetes-sigs/krew/pull/362?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##           master     #362     +/-   ##
=========================================
- Coverage   56.78%   56.68%   -0.1%     
=========================================
  Files          19       19             
  Lines         907      905      -2     
=========================================
- Hits          515      513      -2     
  Misses        339      339             
  Partials       53       53
```


| [Impacted Files](https://codecov.io/gh/kubernetes-sigs/krew/pull/362?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/download/verifier.go](https://codecov.io/gh/kubernetes-sigs/krew/pull/362/diff?src=pr&el=tree#diff-cGtnL2Rvd25sb2FkL3ZlcmlmaWVyLmdv) | `100% <ø> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/362?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/kubernetes-sigs/krew/pull/362?src=pr&el=footer). Last update [d67e087...919a798](https://codecov.io/gh/kubernetes-sigs/krew/pull/362?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ahmetb: /lgtm
/approve
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/362#issuecomment-548554792" title="Approved">ahmetb</a>*, *<a href="https://github.com/kubernetes-sigs/krew/pull/362#" title="Author self-approved">corneliusweig</a>*

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

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/kubernetes-sigs/krew/blob/master/OWNERS)~~ [ahmetb,corneliusweig]

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":[]} -->

Patch:
diff --git a/pkg/download/verifier.go b/pkg/download/verifier.go
index 459e50a..1e8fb5a 100644
--- a/pkg/download/verifier.go
+++ b/pkg/download/verifier.go
@@ -20,7 +20,6 @@ import (
 	"encoding/hex"
 	"hash"
 	"io"
-	"io/ioutil"
 
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
@@ -55,11 +54,3 @@ func (v sha256Verifier) Verify() error {
 	}
 	return errors.Errorf("checksum does not match, want: %x, got %x", v.wantedHash, v.Sum(nil))
 }
-
-var _ Verifier = trueVerifier{}
-
-type trueVerifier struct{ io.Writer }
-
-// NewInsecureVerifier returns a Verifier that always verifies to true.
-func NewInsecureVerifier() Verifier { return trueVerifier{ioutil.Discard} }
-func (trueVerifier) Verify() error  { return nil }


Test patch:
diff --git a/pkg/download/downloader_test.go b/pkg/download/downloader_test.go
index b8960db..6b9da84 100644
--- a/pkg/download/downloader_test.go
+++ b/pkg/download/downloader_test.go
@@ -165,7 +165,7 @@ func TestDownloader_Get(t *testing.T) {
 		{
 			name: "successful get",
 			fields: fields{
-				verifier: NewInsecureVerifier(),
+				verifier: newTrueVerifier(),
 				fetcher:  NewFileFetcher(filepath.Join(testdataPath(), "test-with-directory.zip")),
 			},
 			uri:     "foo/bar/test-with-directory.zip",
@@ -174,7 +174,7 @@ func TestDownloader_Get(t *testing.T) {
 		{
 			name: "fail get by fetching",
 			fields: fields{
-				verifier: NewInsecureVerifier(),
+				verifier: newTrueVerifier(),
 				fetcher:  errorFetcher{},
 			},
 			uri:     "foo/bar/test-with-directory.zip",
@@ -217,7 +217,7 @@ func Test_download(t *testing.T) {
 			name: "successful fetch",
 			args: args{
 				url:      filePath,
-				verifier: NewInsecureVerifier(),
+				verifier: newTrueVerifier(),
 				fetcher:  NewFileFetcher(filePath),
 			},
 			wantReader: bytes.NewReader(downloadOriginal),
@@ -267,6 +267,14 @@ func Test_download(t *testing.T) {
 	}
 }
 
+var _ Verifier = trueVerifier{}
+
+type trueVerifier struct{ io.Writer }
+
+// newTrueVerifier returns a Verifier that always verifies to true.
+func newTrueVerifier() Verifier    { return trueVerifier{ioutil.Discard} }
+func (trueVerifier) Verify() error { return nil }
+
 var _ Verifier = falseVerifier{}
 
 type falseVerifier struct{ io.Writer }
diff --git a/pkg/download/verifier_test.go b/pkg/download/verifier_test.go
index 6b9a82f..c978435 100644
--- a/pkg/download/verifier_test.go
+++ b/pkg/download/verifier_test.go
@@ -58,37 +58,3 @@ func TestSha256Verifier(t *testing.T) {
 		})
 	}
 }
-
-func TestTrueVerifier(t *testing.T) {
-	tests := []struct {
-		name      string
-		write     []byte
-		wantError bool
-	}{
-		{
-			name:      "test okay hash",
-			write:     []byte("hello world"),
-			wantError: false,
-		},
-		{
-			name:      "test wrong hash",
-			write:     []byte("HELLO WORLD"),
-			wantError: false,
-		},
-		{
-			name:      "test empty hash",
-			write:     []byte{},
-			wantError: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			v := NewInsecureVerifier()
-			_, _ = io.Copy(v, bytes.NewReader(tt.write))
-			if err := v.Verify(); (err != nil) != tt.wantError {
-				t.Errorf("NewInsecureVerifier().Write(%x).Verify() = %v, wantReader %v", tt.write, err, tt.wantError)
-				return
-			}
-		})
-	}
-}