Repository: google/go-containerregistry
Pull request #636 (2019-12-16 19:22:03)

Add a bunch of tests to cover Catalog

codecov-io: # [Codecov](https://codecov.io/gh/google/go-containerregistry/pull/636?src=pr&el=h1) Report
> Merging [#636](https://codecov.io/gh/google/go-containerregistry/pull/636?src=pr&el=desc) into [master](https://codecov.io/gh/google/go-containerregistry/commit/ad02268455b7fad97f49fa578d6a432e152db4f3?src=pr&el=desc) will **increase** coverage by `0.99%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/google/go-containerregistry/pull/636/graphs/tree.svg?width=650&token=wG7iiDCdZ4&height=150&src=pr)](https://codecov.io/gh/google/go-containerregistry/pull/636?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #636      +/-   ##
==========================================
+ Coverage   75.36%   76.35%   +0.99%     
==========================================
  Files         101      101              
  Lines        4481     4483       +2     
==========================================
+ Hits         3377     3423      +46     
+ Misses        690      640      -50     
- Partials      414      420       +6
```


| [Impacted Files](https://codecov.io/gh/google/go-containerregistry/pull/636?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/v1/remote/list.go](https://codecov.io/gh/google/go-containerregistry/pull/636/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS9saXN0Lmdv) | `76.66% <100%> (+23.21%)` | :arrow_up: |
| [pkg/internal/retry/retry.go](https://codecov.io/gh/google/go-containerregistry/pull/636/diff?src=pr&el=tree#diff-cGtnL2ludGVybmFsL3JldHJ5L3JldHJ5Lmdv) | `100% <0%> (+10%)` | :arrow_up: |
| [pkg/v1/remote/catalog.go](https://codecov.io/gh/google/go-containerregistry/pull/636/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS9jYXRhbG9nLmdv) | `67.64% <0%> (+42.64%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/go-containerregistry/pull/636?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/google/go-containerregistry/pull/636?src=pr&el=footer). Last update [ad02268...4563964](https://codecov.io/gh/google/go-containerregistry/pull/636?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/v1/remote/list.go b/pkg/v1/remote/list.go
index 32b2a8ff..b2b03a1d 100644
--- a/pkg/v1/remote/list.go
+++ b/pkg/v1/remote/list.go
@@ -129,6 +129,9 @@ func getNextPageURL(resp *http.Response) (*url.URL, error) {
 	if err != nil {
 		return nil, err
 	}
+	if resp.Request == nil || resp.Request.URL == nil {
+		return nil, nil
+	}
 	linkURL = resp.Request.URL.ResolveReference(linkURL)
 	return linkURL, nil
 }


Test patch:
diff --git a/pkg/internal/retry/retry_test.go b/pkg/internal/retry/retry_test.go
index 000a4607..ace6d8d7 100644
--- a/pkg/internal/retry/retry_test.go
+++ b/pkg/internal/retry/retry_test.go
@@ -15,6 +15,7 @@
 package retry
 
 import (
+	"context"
 	"fmt"
 	"testing"
 )
@@ -50,6 +51,10 @@ func TestRetry(t *testing.T) {
 		predicate:   IsTemporary,
 		err:         temp{},
 		shouldRetry: true,
+	}, {
+		predicate:   IsTemporary,
+		err:         context.DeadlineExceeded,
+		shouldRetry: false,
 	}} {
 		// Make sure we retry 5 times if we shouldRetry.
 		steps := 5
diff --git a/pkg/v1/remote/catalog_test.go b/pkg/v1/remote/catalog_test.go
index dbd91b98..58b30b7f 100644
--- a/pkg/v1/remote/catalog_test.go
+++ b/pkg/v1/remote/catalog_test.go
@@ -15,6 +15,8 @@
 package remote
 
 import (
+	"context"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -71,6 +73,74 @@ func TestCatalogPage(t *testing.T) {
 			}
 
 			repos, err := CatalogPage(reg, "", 100)
+			if (err != nil) != tc.wantErr {
+				t.Errorf("CatalogPage() wrong error: %v, want %v: %v\n", (err != nil), tc.wantErr, err)
+			}
+
+			if diff := cmp.Diff(tc.wantRepos, repos); diff != "" {
+				t.Errorf("CatalogPage() wrong repos (-want +got) = %s", diff)
+			}
+		})
+	}
+}
+
+func TestCatalog(t *testing.T) {
+	cases := []struct {
+		name      string
+		pages     [][]byte
+		wantErr   bool
+		wantRepos []string
+	}{{
+		name: "success",
+		pages: [][]byte{
+			[]byte(`{"repositories":["test/one","test/two"]}`),
+			[]byte(`{"repositories":["test/three","test/four"]}`),
+		},
+		wantErr:   false,
+		wantRepos: []string{"test/one", "test/two", "test/three", "test/four"},
+	}, {
+		name:    "not json",
+		pages:   [][]byte{[]byte("notjson")},
+		wantErr: true,
+	}}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			catalogPath := "/v2/_catalog"
+			pageTwo := "/v2/_catalog_two"
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				page := 0
+				switch r.URL.Path {
+				case "/v2/":
+					w.WriteHeader(http.StatusOK)
+				case pageTwo:
+					page = 1
+					fallthrough
+				case catalogPath:
+					if r.Method != http.MethodGet {
+						t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet)
+					}
+
+					if page == 0 {
+						w.Header().Set("Link", fmt.Sprintf("<%s>", pageTwo))
+					}
+					w.Write(tc.pages[page])
+				default:
+					t.Fatalf("Unexpected path: %v", r.URL.Path)
+				}
+			}))
+			defer server.Close()
+			u, err := url.Parse(server.URL)
+			if err != nil {
+				t.Fatalf("url.Parse(%v) = %v", server.URL, err)
+			}
+
+			reg, err := name.NewRegistry(u.Host)
+			if err != nil {
+				t.Fatalf("name.NewRegistry(%v) = %v", u.Host, err)
+			}
+
+			repos, err := Catalog(context.Background(), reg)
 			if (err != nil) != tc.wantErr {
 				t.Errorf("Catalog() wrong error: %v, want %v: %v\n", (err != nil), tc.wantErr, err)
 			}
