From caae3c051d65c1bf4bf12007322f7909828c2dc9 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 22 Nov 2024 12:12:30 +1100 Subject: [PATCH] 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 --- builder/remotecontext/git/gitutils_test.go | 4 +++- integration-cli/docker_cli_cp_utils_test.go | 9 +++++---- integration/build/build_test.go | 2 +- integration/container/copy_test.go | 2 +- integration/container/kill_test.go | 4 ++-- integration/network/ipvlan/ipvlan_test.go | 2 +- integration/network/macvlan/macvlan_test.go | 2 +- integration/plugin/authz/authz_plugin_v2_test.go | 13 ++++++------- pkg/authorization/middleware_unix_test.go | 3 +-- quota/projectquota_test.go | 2 +- volume/service/db_test.go | 3 +-- volume/service/store_test.go | 9 ++++----- 12 files changed, 27 insertions(+), 28 deletions(-) diff --git a/builder/remotecontext/git/gitutils_test.go b/builder/remotecontext/git/gitutils_test.go index d8dcdb085a263..923a6240eb02f 100644 --- 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 index 847d1100a6604..f28c6bf47eebf 100644 --- 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 index 28a6e8e02e3c3..487562ea0fe6d 100644 --- 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 index 045dff1d89a8f..d472490451f0b 100644 --- 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 index 1c6fadf11e7bf..27cf68bf415b3 100644 --- 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 index 78c460cee60a0..21776867643f1 100644 --- 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 index fa7b6854e7138..a516982c48643 100644 --- 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 index 9f05c004acde3..716375d4c5cc6 100644 --- 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 index 680baf83a2cd3..081d502944697 100644 --- 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 index 8857e607f5778..389b242d1cf4d 100644 --- 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 index cff19d5d0bd59..4fe71592e9978 100644 --- 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 index f6d4ac93363b8..049e8ca3b5738 100644 --- 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")