From 613538469b1d72a9b5000f2be0ac386e04068111 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 18 Nov 2024 14:00:32 +0100 Subject: [PATCH 1/4] client: TestImageLoad: rewrite to use table-tests, use asserts Signed-off-by: Sebastiaan van Stijn --- client/image_load_test.go | 93 +++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 52 deletions(-) diff --git a/client/image_load_test.go b/client/image_load_test.go index b184f8cf3af26..3f51caa57557f 100644 --- a/client/image_load_test.go +++ b/client/image_load_test.go @@ -3,10 +3,9 @@ package client // import "github.com/docker/docker/client" import ( "bytes" "context" - "fmt" "io" "net/http" - "strings" + "net/url" "testing" "github.com/docker/docker/api/types/image" @@ -25,73 +24,63 @@ func TestImageLoadError(t *testing.T) { } func TestImageLoad(t *testing.T) { - expectedURL := "/images/load" - expectedInput := "inputBody" - expectedOutput := "outputBody" - loadCases := []struct { + const ( + expectedURL = "/images/load" + expectedContentType = "application/x-tar" + expectedInput = "inputBody" + expectedOutput = "outputBody" + ) + tests := []struct { + doc string quiet bool responseContentType string expectedResponseJSON bool - expectedQueryParams map[string]string + expectedQueryParams url.Values }{ { + doc: "plain-text", quiet: false, responseContentType: "text/plain", expectedResponseJSON: false, - expectedQueryParams: map[string]string{ - "quiet": "0", + expectedQueryParams: url.Values{ + "quiet": {"0"}, }, }, { + doc: "json quiet", quiet: true, responseContentType: "application/json", expectedResponseJSON: true, - expectedQueryParams: map[string]string{ - "quiet": "1", + expectedQueryParams: url.Values{ + "quiet": {"1"}, }, }, } - for _, loadCase := range loadCases { - client := &Client{ - client: newMockClient(func(req *http.Request) (*http.Response, error) { - if !strings.HasPrefix(req.URL.Path, expectedURL) { - return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL) - } - contentType := req.Header.Get("Content-Type") - if contentType != "application/x-tar" { - return nil, fmt.Errorf("content-type not set in URL headers properly. Expected 'application/x-tar', got %s", contentType) - } - query := req.URL.Query() - for key, expected := range loadCase.expectedQueryParams { - actual := query.Get(key) - if actual != expected { - return nil, fmt.Errorf("%s not set in URL query properly. Expected '%s', got %s", key, expected, actual) - } - } - headers := http.Header{} - headers.Add("Content-Type", loadCase.responseContentType) - return &http.Response{ - StatusCode: http.StatusOK, - Body: io.NopCloser(bytes.NewReader([]byte(expectedOutput))), - Header: headers, - }, nil - }), - } + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { + client := &Client{ + client: newMockClient(func(req *http.Request) (*http.Response, error) { + assert.Check(t, is.Equal(req.URL.Path, expectedURL)) + assert.Check(t, is.Equal(req.Header.Get("Content-Type"), expectedContentType)) + assert.Check(t, is.DeepEqual(req.URL.Query(), tc.expectedQueryParams)) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader([]byte(expectedOutput))), + Header: http.Header{"Content-Type": []string{tc.responseContentType}}, + }, nil + }), + } - input := bytes.NewReader([]byte(expectedInput)) - imageLoadResponse, err := client.ImageLoad(context.Background(), input, image.LoadOptions{Quiet: loadCase.quiet}) - if err != nil { - t.Fatal(err) - } - if imageLoadResponse.JSON != loadCase.expectedResponseJSON { - t.Fatalf("expected a JSON response, was not.") - } - body, err := io.ReadAll(imageLoadResponse.Body) - if err != nil { - t.Fatal(err) - } - if string(body) != expectedOutput { - t.Fatalf("expected %s, got %s", expectedOutput, string(body)) - } + input := bytes.NewReader([]byte(expectedInput)) + imageLoadResponse, err := client.ImageLoad(context.Background(), input, image.LoadOptions{ + Quiet: tc.quiet, + }) + assert.NilError(t, err) + assert.Check(t, is.Equal(imageLoadResponse.JSON, tc.expectedResponseJSON)) + + body, err := io.ReadAll(imageLoadResponse.Body) + assert.NilError(t, err) + assert.Check(t, is.Equal(string(body), expectedOutput)) + }) } } From 1ea24b7be3d32ad66f24fbf180b633dd09594754 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 18 Nov 2024 14:06:12 +0100 Subject: [PATCH 2/4] client: TestImageLoad: add test-case for platform Signed-off-by: Sebastiaan van Stijn --- client/image_load_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/client/image_load_test.go b/client/image_load_test.go index 3f51caa57557f..fa37dc5dd444f 100644 --- a/client/image_load_test.go +++ b/client/image_load_test.go @@ -10,6 +10,7 @@ import ( "github.com/docker/docker/api/types/image" "github.com/docker/docker/errdefs" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -33,6 +34,7 @@ func TestImageLoad(t *testing.T) { tests := []struct { doc string quiet bool + platform *ocispec.Platform responseContentType string expectedResponseJSON bool expectedQueryParams url.Values @@ -55,6 +57,16 @@ func TestImageLoad(t *testing.T) { "quiet": {"1"}, }, }, + { + doc: "json with platform", + platform: &ocispec.Platform{Architecture: "arm64", OS: "linux", Variant: "v8"}, + responseContentType: "application/json", + expectedResponseJSON: true, + expectedQueryParams: url.Values{ + "platform": {`{"architecture":"arm64","os":"linux","variant":"v8"}`}, + "quiet": {"0"}, + }, + }, } for _, tc := range tests { t.Run(tc.doc, func(t *testing.T) { @@ -73,7 +85,8 @@ func TestImageLoad(t *testing.T) { input := bytes.NewReader([]byte(expectedInput)) imageLoadResponse, err := client.ImageLoad(context.Background(), input, image.LoadOptions{ - Quiet: tc.quiet, + Quiet: tc.quiet, + Platform: tc.platform, }) assert.NilError(t, err) assert.Check(t, is.Equal(imageLoadResponse.JSON, tc.expectedResponseJSON)) From 2bab030d6c178d806bd780de409492178f21f78a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 18 Nov 2024 14:29:10 +0100 Subject: [PATCH 3/4] client: TestImageSave: use table-test, asserts, add platform test-case Signed-off-by: Sebastiaan van Stijn --- client/image_save_test.go | 77 +++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/client/image_save_test.go b/client/image_save_test.go index 935fae598e536..31eec84b95f85 100644 --- a/client/image_save_test.go +++ b/client/image_save_test.go @@ -3,15 +3,14 @@ package client // import "github.com/docker/docker/client" import ( "bytes" "context" - "fmt" "io" "net/http" - "reflect" - "strings" + "net/url" "testing" "github.com/docker/docker/api/types/image" "github.com/docker/docker/errdefs" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -25,35 +24,51 @@ func TestImageSaveError(t *testing.T) { } func TestImageSave(t *testing.T) { - expectedURL := "/images/get" - client := &Client{ - client: newMockClient(func(r *http.Request) (*http.Response, error) { - if !strings.HasPrefix(r.URL.Path, expectedURL) { - return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, r.URL) - } - query := r.URL.Query() - names := query["names"] - expectedNames := []string{"image_id1", "image_id2"} - if !reflect.DeepEqual(names, expectedNames) { - return nil, fmt.Errorf("names not set in URL query properly. Expected %v, got %v", names, expectedNames) + const ( + expectedURL = "/images/get" + expectedOutput = "outputBody" + ) + tests := []struct { + doc string + options image.SaveOptions + expectedQueryParams url.Values + }{ + { + doc: "no platform", + expectedQueryParams: url.Values{ + "names": {"image_id1", "image_id2"}, + }, + }, + { + doc: "platform", + options: image.SaveOptions{ + Platform: &ocispec.Platform{Architecture: "arm64", OS: "linux", Variant: "v8"}, + }, + expectedQueryParams: url.Values{ + "names": {"image_id1", "image_id2"}, + "platform": {`{"architecture":"arm64","os":"linux","variant":"v8"}`}, + }, + }, + } + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { + client := &Client{ + client: newMockClient(func(req *http.Request) (*http.Response, error) { + assert.Check(t, is.Equal(req.URL.Path, expectedURL)) + assert.Check(t, is.DeepEqual(req.URL.Query(), tc.expectedQueryParams)) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader([]byte(expectedOutput))), + }, nil + }), } + resp, err := client.ImageSave(context.Background(), []string{"image_id1", "image_id2"}, tc.options) + assert.NilError(t, err) + defer assert.NilError(t, resp.Close()) - return &http.Response{ - StatusCode: http.StatusOK, - Body: io.NopCloser(bytes.NewReader([]byte("response"))), - }, nil - }), - } - saveResponse, err := client.ImageSave(context.Background(), []string{"image_id1", "image_id2"}, image.SaveOptions{}) - if err != nil { - t.Fatal(err) - } - response, err := io.ReadAll(saveResponse) - if err != nil { - t.Fatal(err) - } - saveResponse.Close() - if string(response) != "response" { - t.Fatalf("expected response to contain 'response', got %s", string(response)) + body, err := io.ReadAll(resp) + assert.NilError(t, err) + assert.Equal(t, string(body), expectedOutput) + }) } } From a333c2990f027403753d9499d40e0b192fd289f1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 18 Nov 2024 15:14:33 +0100 Subject: [PATCH 4/4] client: TestImageImport: use table-test, asserts, add platform test-case Signed-off-by: Sebastiaan van Stijn --- client/image_import_test.go | 123 ++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 53 deletions(-) diff --git a/client/image_import_test.go b/client/image_import_test.go index 91e6cc1a0378f..3db929e1e93cd 100644 --- a/client/image_import_test.go +++ b/client/image_import_test.go @@ -3,10 +3,9 @@ package client // import "github.com/docker/docker/client" import ( "bytes" "context" - "fmt" "io" "net/http" - "reflect" + "net/url" "strings" "testing" @@ -25,58 +24,76 @@ func TestImageImportError(t *testing.T) { } func TestImageImport(t *testing.T) { - expectedURL := "/images/create" - client := &Client{ - client: newMockClient(func(r *http.Request) (*http.Response, error) { - if !strings.HasPrefix(r.URL.Path, expectedURL) { - return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, r.URL) - } - query := r.URL.Query() - fromSrc := query.Get("fromSrc") - if fromSrc != "image_source" { - return nil, fmt.Errorf("fromSrc not set in URL query properly. Expected 'image_source', got %s", fromSrc) - } - repo := query.Get("repo") - if repo != "repository_name:imported" { - return nil, fmt.Errorf("repo not set in URL query properly. Expected 'repository_name:imported', got %s", repo) - } - tag := query.Get("tag") - if tag != "imported" { - return nil, fmt.Errorf("tag not set in URL query properly. Expected 'imported', got %s", tag) - } - message := query.Get("message") - if message != "A message" { - return nil, fmt.Errorf("message not set in URL query properly. Expected 'A message', got %s", message) - } - changes := query["changes"] - expectedChanges := []string{"change1", "change2"} - if !reflect.DeepEqual(expectedChanges, changes) { - return nil, fmt.Errorf("changes not set in URL query properly. Expected %v, got %v", expectedChanges, changes) + const ( + expectedURL = "/images/create" + expectedOutput = "outputBody" + ) + tests := []struct { + doc string + options image.ImportOptions + expectedQueryParams url.Values + }{ + { + doc: "no options", + expectedQueryParams: url.Values{ + "fromSrc": {"image_source"}, + "message": {""}, + "repo": {"repository_name:imported"}, + "tag": {""}, + }, + }, + { + doc: "change options", + options: image.ImportOptions{ + Tag: "imported", + Message: "A message", + Changes: []string{"change1", "change2"}, + }, + expectedQueryParams: url.Values{ + "changes": {"change1", "change2"}, + "fromSrc": {"image_source"}, + "message": {"A message"}, + "repo": {"repository_name:imported"}, + "tag": {"imported"}, + }, + }, + { + doc: "with platform", + options: image.ImportOptions{ + Platform: "linux/amd64", + }, + expectedQueryParams: url.Values{ + "fromSrc": {"image_source"}, + "message": {""}, + "platform": {"linux/amd64"}, + "repo": {"repository_name:imported"}, + "tag": {""}, + }, + }, + } + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { + client := &Client{ + client: newMockClient(func(req *http.Request) (*http.Response, error) { + assert.Check(t, is.Equal(req.URL.Path, expectedURL)) + query := req.URL.Query() + assert.Check(t, is.DeepEqual(query, tc.expectedQueryParams)) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader([]byte(expectedOutput))), + }, nil + }), } + resp, err := client.ImageImport(context.Background(), image.ImportSource{ + Source: strings.NewReader("source"), + SourceName: "image_source", + }, "repository_name:imported", tc.options) + assert.NilError(t, err) + defer assert.NilError(t, resp.Close()) - return &http.Response{ - StatusCode: http.StatusOK, - Body: io.NopCloser(bytes.NewReader([]byte("response"))), - }, nil - }), - } - importResponse, err := client.ImageImport(context.Background(), image.ImportSource{ - Source: strings.NewReader("source"), - SourceName: "image_source", - }, "repository_name:imported", image.ImportOptions{ - Tag: "imported", - Message: "A message", - Changes: []string{"change1", "change2"}, - }) - if err != nil { - t.Fatal(err) - } - response, err := io.ReadAll(importResponse) - if err != nil { - t.Fatal(err) - } - importResponse.Close() - if string(response) != "response" { - t.Fatalf("expected response to contain 'response', got %s", string(response)) + body, err := io.ReadAll(resp) + assert.NilError(t, err) + assert.Equal(t, string(body), expectedOutput) + }) } }