tests: migrate to assert.ErrorContains when possible

If we have an error type that we're checking a substring against, we should really be checking using ErrorContains to indicate the right semantics to assert. Mostly done using these transforms: find . -type f -name "*_test.go" | \ xargs gofmt -w -r 'assert.Assert(t, is.ErrorContains(e, s)) -> assert.ErrorContains(t, e, s)' find . -type f -name "*_test.go" | \ xargs gofmt -w -r 'assert.Assert(t, is.Contains(err.Error(), s)) -> assert.ErrorContains(t, err, s)' find . -type f -name "*_test.go" | \ xargs gofmt -w -r 'assert.Check(t, is.Contains(err.Error(), s)) -> assert.Check(t, is.ErrorContains(err, s))' As well as some small fixups to helpers that were doing strings.Contains explicitly. Signed-off-by: Aleksa Sarai <[email protected]>
robmry · Nov 22, 2024 · caae3c0 · caae3c0
1 parent 1b470d1
commit caae3c0
Show file tree

Hide file tree

Showing 12 changed files with 27 additions and 28 deletions.
diff --git a/builder/remotecontext/git/gitutils_test.go b/builder/remotecontext/git/gitutils_test.go
@@ -324,7 +324,7 @@ func TestCheckoutGit(t *testing.T) {
 				assert.Check(t, is.Equal("subcontents", string(b)))
 			} else {
 				_, err := os.Stat(filepath.Join(r, "sub/subfile"))
-				assert.Assert(t, is.ErrorContains(err, ""))
+				assert.ErrorContains(t, err, "")
 				assert.Assert(t, os.IsNotExist(err))
 			}
 
@@ -373,6 +373,8 @@ func TestGitInvalidRef(t *testing.T) {
 	for _, url := range gitUrls {
 		_, err := Clone(url)
 		assert.Assert(t, err != nil)
+		// On Windows, git has different case for the "invalid refspec" error,
+		// so we can't use ErrorContains.
 		assert.Check(t, is.Contains(strings.ToLower(err.Error()), "invalid refspec"))
 	}
 }
diff --git a/integration-cli/docker_cli_cp_utils_test.go b/integration-cli/docker_cli_cp_utils_test.go
@@ -14,6 +14,7 @@ import (
 	"github.com/docker/docker/integration-cli/cli"
 	"github.com/docker/docker/pkg/archive"
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 )
 
 type fileType uint32
@@ -225,12 +226,12 @@ func getTestDir(c *testing.T, label string) (tmpDir string) {
 	return
 }
 
-func isCpDirNotExist(err error) bool {
-	return strings.Contains(err.Error(), archive.ErrDirNotExists.Error())
+func isCpDirNotExist(err error) is.Comparison {
+	return is.ErrorContains(err, archive.ErrDirNotExists.Error())
 }
 
-func isCpCannotCopyDir(err error) bool {
-	return strings.Contains(err.Error(), archive.ErrCannotCopyDir.Error())
+func isCpCannotCopyDir(err error) is.Comparison {
+	return is.ErrorContains(err, archive.ErrCannotCopyDir.Error())
 }
 
 func fileContentEquals(c *testing.T, filename, contents string) error {

diff --git a/integration/build/build_test.go b/integration/build/build_test.go
@@ -627,7 +627,7 @@ func TestBuildWithEmptyDockerfile(t *testing.T) {
 					ForceRemove: true,
 				})
 
-			assert.Check(t, is.Contains(err.Error(), tc.expectedErr))
+			assert.Check(t, is.ErrorContains(err, tc.expectedErr))
 		})
 	}
 }

diff --git a/integration/container/copy_test.go b/integration/container/copy_test.go
@@ -46,7 +46,7 @@ func TestCopyFromContainerPathIsNotDir(t *testing.T) {
 		expected = "The filename, directory name, or volume label syntax is incorrect."
 	}
 	_, _, err := apiClient.CopyFromContainer(ctx, cid, path)
