Merge pull request moby#48819 from thaJeztah/image_history_nowrap

client: Client.ImageHistory: don't decorate error twice, and improve tests

client/image_history.go

@@ -3,7 +3,6 @@ package client // import "github.com/docker/docker/client"
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"net/url"
 
 	"github.com/docker/docker/api/types/image"
@@ -19,18 +18,18 @@ func (cli *Client) ImageHistory(ctx context.Context, imageID string, opts image.
 
 		p, err := encodePlatform(opts.Platform)
 		if err != nil {
-			return nil, fmt.Errorf("invalid platform: %v", err)
+			return nil, err
 		}
 		query.Set("platform", p)
 	}
 
-	var history []image.HistoryResponseItem
 	serverResp, err := cli.get(ctx, "/images/"+imageID+"/history", query, nil)
 	defer ensureReaderClosed(serverResp)
 	if err != nil {
-		return history, err
+		return nil, err
 	}
 
+	var history []image.HistoryResponseItem
 	err = json.NewDecoder(serverResp.body).Decode(&history)
 	return history, err
 }

client/image_history_test.go

@@ -1,17 +1,15 @@
 package client // import "github.com/docker/docker/client"
 
 import (
-	"bytes"
 	"context"
-	"encoding/json"
-	"fmt"
 	"io"
 	"net/http"
 	"strings"
 	"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,37 +23,39 @@ func TestImageHistoryError(t *testing.T) {
 }
 
 func TestImageHistory(t *testing.T) {
-	expectedURL := "/images/image_id/history"
+	const (
+		expectedURL      = "/images/image_id/history"
+		historyResponse  = `[{"Comment":"","Created":0,"CreatedBy":"","Id":"image_id1","Size":0,"Tags":["tag1","tag2"]},{"Comment":"","Created":0,"CreatedBy":"","Id":"image_id2","Size":0,"Tags":["tag1","tag2"]}]`
+		expectedPlatform = `{"architecture":"arm64","os":"linux","variant":"v8"}`
+	)
 	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)
-			}
-			b, err := json.Marshal([]image.HistoryResponseItem{
-				{
-					ID:   "image_id1",
-					Tags: []string{"tag1", "tag2"},
-				},
-				{
-					ID:   "image_id2",
-					Tags: []string{"tag1", "tag2"},
-				},
-			})
-			if err != nil {
-				return nil, err
-			}
-
+			assert.Check(t, is.Equal(r.URL.Path, expectedURL))
+			assert.Check(t, is.Equal(r.URL.Query().Get("platform"), expectedPlatform))
 			return &http.Response{
 				StatusCode: http.StatusOK,
-				Body:       io.NopCloser(bytes.NewReader(b)),
+				Body:       io.NopCloser(strings.NewReader(historyResponse)),
 			}, nil
 		}),
 	}
-	imageHistories, err := client.ImageHistory(context.Background(), "image_id", image.HistoryOptions{})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(imageHistories) != 2 {
-		t.Fatalf("expected 2 containers, got %v", imageHistories)
+	expected := []image.HistoryResponseItem{
+		{
+			ID:   "image_id1",
+			Tags: []string{"tag1", "tag2"},
+		},
+		{
+			ID:   "image_id2",
+			Tags: []string{"tag1", "tag2"},
+		},
 	}
+
+	imageHistories, err := client.ImageHistory(context.Background(), "image_id", image.HistoryOptions{
+		Platform: &ocispec.Platform{
+			Architecture: "arm64",
+			OS:           "linux",
+			Variant:      "v8",
+		},
+	})
+	assert.NilError(t, err)
+	assert.Check(t, is.DeepEqual(imageHistories, expected))
 }