@@ -81,3 +151,32 @@ func TestCatalogPage(t *testing.T) {
 		})
 	}
 }
+
+func TestCancelledCatalog(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/v2/":
+			w.WriteHeader(http.StatusOK)
+		default:
+			t.Fatalf("Unexpected path: %v", r.URL.Path)
+		}
+	}))
+	defer server.Close()
+	u, err := url.Parse(server.URL)
+	if err != nil {
+		t.Fatalf("url.Parse(%v) = %v", server.URL, err)
+	}
+
+	reg, err := name.NewRegistry(u.Host)
+	if err != nil {
+		t.Fatalf("name.NewRegistry(%v) = %v", u.Host, err)
+	}
+
+	_, err = Catalog(ctx, reg)
+	if want, got := context.Canceled, err; got != want {
+		t.Errorf("wanted %v got %v", want, got)
+	}
+}
diff --git a/pkg/v1/remote/list_test.go b/pkg/v1/remote/list_test.go
index 8b472588..8f7dcac1 100644
--- a/pkg/v1/remote/list_test.go
+++ b/pkg/v1/remote/list_test.go
@@ -15,6 +15,7 @@
 package remote
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -83,3 +84,75 @@ func TestList(t *testing.T) {
 		})
 	}
 }
+
+func TestCancelledList(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	repoName := "doesnotmatter"
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/v2/":
+			w.WriteHeader(http.StatusOK)
+		default:
+			t.Fatalf("Unexpected path: %v", r.URL.Path)
+		}
+	}))
+	defer server.Close()
+	u, err := url.Parse(server.URL)
+	if err != nil {
+		t.Fatalf("url.Parse(%v) = %v", server.URL, err)
+	}
+
+	repo, err := name.NewRepository(fmt.Sprintf("%s/%s", u.Host, repoName), name.WeakValidation)
+	if err != nil {
+		t.Fatalf("name.NewRepository(%v) = %v", repoName, err)
+	}
+
+	_, err = ListWithContext(ctx, repo)
+	if want, got := context.Canceled, err; got != want {
+		t.Errorf("wanted %v got %v", want, got)
+	}
+}
+
+func makeResp(hdr string) *http.Response {
+	return &http.Response{
+		Header: http.Header{
+			"Link": []string{hdr},
+		},
+	}
+}
+
+func TestGetNextPageURL(t *testing.T) {
+	for _, hdr := range []string{
+		"",
+		"<",
+		"><",
+		"<>",
+		fmt.Sprintf("<%c>", 0x7f), // makes url.Parse fail
+	} {
+		u, err := getNextPageURL(makeResp(hdr))
+		if err == nil && u != nil {
+			t.Errorf("Expected err, got %+v", u)
+		}
+	}
+
+	good := &http.Response{
+		Header: http.Header{
+			"Link": []string{"<example.com>"},
+		},
+		Request: &http.Request{
+			URL: &url.URL{
+				Scheme: "https",
+			},
+		},
+	}
+	u, err := getNextPageURL(good)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if u.Scheme != "https" {
+		t.Errorf("expected scheme to match request, got %s", u.Scheme)
+	}
+}