-	assert.Assert(t, is.ErrorContains(err, expected))
+	assert.ErrorContains(t, err, expected)
 }
 
 func TestCopyToContainerPathDoesNotExist(t *testing.T) {

diff --git a/integration/container/kill_test.go b/integration/container/kill_test.go
@@ -126,8 +126,8 @@ func TestKillStoppedContainer(t *testing.T) {
 	apiClient := testEnv.APIClient()
 	id := container.Create(ctx, t, apiClient)
 	err := apiClient.ContainerKill(ctx, id, "SIGKILL")
-	assert.Assert(t, is.ErrorContains(err, ""))
-	assert.Assert(t, is.Contains(err.Error(), "is not running"))
+	assert.ErrorContains(t, err, "")
+	assert.ErrorContains(t, err, "is not running")
 }
 
 func TestKillDifferentUserContainer(t *testing.T) {

diff --git a/integration/network/ipvlan/ipvlan_test.go b/integration/network/ipvlan/ipvlan_test.go
@@ -452,7 +452,7 @@ func testIpvlanExperimentalV4Only(t *testing.T, ctx context.Context, client dcli
 		net.WithIPv4(false),
 	)
 	defer client.NetworkRemove(ctx, "testnet")
-	assert.Assert(t, is.ErrorContains(err, "IPv4 can only be disabled if experimental features are enabled"))
+	assert.ErrorContains(t, err, "IPv4 can only be disabled if experimental features are enabled")
 }
 
 // Check that an ipvlan interface with '--ipv6=false' doesn't get kernel-assigned

diff --git a/integration/network/macvlan/macvlan_test.go b/integration/network/macvlan/macvlan_test.go
@@ -448,7 +448,7 @@ func testMacvlanExperimentalV4Only(t *testing.T, ctx context.Context, client cli
 		net.WithIPv4(false),
 	)
 	defer client.NetworkRemove(ctx, "testnet")
-	assert.Assert(t, is.ErrorContains(err, "IPv4 can only be disabled if experimental features are enabled"))
+	assert.ErrorContains(t, err, "IPv4 can only be disabled if experimental features are enabled")
 }
 
 // Check that a macvlan interface with '--ipv6=false' doesn't get kernel-assigned

diff --git a/integration/plugin/authz/authz_plugin_v2_test.go b/integration/plugin/authz/authz_plugin_v2_test.go
@@ -15,7 +15,6 @@ import (
 	"github.com/docker/docker/integration/internal/container"
 	"github.com/docker/docker/integration/internal/requirement"
 	"gotest.tools/v3/assert"
-	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/skip"
 )
 
@@ -72,7 +71,7 @@ func TestAuthZPluginV2Disable(t *testing.T) {
 
 	_, err = c.VolumeCreate(ctx, volume.CreateOptions{Driver: "local"})
 	assert.Assert(t, err != nil)
-	assert.Assert(t, is.Contains(err.Error(), fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag)))
+	assert.ErrorContains(t, err, fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))
 
 	// disable the plugin
 	err = c.PluginDisable(ctx, authzPluginNameWithTag, types.PluginDisableOptions{})
@@ -98,24 +97,24 @@ func TestAuthZPluginV2RejectVolumeRequests(t *testing.T) {
 
 	_, err = c.VolumeCreate(ctx, volume.CreateOptions{Driver: "local"})
 	assert.Assert(t, err != nil)
-	assert.Assert(t, is.Contains(err.Error(), fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag)))
+	assert.ErrorContains(t, err, fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))
 
 	_, err = c.VolumeList(ctx, volume.ListOptions{})
 	assert.Assert(t, err != nil)
-	assert.Assert(t, is.Contains(err.Error(), fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag)))
+	assert.ErrorContains(t, err, fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))
 
 	// The plugin will block the command before it can determine the volume does not exist
 	err = c.VolumeRemove(ctx, "test", false)
 	assert.Assert(t, err != nil)
-	assert.Assert(t, is.Contains(err.Error(), fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag)))
+	assert.ErrorContains(t, err, fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))
 
 	_, err = c.VolumeInspect(ctx, "test")
 	assert.Assert(t, err != nil)
-	assert.Assert(t, is.Contains(err.Error(), fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag)))
+	assert.ErrorContains(t, err, fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))
 
 	_, err = c.VolumesPrune(ctx, filters.Args{})
 	assert.Assert(t, err != nil)
-	assert.Assert(t, is.Contains(err.Error(), fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag)))
+	assert.ErrorContains(t, err, fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))
 }
 
 func TestAuthZPluginV2BadManifestFailsDaemonStart(t *testing.T) {

diff --git a/pkg/authorization/middleware_unix_test.go b/pkg/authorization/middleware_unix_test.go
@@ -10,7 +10,6 @@ import (
 
 	"github.com/docker/docker/pkg/plugingetter"
 	"gotest.tools/v3/assert"
-	is "gotest.tools/v3/assert/cmp"
 )
 
 func TestMiddlewareWrapHandler(t *testing.T) {
@@ -47,7 +46,7 @@ func TestMiddlewareWrapHandler(t *testing.T) {
 			Msg:   "Server Auth Not Allowed",
 		}
 		if err := mdHandler(ctx, resp, req, map[string]string{}); err == nil {
-			assert.Assert(t, is.ErrorContains(err, ""))
+			assert.ErrorContains(t, err, "")
 		}
 	})
 

