Merge pull request #6721 from thaJeztah/less_reflect

reduce some uses of reflect package

Author: thaJeztah

cli/command/container/opts.go

@@ -795,7 +795,7 @@ func parseNetworkOpts(copts *containerOptions) (map[string]*network.EndpointSett
 		// and only a single network is specified, omit the endpoint-configuration
 		// on the client (the daemon will still create it when creating the container)
 		if i == 0 && len(copts.netMode.Value()) == 1 {
-			if ep == nil || reflect.DeepEqual(*ep, network.EndpointSettings{}) {
+			if ep == nil || reflect.ValueOf(*ep).IsZero() {
 				continue
 			}
 		}

cli/command/formatter/reflect_test.go

@@ -4,8 +4,10 @@
 package formatter
 
 import (
-	"reflect"
 	"testing"
+
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 )
 
 type dummy struct{}
@@ -45,24 +47,18 @@ var dummyExpected = map[string]any{
 func TestMarshalMap(t *testing.T) {
 	d := dummy{}
 	m, err := marshalMap(&d)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if !reflect.DeepEqual(dummyExpected, m) {
-		t.Fatalf("expected %+v, got %+v",
-			dummyExpected, m)
-	}
+	assert.NilError(t, err)
+	assert.Check(t, is.DeepEqual(m, dummyExpected))
 }
 
 func TestMarshalMapBad(t *testing.T) {
-	if _, err := marshalMap(nil); err == nil {
-		t.Fatal("expected an error (argument is nil)")
-	}
-	if _, err := marshalMap(dummy{}); err == nil {
-		t.Fatal("expected an error (argument is non-pointer)")
-	}
+	_, err := marshalMap(nil)
+	assert.Check(t, is.Error(err, "expected a pointer to a struct, got invalid"), "expected an error (argument is nil)")
+
+	_, err = marshalMap(dummy{})
+	assert.Check(t, is.Error(err, "expected a pointer to a struct, got struct"), "expected an error (argument is non-pointer)")
+
 	x := 42
-	if _, err := marshalMap(&x); err == nil {
-		t.Fatal("expected an error (argument is a pointer to non-struct)")
-	}
+	_, err = marshalMap(&x)
+	assert.Check(t, is.Error(err, "expected a pointer to a struct, got a pointer to int"), "expected an error (argument is a pointer to non-struct)")
 }

cli/command/node/formatter.go

@@ -1,9 +1,9 @@
 package node
 
 import (
+	"bytes"
 	"encoding/base64"
 	"fmt"
-	"reflect"
 	"strings"
 
 	"github.com/docker/cli/cli/command/formatter"
@@ -170,15 +170,23 @@ func (c *nodeContext) ManagerStatus() string {
 }
 
 func (c *nodeContext) TLSStatus() string {
-	if c.info.Cluster == nil || reflect.DeepEqual(c.info.Cluster.TLSInfo, swarm.TLSInfo{}) || reflect.DeepEqual(c.n.Description.TLSInfo, swarm.TLSInfo{}) {
+	if c.info.Cluster == nil || isEmptyTLSInfo(c.info.Cluster.TLSInfo) || isEmptyTLSInfo(c.n.Description.TLSInfo) {
 		return "Unknown"
 	}
-	if reflect.DeepEqual(c.n.Description.TLSInfo, c.info.Cluster.TLSInfo) {
+	if equalTLSInfo(c.n.Description.TLSInfo, c.info.Cluster.TLSInfo) {
 		return "Ready"
 	}
 	return "Needs Rotation"
 }
 
+func isEmptyTLSInfo(t swarm.TLSInfo) bool {
+	return t.TrustRoot == "" && len(t.CertIssuerSubject) == 0 && len(t.CertIssuerPublicKey) == 0
+}
+
+func equalTLSInfo(t, o swarm.TLSInfo) bool {
+	return t.TrustRoot == o.TrustRoot && bytes.Equal(t.CertIssuerSubject, o.CertIssuerSubject) && bytes.Equal(t.CertIssuerPublicKey, o.CertIssuerPublicKey)
+}
+
 func (c *nodeContext) EngineVersion() string {
 	return c.n.Description.Engine.EngineVersion
 }
@@ -320,8 +328,7 @@ func (ctx *nodeInspectContext) EngineVersion() string {
 }
 
 func (ctx *nodeInspectContext) HasTLSInfo() bool {
-	tlsInfo := ctx.Node.Description.TLSInfo
-	return !reflect.DeepEqual(tlsInfo, swarm.TLSInfo{})
+	return !isEmptyTLSInfo(ctx.Node.Description.TLSInfo)
 }
 
 func (ctx *nodeInspectContext) TLSInfoTrustRoot() string {

cli/command/service/update_test.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"fmt"
 	"net/netip"
-	"reflect"
 	"sort"
+	"strconv"
 	"testing"
 	"time"
 
@@ -368,23 +368,23 @@ func TestUpdateHealthcheckTable(t *testing.T) {
 			err:   "--no-healthcheck conflicts with --health-* options",
 		},
 	}
-	for i, c := range testCases {
-		flags := newUpdateCommand(nil).Flags()
-		for _, flag := range c.flags {
-			flags.Set(flag[0], flag[1])
-		}
-		cspec := &swarm.ContainerSpec{
-			Healthcheck: c.initial,
-		}
-		err := updateHealthcheck(flags, cspec)
-		if c.err != "" {
-			assert.Error(t, err, c.err)
-		} else {
-			assert.NilError(t, err)
-			if !reflect.DeepEqual(cspec.Healthcheck, c.expected) {
-				t.Errorf("incorrect result for test %d, expected health config:\n\t%#v\ngot:\n\t%#v", i, c.expected, cspec.Healthcheck)
+	for i, tc := range testCases {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			flags := newUpdateCommand(nil).Flags()
+			for _, flag := range tc.flags {
+				assert.Check(t, flags.Set(flag[0], flag[1]))
 			}
-		}
+			cspec := &swarm.ContainerSpec{
+				Healthcheck: tc.initial,
+			}
+			err := updateHealthcheck(flags, cspec)
+			if tc.err != "" {
+				assert.Error(t, err, tc.err)
+			} else {
+				assert.NilError(t, err)
+				assert.Check(t, is.DeepEqual(cspec.Healthcheck, tc.expected))
+			}
+		})
 	}
 }
 

cli/command/telemetry_utils_test.go

@@ -4,14 +4,15 @@ import (
 	"bytes"
 	"context"
 	"io"
-	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/docker/cli/cli/streams"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/spf13/cobra"
 	"go.opentelemetry.io/otel/attribute"
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 )
 
 func setupCobraCommands() (*cobra.Command, *cobra.Command, *cobra.Command) {
@@ -139,7 +140,7 @@ func TestStdioAttributes(t *testing.T) {
 			cli.Out().SetIsTerminal(tc.stdoutTty)
 			actual := stdioAttributes(cli)
 
-			assert.Check(t, reflect.DeepEqual(actual, tc.expected))
+			assert.Check(t, is.DeepEqual(actual, tc.expected, cmpopts.EquateComparable(attribute.Value{})))
 		})
 	}
 }
@@ -179,7 +180,7 @@ func TestAttributesFromError(t *testing.T) {
 		t.Run(tc.testName, func(t *testing.T) {
 			t.Parallel()
 			actual := attributesFromError(tc.err)
-			assert.Check(t, reflect.DeepEqual(actual, tc.expected))
+			assert.Check(t, is.DeepEqual(actual, tc.expected, cmpopts.EquateComparable(attribute.Value{})))
 		})
 	}
 }

cli/command/volume/create_test.go

@@ -1,10 +1,13 @@
+// FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16:
+//go:build go1.24
+
 package volume
 
 import (
 	"errors"
 	"fmt"
 	"io"
-	"reflect"
+	"maps"
 	"sort"
 	"strings"
 	"testing"
@@ -124,10 +127,10 @@ func TestVolumeCreateWithFlags(t *testing.T) {
 			if options.Driver != expectedDriver {
 				return client.VolumeCreateResult{}, fmt.Errorf("expected driver %q, got %q", expectedDriver, options.Driver)
 			}
-			if !reflect.DeepEqual(options.DriverOpts, expectedOpts) {
+			if !maps.Equal(options.DriverOpts, expectedOpts) {
 				return client.VolumeCreateResult{}, fmt.Errorf("expected drivers opts %v, got %v", expectedOpts, options.DriverOpts)
 			}
-			if !reflect.DeepEqual(options.Labels, expectedLabels) {
+			if !maps.Equal(options.Labels, expectedLabels) {
 				return client.VolumeCreateResult{}, fmt.Errorf("expected labels %v, got %v", expectedLabels, options.Labels)
 			}
 			return client.VolumeCreateResult{

cli/connhelper/connhelper_test.go

@@ -1,7 +1,6 @@
 package connhelper
 
 import (
-	"reflect"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -27,7 +26,8 @@ func TestSSHFlags(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		assert.DeepEqual(t, addSSHTimeout(tc.in), tc.out)
+		result := addSSHTimeout(tc.in)
+		assert.DeepEqual(t, result, tc.out)
 	}
 }
 
@@ -57,9 +57,7 @@ func TestDisablePseudoTerminalAllocation(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			result := disablePseudoTerminalAllocation(tc.sshFlags)
-			if !reflect.DeepEqual(result, tc.expected) {
-				t.Errorf("expected %v, got %v", tc.expected, result)
-			}
+			assert.DeepEqual(t, result, tc.expected)
 		})
 	}
 }