diff --git a/quota/projectquota_test.go b/quota/projectquota_test.go
@@ -61,7 +61,7 @@ func testBiggerThanQuota(t *testing.T, ctrl *Control, homeDir, testDir, testSubD
 
 	biggerThanQuotaFile := filepath.Join(testSubDir, "bigger-than-quota")
 	err := os.WriteFile(biggerThanQuotaFile, make([]byte, testQuotaSize+1), 0o644)
-	assert.Assert(t, is.ErrorContains(err, ""))
+	assert.ErrorContains(t, err, "")
 	if err == io.ErrShortWrite {
 		assert.NilError(t, os.Remove(biggerThanQuotaFile))
 	}

diff --git a/volume/service/db_test.go b/volume/service/db_test.go
@@ -8,7 +8,6 @@ import (
 
 	bolt "go.etcd.io/bbolt"
 	"gotest.tools/v3/assert"
-	is "gotest.tools/v3/assert/cmp"
 )
 
 func TestSetGetMeta(t *testing.T) {
@@ -25,7 +24,7 @@ func TestSetGetMeta(t *testing.T) {
 	defer store.Shutdown()
 
 	_, err = store.getMeta("test")
-	assert.Assert(t, is.ErrorContains(err, ""))
+	assert.ErrorContains(t, err, "")
 
 	err = db.Update(func(tx *bolt.Tx) error {
 		_, err := tx.CreateBucket(volumeBucketName)

diff --git a/volume/service/store_test.go b/volume/service/store_test.go
@@ -15,7 +15,6 @@ import (
 	volumetestutils "github.com/docker/docker/volume/testutils"
 	"github.com/google/go-cmp/cmp"
 	"gotest.tools/v3/assert"
-	is "gotest.tools/v3/assert/cmp"
 )
 
 func TestCreate(t *testing.T) {
@@ -300,7 +299,7 @@ func TestRefDerefRemove(t *testing.T) {
 	assert.NilError(t, err)
 
 	err = s.Remove(ctx, v)
-	assert.Assert(t, is.ErrorContains(err, ""))
+	assert.ErrorContains(t, err, "")
 	assert.Equal(t, errVolumeInUse, err.(*OpErr).Err)
 
 	s.Release(ctx, v.Name(), "test-ref")
@@ -318,7 +317,7 @@ func TestGet(t *testing.T) {
 
 	ctx := context.Background()
 	_, err := s.Get(ctx, "not-exist")
-	assert.Assert(t, is.ErrorContains(err, ""))
+	assert.ErrorContains(t, err, "")
 	assert.Equal(t, errNoSuchVolume, err.(*OpErr).Err)
 
 	v1, err := s.Create(ctx, "test", driverName, opts.WithCreateLabels(map[string]string{"a": "1"}))
@@ -345,7 +344,7 @@ func TestGetWithReference(t *testing.T) {
 
 	ctx := context.Background()
 	_, err := s.Get(ctx, "not-exist", opts.WithGetDriver(driverName), opts.WithGetReference("test-ref"))
-	assert.Assert(t, is.ErrorContains(err, ""))
+	assert.ErrorContains(t, err, "")
 
 	v1, err := s.Create(ctx, "test", driverName, opts.WithCreateLabels(map[string]string{"a": "1"}))
 	assert.NilError(t, err)
@@ -355,7 +354,7 @@ func TestGetWithReference(t *testing.T) {
 	assert.DeepEqual(t, v1, v2, cmpVolume)
 
 	err = s.Remove(ctx, v2)
-	assert.Assert(t, is.ErrorContains(err, ""))
+	assert.ErrorContains(t, err, "")
 	assert.Equal(t, errVolumeInUse, err.(*OpErr).Err)
 
 	s.Release(ctx, v2.Name(), "test-ref")
-Original file line number
+Diff line change
@@ Expand Up / @@ -627,7 +627,7 @@ func TestBuildWithEmptyDockerfile(t *testing.T) { @@
     					ForceRemove: true,
     				})
-    			assert.Check(t, is.Contains(err.Error(), tc.expectedErr))
+    			assert.Check(t, is.ErrorContains(err, tc.expectedErr))
     		})
     	}
     }
@@ Expand Down @@