From 5155cda716ad7f519e608c380e43d1ca90f855c0 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 21 Dec 2017 16:27:57 -0500 Subject: [PATCH] Post migration fixes Fix tests that failed when using cmp.Compare() internal/test/testutil/assert InDelta Fix DeepEqual with kube metav1.Time Convert some ErrorContains to assert Signed-off-by: Daniel Nephin --- cli/command/container/attach_test.go | 13 ++- cli/command/container/create_test.go | 6 +- cli/command/container/exec_test.go | 2 +- cli/command/container/opts_test.go | 2 +- cli/command/container/stats_helpers_test.go | 17 ++- cli/command/formatter/checkpoint_test.go | 8 +- cli/command/formatter/config_test.go | 2 +- cli/command/formatter/search_test.go | 6 +- cli/command/formatter/stack_test.go | 2 +- cli/command/image/build_test.go | 3 +- cli/command/network/list_test.go | 17 +-- cli/command/service/ps_test.go | 36 +++--- cli/command/stack/kubernetes/loader_test.go | 15 ++- cli/command/stack/ps_test.go | 1 - .../stack/swarm/deploy_bundlefile_test.go | 20 ++-- cli/command/system/info_test.go | 2 +- cli/command/system/version_test.go | 13 ++- cli/command/trust/key_generate_test.go | 12 +- cli/command/trust/key_load_test.go | 16 +-- cli/command/trust/sign_test.go | 5 +- cli/compose/loader/loader_test.go | 108 +++++++----------- cli/compose/schema/schema_test.go | 7 +- cli/compose/template/template_test.go | 10 +- cli/manifest/store/store_test.go | 25 ++-- cli/required_test.go | 5 +- internal/test/testutil/assert.go | 14 ++- kubernetes/check_test.go | 2 +- templates/templates_test.go | 2 +- 28 files changed, 177 insertions(+), 194 deletions(-) diff --git a/cli/command/container/attach_test.go b/cli/command/container/attach_test.go index 1a4e015922..0d519de65b 100644 --- a/cli/command/container/attach_test.go +++ b/cli/command/container/attach_test.go @@ -7,7 +7,6 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/internal/test" - "github.com/docker/cli/internal/test/testutil" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/gotestyourself/gotestyourself/assert" @@ -75,7 +74,7 @@ func TestNewAttachCommandErrors(t *testing.T) { cmd := NewAttachCommand(test.NewFakeCli(&fakeClient{inspectFunc: tc.containerInspectFunc})) cmd.SetOutput(ioutil.Discard) cmd.SetArgs(tc.args) - testutil.ErrorContains(t, cmd.Execute(), tc.expectedError) + assert.ErrorContains(t, cmd.Execute(), tc.expectedError) } } @@ -102,9 +101,7 @@ func TestGetExitStatus(t *testing.T) { }, { result: &container.ContainerWaitOKBody{ - Error: &container.ContainerWaitOKBodyError{ - expectedErr.Error(), - }, + Error: &container.ContainerWaitOKBodyError{expectedErr.Error()}, }, expectedError: expectedErr, }, @@ -124,6 +121,10 @@ func TestGetExitStatus(t *testing.T) { resultC <- *testcase.result } err := getExitStatus(errC, resultC) - assert.Check(t, is.DeepEqual(testcase.expectedError, err)) + if testcase.expectedError == nil { + assert.Check(t, err) + } else { + assert.Check(t, is.Error(err, testcase.expectedError.Error())) + } } } diff --git a/cli/command/container/create_test.go b/cli/command/container/create_test.go index 3522147b3e..32beaa06ab 100644 --- a/cli/command/container/create_test.go +++ b/cli/command/container/create_test.go @@ -10,10 +10,10 @@ import ( "testing" "github.com/docker/cli/internal/test" - "github.com/docker/cli/internal/test/testutil" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" + "github.com/google/go-cmp/cmp" "github.com/gotestyourself/gotestyourself/assert" is "github.com/gotestyourself/gotestyourself/assert/cmp" "github.com/gotestyourself/gotestyourself/fs" @@ -23,7 +23,7 @@ import ( func TestCIDFileNoOPWithNoFilename(t *testing.T) { file, err := newCIDFile("") assert.NilError(t, err) - assert.Check(t, is.DeepEqual(&cidFile{}, file)) + assert.DeepEqual(t, &cidFile{}, file, cmp.AllowUnexported(cidFile{})) assert.Check(t, file.Write("id")) assert.Check(t, file.Close()) @@ -34,7 +34,7 @@ func TestNewCIDFileWhenFileAlreadyExists(t *testing.T) { defer tempfile.Remove() _, err := newCIDFile(tempfile.Path()) - testutil.ErrorContains(t, err, "Container ID file found") + assert.ErrorContains(t, err, "Container ID file found") } func TestCIDFileCloseWithNoWrite(t *testing.T) { diff --git a/cli/command/container/exec_test.go b/cli/command/container/exec_test.go index e6f82234ff..9e61eaa502 100644 --- a/cli/command/container/exec_test.go +++ b/cli/command/container/exec_test.go @@ -198,7 +198,7 @@ func TestGetExecExitStatus(t *testing.T) { }, } err := getExecExitStatus(context.Background(), client, execID) - assert.Check(t, is.DeepEqual(testcase.expectedError, err)) + assert.Check(t, is.Equal(testcase.expectedError, err)) } } diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index 31b7d12c81..cfb53425e6 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -67,7 +67,7 @@ func setupRunFlags() (*pflag.FlagSet, *containerOptions) { func parseMustError(t *testing.T, args string) { _, _, _, err := parseRun(strings.Split(args+" ubuntu bash", " ")) - assert.Check(t, is.ErrorContains(err, ""), args) + assert.ErrorContains(t, err, "", args) } func mustParse(t *testing.T, args string) (*container.Config, *container.HostConfig) { diff --git a/cli/command/container/stats_helpers_test.go b/cli/command/container/stats_helpers_test.go index 17de93e8cf..099f425d49 100644 --- a/cli/command/container/stats_helpers_test.go +++ b/cli/command/container/stats_helpers_test.go @@ -1,6 +1,7 @@ package container import ( + "fmt" "testing" "github.com/docker/docker/api/types" @@ -15,7 +16,7 @@ func TestCalculateMemUsageUnixNoCache(t *testing.T) { result := calculateMemUsageUnixNoCache(stats) // Then - assert.InDelta(t, 100.0, result, 1e-6) + assert.Assert(t, inDelta(100.0, result, 1e-6)) } func TestCalculateMemPercentUnixNoCache(t *testing.T) { @@ -27,10 +28,20 @@ func TestCalculateMemPercentUnixNoCache(t *testing.T) { // When and Then t.Run("Limit is set", func(t *testing.T) { result := calculateMemPercentUnixNoCache(someLimit, used) - assert.InDelta(t, 70.0, result, 1e-6) + assert.Assert(t, inDelta(70.0, result, 1e-6)) }) t.Run("No limit, no cgroup data", func(t *testing.T) { result := calculateMemPercentUnixNoCache(noLimit, used) - assert.InDelta(t, 0.0, result, 1e-6) + assert.Assert(t, inDelta(0.0, result, 1e-6)) }) } + +func inDelta(x, y, delta float64) func() (bool, string) { + return func() (bool, string) { + diff := x - y + if diff < -delta || diff > delta { + return false, fmt.Sprintf("%f != %f within %f", x, y, delta) + } + return true, "" + } +} diff --git a/cli/command/formatter/checkpoint_test.go b/cli/command/formatter/checkpoint_test.go index c592c4d499..26c517d580 100644 --- a/cli/command/formatter/checkpoint_test.go +++ b/cli/command/formatter/checkpoint_test.go @@ -6,7 +6,6 @@ import ( "github.com/docker/docker/api/types" "github.com/gotestyourself/gotestyourself/assert" - is "github.com/gotestyourself/gotestyourself/assert/cmp" ) func TestCheckpointContextFormatWrite(t *testing.T) { @@ -47,10 +46,7 @@ checkpoint-3: out := bytes.NewBufferString("") testcase.context.Output = out err := CheckpointWrite(testcase.context, checkpoints) - if err != nil { - assert.Check(t, is.ErrorContains(err, ""), testcase.expected) - } else { - assert.Check(t, is.Equal(out.String(), testcase.expected)) - } + assert.NilError(t, err) + assert.Equal(t, out.String(), testcase.expected) } } diff --git a/cli/command/formatter/config_test.go b/cli/command/formatter/config_test.go index 566a7d0123..3a5efb80d2 100644 --- a/cli/command/formatter/config_test.go +++ b/cli/command/formatter/config_test.go @@ -56,7 +56,7 @@ id_rsa out := bytes.NewBufferString("") testcase.context.Output = out if err := ConfigWrite(testcase.context, configs); err != nil { - assert.Check(t, is.ErrorContains(err, ""), testcase.expected) + assert.ErrorContains(t, err, testcase.expected) } else { assert.Check(t, is.Equal(out.String(), testcase.expected)) } diff --git a/cli/command/formatter/search_test.go b/cli/command/formatter/search_test.go index b6f6b97bda..e5a2d1a91a 100644 --- a/cli/command/formatter/search_test.go +++ b/cli/command/formatter/search_test.go @@ -155,7 +155,7 @@ result2 5 testcase.context.Output = out err := SearchWrite(testcase.context, results, false, 0) if err != nil { - assert.Check(t, is.ErrorContains(err, ""), testcase.expected) + assert.Check(t, is.ErrorContains(err, testcase.expected)) } else { assert.Check(t, is.Equal(out.String(), testcase.expected)) } @@ -192,7 +192,7 @@ result2 testcase.context.Output = out err := SearchWrite(testcase.context, results, true, 0) if err != nil { - assert.Check(t, is.ErrorContains(err, ""), testcase.expected) + assert.Check(t, is.ErrorContains(err, testcase.expected)) } else { assert.Check(t, is.Equal(out.String(), testcase.expected)) } @@ -227,7 +227,7 @@ result1 testcase.context.Output = out err := SearchWrite(testcase.context, results, false, 6) if err != nil { - assert.Check(t, is.ErrorContains(err, ""), testcase.expected) + assert.Check(t, is.ErrorContains(err, testcase.expected)) } else { assert.Check(t, is.Equal(out.String(), testcase.expected)) } diff --git a/cli/command/formatter/stack_test.go b/cli/command/formatter/stack_test.go index 7c04803a44..3fa73fefe0 100644 --- a/cli/command/formatter/stack_test.go +++ b/cli/command/formatter/stack_test.go @@ -57,7 +57,7 @@ bar testcase.context.Output = out err := StackWrite(testcase.context, stacks) if err != nil { - assert.Check(t, is.ErrorContains(err, ""), testcase.expected) + assert.Check(t, is.ErrorContains(err, testcase.expected)) } else { assert.Check(t, is.Equal(out.String(), testcase.expected)) } diff --git a/cli/command/image/build_test.go b/cli/command/image/build_test.go index ae59a5fc5d..38451c6ea9 100644 --- a/cli/command/image/build_test.go +++ b/cli/command/image/build_test.go @@ -166,8 +166,7 @@ func TestRunBuildFromGitHubSpecialCase(t *testing.T) { cmd.SetArgs([]string{"github.com/docker/no-such-repository"}) cmd.SetOutput(ioutil.Discard) err := cmd.Execute() - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "unable to prepare context: unable to 'git clone'")) + assert.ErrorContains(t, err, "unable to prepare context: unable to 'git clone'") } // TestRunBuildFromLocalGitHubDirNonExistingRepo tests that a local directory diff --git a/cli/command/network/list_test.go b/cli/command/network/list_test.go index 023e04090f..4c1a437c03 100644 --- a/cli/command/network/list_test.go +++ b/cli/command/network/list_test.go @@ -1,17 +1,15 @@ package network import ( - "testing" - "io/ioutil" - "strings" + "testing" "github.com/docker/cli/internal/test" . "github.com/docker/cli/internal/test/builders" - "github.com/docker/cli/internal/test/testutil" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" + "github.com/google/go-cmp/cmp" "github.com/gotestyourself/gotestyourself/assert" is "github.com/gotestyourself/gotestyourself/assert/cmp" "github.com/gotestyourself/gotestyourself/golden" @@ -39,23 +37,18 @@ func TestNetworkListErrors(t *testing.T) { }), ) cmd.SetOutput(ioutil.Discard) - testutil.ErrorContains(t, cmd.Execute(), tc.expectedError) - + assert.ErrorContains(t, cmd.Execute(), tc.expectedError) } } func TestNetworkListWithFlags(t *testing.T) { - - filterArgs := filters.NewArgs() - filterArgs.Add("image.name", "ubuntu") - expectedOpts := types.NetworkListOptions{ - Filters: filterArgs, + Filters: filters.NewArgs(filters.Arg("image.name", "ubuntu")), } cli := test.NewFakeCli(&fakeClient{ networkListFunc: func(ctx context.Context, options types.NetworkListOptions) ([]types.NetworkResource, error) { - assert.Check(t, is.DeepEqual(expectedOpts, options), "not expected options error") + assert.Check(t, is.DeepEqual(expectedOpts, options, cmp.AllowUnexported(filters.Args{}))) return []types.NetworkResource{*NetworkResource(NetworkResourceID("123454321"), NetworkResourceName("network_1"), NetworkResourceDriver("09.7.01"), diff --git a/cli/command/service/ps_test.go b/cli/command/service/ps_test.go index 7ea07883ef..0f50edc0d0 100644 --- a/cli/command/service/ps_test.go +++ b/cli/command/service/ps_test.go @@ -8,6 +8,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/swarm" + "github.com/google/go-cmp/cmp" "github.com/gotestyourself/gotestyourself/assert" is "github.com/gotestyourself/gotestyourself/assert/cmp" "golang.org/x/net/context" @@ -36,12 +37,13 @@ func TestCreateFilter(t *testing.T) { assert.NilError(t, err) assert.Check(t, is.DeepEqual(notfound, []string{"no such service: notfound"})) - expected := filters.NewArgs() - expected.Add("service", "idmatch") - expected.Add("service", "idprefixmatch") - expected.Add("service", "cccccccc") - expected.Add("node", "somenode") - assert.Check(t, is.DeepEqual(expected, actual)) + expected := filters.NewArgs( + filters.Arg("service", "idmatch"), + filters.Arg("service", "idprefixmatch"), + filters.Arg("service", "cccccccc"), + filters.Arg("node", "somenode"), + ) + assert.DeepEqual(t, expected, actual, cmpFilters) } func TestCreateFilterWithAmbiguousIDPrefixError(t *testing.T) { @@ -108,10 +110,11 @@ func TestRunPSQuiet(t *testing.T) { func TestUpdateNodeFilter(t *testing.T) { selfNodeID := "foofoo" - filter := filters.NewArgs() - filter.Add("node", "one") - filter.Add("node", "two") - filter.Add("node", "self") + filter := filters.NewArgs( + filters.Arg("node", "one"), + filters.Arg("node", "two"), + filters.Arg("node", "self"), + ) client := &fakeClient{ infoFunc: func(_ context.Context) (types.Info, error) { @@ -121,9 +124,12 @@ func TestUpdateNodeFilter(t *testing.T) { updateNodeFilter(context.Background(), client, filter) - expected := filters.NewArgs() - expected.Add("node", "one") - expected.Add("node", "two") - expected.Add("node", selfNodeID) - assert.Check(t, is.DeepEqual(expected, filter)) + expected := filters.NewArgs( + filters.Arg("node", "one"), + filters.Arg("node", "two"), + filters.Arg("node", selfNodeID), + ) + assert.DeepEqual(t, expected, filter, cmpFilters) } + +var cmpFilters = cmp.AllowUnexported(filters.Args{}) diff --git a/cli/command/stack/kubernetes/loader_test.go b/cli/command/stack/kubernetes/loader_test.go index 8e13dbc428..84a53da6fe 100644 --- a/cli/command/stack/kubernetes/loader_test.go +++ b/cli/command/stack/kubernetes/loader_test.go @@ -5,6 +5,7 @@ import ( composetypes "github.com/docker/cli/cli/compose/types" apiv1beta1 "github.com/docker/cli/kubernetes/compose/v1beta1" + "github.com/google/go-cmp/cmp" "github.com/gotestyourself/gotestyourself/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -30,7 +31,7 @@ func TestLoadStack(t *testing.T) { Name: "foo", }, Spec: apiv1beta1.StackSpec{ - ComposeFile: string(`version: "3.1" + ComposeFile: `version: "3.1" services: bar: image: bar @@ -40,7 +41,15 @@ networks: {} volumes: {} secrets: {} configs: {} -`), +`, }, - }, s) + }, s, cmpKubeAPITime) } + +// TODO: this can be removed when k8s.io/apimachinery is updated to > 1.9.0 +var cmpKubeAPITime = cmp.Comparer(func(x, y *metav1.Time) bool { + if x == nil || y == nil { + return x == y + } + return x.Time.Equal(y.Time) +}) diff --git a/cli/command/stack/ps_test.go b/cli/command/stack/ps_test.go index de19f3958a..36813ca501 100644 --- a/cli/command/stack/ps_test.go +++ b/cli/command/stack/ps_test.go @@ -62,7 +62,6 @@ func TestStackPsEmptyStack(t *testing.T) { cmd.SetArgs([]string{"foo"}) cmd.SetOutput(ioutil.Discard) - assert.Check(t, is.ErrorContains(cmd.Execute(), "")) assert.Check(t, is.Error(cmd.Execute(), "nothing found in stack: foo")) assert.Check(t, is.Equal("", fakeCli.OutBuffer().String())) } diff --git a/cli/command/stack/swarm/deploy_bundlefile_test.go b/cli/command/stack/swarm/deploy_bundlefile_test.go index a7636aa222..cc1535fc9b 100644 --- a/cli/command/stack/swarm/deploy_bundlefile_test.go +++ b/cli/command/stack/swarm/deploy_bundlefile_test.go @@ -2,7 +2,6 @@ package swarm import ( "bytes" - "fmt" "path/filepath" "testing" @@ -14,27 +13,28 @@ func TestLoadBundlefileErrors(t *testing.T) { testCases := []struct { namespace string path string - expectedError error + expectedError string }{ { namespace: "namespace_foo", - expectedError: fmt.Errorf("Bundle %s.dab not found", "namespace_foo"), + expectedError: "Bundle namespace_foo.dab not found", }, { namespace: "namespace_foo", path: "invalid_path", - expectedError: fmt.Errorf("Bundle %s not found", "invalid_path"), - }, - { - namespace: "namespace_foo", - path: filepath.Join("testdata", "bundlefile_with_invalid_syntax"), - expectedError: fmt.Errorf("Error reading"), + expectedError: "Bundle invalid_path not found", }, + // FIXME: this test never working, testdata file is missing from repo + //{ + // namespace: "namespace_foo", + // path: string(golden.Get(t, "bundlefile_with_invalid_syntax")), + // expectedError: "Error reading", + //}, } for _, tc := range testCases { _, err := loadBundlefile(&bytes.Buffer{}, tc.namespace, tc.path) - assert.Check(t, is.ErrorContains(err, ""), tc.expectedError) + assert.ErrorContains(t, err, tc.expectedError) } } diff --git a/cli/command/system/info_test.go b/cli/command/system/info_test.go index a1fd3d6980..7e2dd6bff8 100644 --- a/cli/command/system/info_test.go +++ b/cli/command/system/info_test.go @@ -227,7 +227,7 @@ func TestPrettyPrintInfo(t *testing.T) { }, } { cli := test.NewFakeCli(&fakeClient{}) - assert.Check(t, prettyPrintInfo(cli, tc.dockerInfo)) + assert.NilError(t, prettyPrintInfo(cli, tc.dockerInfo)) golden.Assert(t, cli.OutBuffer().String(), tc.expectedGolden+".golden") if tc.warningsGolden != "" { golden.Assert(t, cli.ErrBuffer().String(), tc.warningsGolden+".golden") diff --git a/cli/command/system/version_test.go b/cli/command/system/version_test.go index d0e72f6cdc..c704ce2051 100644 --- a/cli/command/system/version_test.go +++ b/cli/command/system/version_test.go @@ -23,12 +23,15 @@ func TestVersionWithoutServer(t *testing.T) { }) cmd := NewVersionCommand(cli) cmd.SetOutput(cli.Err()) - assert.Check(t, is.ErrorContains(cmd.Execute(), "")) - assert.Check(t, is.Contains(cleanTabs(cli.OutBuffer().String()), "Client:")) - assert.NotContains(t, cleanTabs(cli.OutBuffer().String()), "Server:") + assert.ErrorContains(t, cmd.Execute(), "no server") + out := cli.OutBuffer().String() + // TODO: use an assertion like e2e/image/build_test.go:assertBuildOutput() + // instead of contains/not contains + assert.Check(t, is.Contains(out, "Client:")) + assert.Assert(t, !strings.Contains(out, "Server:"), "actual: %s", out) } -func fakeServerVersion(ctx context.Context) (types.Version, error) { +func fakeServerVersion(_ context.Context) (types.Version, error) { return types.Version{ Version: "docker-dev", APIVersion: api.DefaultVersion, @@ -39,7 +42,7 @@ func TestVersionWithOrchestrator(t *testing.T) { cli := test.NewFakeCli(&fakeClient{serverVersion: fakeServerVersion}) cli.SetClientInfo(func() command.ClientInfo { return command.ClientInfo{Orchestrator: "swarm"} }) cmd := NewVersionCommand(cli) - assert.Check(t, cmd.Execute()) + assert.NilError(t, cmd.Execute()) assert.Check(t, is.Contains(cleanTabs(cli.OutBuffer().String()), "Orchestrator: swarm")) } diff --git a/cli/command/trust/key_generate_test.go b/cli/command/trust/key_generate_test.go index d5bb6da1ea..8d7b97beae 100644 --- a/cli/command/trust/key_generate_test.go +++ b/cli/command/trust/key_generate_test.go @@ -121,19 +121,15 @@ func TestValidateKeyArgs(t *testing.T) { assert.Check(t, err) err = validateKeyArgs("a/b", pubKeyCWD) - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Equal(err.Error(), "key name \"a/b\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character")) + assert.Error(t, err, "key name \"a/b\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character") err = validateKeyArgs("-", pubKeyCWD) - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Equal(err.Error(), "key name \"-\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character")) + assert.Error(t, err, "key name \"-\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character") assert.Check(t, ioutil.WriteFile(filepath.Join(pubKeyCWD, "a.pub"), []byte("abc"), notary.PrivExecPerms)) err = validateKeyArgs("a", pubKeyCWD) - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Equal(err.Error(), fmt.Sprintf("public key file already exists: \"%s/a.pub\"", pubKeyCWD))) + assert.Error(t, err, fmt.Sprintf("public key file already exists: \"%s/a.pub\"", pubKeyCWD)) err = validateKeyArgs("a", "/random/dir/") - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Equal(err.Error(), "public key path does not exist: \"/random/dir/\"")) + assert.Error(t, err, "public key path does not exist: \"/random/dir/\"") } diff --git a/cli/command/trust/key_load_test.go b/cli/command/trust/key_load_test.go index ca4eb8c20c..e4da34e94f 100644 --- a/cli/command/trust/key_load_test.go +++ b/cli/command/trust/key_load_test.go @@ -184,22 +184,22 @@ func testLoadKeyTooPermissive(t *testing.T, privKeyFixture []byte) { // import the key to our keyStorageDir _, err = getPrivKeyBytesFromPath(privKeyFilepath) - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath), err.Error())) + expected := fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath) + assert.Error(t, err, expected) privKeyFilepath = filepath.Join(privKeyDir, "privkey667.pem") assert.Check(t, ioutil.WriteFile(privKeyFilepath, privKeyFixture, 0677)) _, err = getPrivKeyBytesFromPath(privKeyFilepath) - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath), err.Error())) + expected = fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath) + assert.Error(t, err, expected) privKeyFilepath = filepath.Join(privKeyDir, "privkey777.pem") assert.Check(t, ioutil.WriteFile(privKeyFilepath, privKeyFixture, 0777)) _, err = getPrivKeyBytesFromPath(privKeyFilepath) - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath), err.Error())) + expected = fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath) + assert.Error(t, err, expected) privKeyFilepath = filepath.Join(privKeyDir, "privkey400.pem") assert.Check(t, ioutil.WriteFile(privKeyFilepath, privKeyFixture, 0400)) @@ -240,6 +240,6 @@ func TestLoadPubKeyFailure(t *testing.T) { // import the key to our keyStorageDir - it should fail err = loadPrivKeyBytesToStore(pubKeyBytes, privKeyImporters, pubKeyFilepath, "signer-name", cannedPasswordRetriever) - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(fmt.Sprintf("provided file %s is not a supported private key - to add a signer's public key use docker trust signer add", pubKeyFilepath), err.Error())) + expected := fmt.Sprintf("provided file %s is not a supported private key - to add a signer's public key use docker trust signer add", pubKeyFilepath) + assert.Error(t, err, expected) } diff --git a/cli/command/trust/sign_test.go b/cli/command/trust/sign_test.go index 6af39a27f7..be6c27afb7 100644 --- a/cli/command/trust/sign_test.go +++ b/cli/command/trust/sign_test.go @@ -79,8 +79,7 @@ func TestTrustSignCommandOfflineErrors(t *testing.T) { cmd := newSignCommand(cli) cmd.SetArgs([]string{"reg-name.io/image:tag"}) cmd.SetOutput(ioutil.Discard) - assert.Check(t, is.ErrorContains(cmd.Execute(), "")) - testutil.ErrorContains(t, cmd.Execute(), "client is offline") + assert.ErrorContains(t, cmd.Execute(), "client is offline") } func TestGetOrGenerateNotaryKey(t *testing.T) { @@ -112,7 +111,7 @@ func TestGetOrGenerateNotaryKey(t *testing.T) { assert.Check(t, notaryRepo.GetCryptoService().GetKey(rootKeyB.ID()) != nil) // The key we retrieved should be identical to the one we generated - assert.Check(t, is.DeepEqual(rootKeyA, rootKeyB)) + assert.Check(t, is.DeepEqual(rootKeyA.Public(), rootKeyB.Public())) // Now also try with a delegation key releasesKey, err := getOrGenerateNotaryKey(notaryRepo, data.RoleName(trust.ReleasesRole)) diff --git a/cli/compose/loader/loader_test.go b/cli/compose/loader/loader_test.go index 5c53473769..93f87cefa8 100644 --- a/cli/compose/loader/loader_test.go +++ b/cli/compose/loader/loader_test.go @@ -4,6 +4,7 @@ import ( "bytes" "io/ioutil" "os" + "reflect" "sort" "testing" "time" @@ -225,16 +226,13 @@ func TestParseAndLoad(t *testing.T) { func TestInvalidTopLevelObjectType(t *testing.T) { _, err := loadYAML("1") - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "Top-level object must be a mapping")) + assert.ErrorContains(t, err, "Top-level object must be a mapping") _, err = loadYAML("\"hello\"") - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "Top-level object must be a mapping")) + assert.ErrorContains(t, err, "Top-level object must be a mapping") _, err = loadYAML("[\"hello\"]") - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "Top-level object must be a mapping")) + assert.ErrorContains(t, err, "Top-level object must be a mapping") } func TestNonStringKeys(t *testing.T) { @@ -244,8 +242,7 @@ version: "3" foo: image: busybox `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "Non-string key at top level: 123")) + assert.ErrorContains(t, err, "Non-string key at top level: 123") _, err = loadYAML(` version: "3" @@ -255,8 +252,7 @@ services: 123: image: busybox `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "Non-string key in services: 123")) + assert.ErrorContains(t, err, "Non-string key in services: 123") _, err = loadYAML(` version: "3" @@ -269,8 +265,7 @@ networks: config: - 123: oh dear `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "Non-string key in networks.default.ipam.config[0]: 123")) + assert.ErrorContains(t, err, "Non-string key in networks.default.ipam.config[0]: 123") _, err = loadYAML(` version: "3" @@ -280,8 +275,7 @@ services: environment: 1: FOO `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "Non-string key in services.dict-env.environment: 1")) + assert.ErrorContains(t, err, "Non-string key in services.dict-env.environment: 1") } func TestSupportedVersion(t *testing.T) { @@ -309,8 +303,7 @@ services: foo: image: busybox `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "version")) + assert.ErrorContains(t, err, "version") _, err = loadYAML(` version: "2.0" @@ -318,8 +311,7 @@ services: foo: image: busybox `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "version")) + assert.ErrorContains(t, err, "version") } func TestInvalidVersion(t *testing.T) { @@ -329,8 +321,7 @@ services: foo: image: busybox `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "version must be a string")) + assert.ErrorContains(t, err, "version must be a string") } func TestV1Unsupported(t *testing.T) { @@ -338,7 +329,7 @@ func TestV1Unsupported(t *testing.T) { foo: image: busybox `) - assert.Check(t, is.ErrorContains(err, "")) + assert.ErrorContains(t, err, "unsupported Compose file version: 1.0") } func TestNonMappingObject(t *testing.T) { @@ -348,16 +339,14 @@ services: - foo: image: busybox `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "services must be a mapping")) + assert.ErrorContains(t, err, "services must be a mapping") _, err = loadYAML(` version: "3" services: foo: busybox `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "services.foo must be a mapping")) + assert.ErrorContains(t, err, "services.foo must be a mapping") _, err = loadYAML(` version: "3" @@ -365,16 +354,14 @@ networks: - default: driver: bridge `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "networks must be a mapping")) + assert.ErrorContains(t, err, "networks must be a mapping") _, err = loadYAML(` version: "3" networks: default: bridge `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "networks.default must be a mapping")) + assert.ErrorContains(t, err, "networks.default must be a mapping") _, err = loadYAML(` version: "3" @@ -382,16 +369,14 @@ volumes: - data: driver: local `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "volumes must be a mapping")) + assert.ErrorContains(t, err, "volumes must be a mapping") _, err = loadYAML(` version: "3" volumes: data: local `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "volumes.data must be a mapping")) + assert.ErrorContains(t, err, "volumes.data must be a mapping") } func TestNonStringImage(t *testing.T) { @@ -401,8 +386,7 @@ services: foo: image: ["busybox", "latest"] `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "services.foo.image must be a string")) + assert.ErrorContains(t, err, "services.foo.image must be a string") } func TestLoadWithEnvironment(t *testing.T) { @@ -452,8 +436,7 @@ services: environment: FOO: ["1"] `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "services.dict-env.environment.FOO must be a string, number or null")) + assert.ErrorContains(t, err, "services.dict-env.environment.FOO must be a string, number or null") } func TestInvalidEnvironmentObject(t *testing.T) { @@ -464,8 +447,7 @@ services: image: busybox environment: "FOO=1" `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "services.dict-env.environment must be a mapping")) + assert.ErrorContains(t, err, "services.dict-env.environment must be a mapping") } func TestLoadWithEnvironmentInterpolation(t *testing.T) { @@ -736,11 +718,9 @@ services: service: foo `) - assert.Assert(t, is.ErrorContains(err, "")) - forbidden, ok := err.(*ForbiddenPropertiesError) - assert.Check(t, ok, "error type is %T instead of ForbiddenPropertiesError", err) + assert.ErrorType(t, err, reflect.TypeOf(&ForbiddenPropertiesError{})) - props := forbidden.Properties + props := err.(*ForbiddenPropertiesError).Properties assert.Check(t, is.Len(props, 2)) assert.Check(t, is.Contains(props, "volume_driver")) assert.Check(t, is.Contains(props, "extends")) @@ -757,8 +737,7 @@ func TestInvalidResource(t *testing.T) { impossible: x: 1 `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "Additional property impossible is not allowed")) + assert.ErrorContains(t, err, "Additional property impossible is not allowed") } func TestInvalidExternalAndDriverCombination(t *testing.T) { @@ -770,9 +749,8 @@ volumes: driver: foobar `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "conflicting parameters \"external\" and \"driver\" specified for volume")) - assert.Check(t, is.Contains(err.Error(), "external_volume")) + assert.ErrorContains(t, err, "conflicting parameters \"external\" and \"driver\" specified for volume") + assert.ErrorContains(t, err, "external_volume") } func TestInvalidExternalAndDirverOptsCombination(t *testing.T) { @@ -785,9 +763,8 @@ volumes: beep: boop `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "conflicting parameters \"external\" and \"driver_opts\" specified for volume")) - assert.Check(t, is.Contains(err.Error(), "external_volume")) + assert.ErrorContains(t, err, "conflicting parameters \"external\" and \"driver_opts\" specified for volume") + assert.ErrorContains(t, err, "external_volume") } func TestInvalidExternalAndLabelsCombination(t *testing.T) { @@ -800,9 +777,8 @@ volumes: - beep=boop `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "conflicting parameters \"external\" and \"labels\" specified for volume")) - assert.Check(t, is.Contains(err.Error(), "external_volume")) + assert.ErrorContains(t, err, "conflicting parameters \"external\" and \"labels\" specified for volume") + assert.ErrorContains(t, err, "external_volume") } func TestLoadVolumeInvalidExternalNameAndNameCombination(t *testing.T) { @@ -815,9 +791,8 @@ volumes: name: external_name `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "volume.external.name and volume.name conflict; only use volume.name")) - assert.Check(t, is.Contains(err.Error(), "external_volume")) + assert.ErrorContains(t, err, "volume.external.name and volume.name conflict; only use volume.name") + assert.ErrorContains(t, err, "external_volume") } func durationPtr(value time.Duration) *time.Duration { @@ -890,8 +865,7 @@ services: tmpfs: size: 10000 `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "services.tmpfs.volumes.0 Additional property tmpfs is not allowed")) + assert.ErrorContains(t, err, "services.tmpfs.volumes.0 Additional property tmpfs is not allowed") } func TestLoadBindMountSourceMustNotBeEmpty(t *testing.T) { @@ -971,8 +945,7 @@ services: tmpfs: size: -1 `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "services.tmpfs.volumes.0.tmpfs.size Must be greater than or equal to 0")) + assert.ErrorContains(t, err, "services.tmpfs.volumes.0.tmpfs.size Must be greater than or equal to 0") } func TestLoadTmpfsVolumeSizeMustBeInteger(t *testing.T) { @@ -987,8 +960,7 @@ services: tmpfs: size: 0.0001 `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "services.tmpfs.volumes.0.tmpfs.size must be a integer")) + assert.ErrorContains(t, err, "services.tmpfs.volumes.0.tmpfs.size must be a integer") } func serviceSort(services []types.ServiceConfig) []types.ServiceConfig { @@ -1295,9 +1267,8 @@ secrets: name: external_name `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "secret.external.name and secret.name conflict; only use secret.name")) - assert.Check(t, is.Contains(err.Error(), "external_secret")) + assert.ErrorContains(t, err, "secret.external.name and secret.name conflict; only use secret.name") + assert.ErrorContains(t, err, "external_secret") } func TestLoadSecretsWarnOnDeprecatedExternalNameVersion35(t *testing.T) { @@ -1383,7 +1354,6 @@ networks: name: external_name `) - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "network.external.name and network.name conflict; only use network.name")) - assert.Check(t, is.Contains(err.Error(), "foo")) + assert.ErrorContains(t, err, "network.external.name and network.name conflict; only use network.name") + assert.ErrorContains(t, err, "foo") } diff --git a/cli/compose/schema/schema_test.go b/cli/compose/schema/schema_test.go index 6b3f58a604..7dd9d296a9 100644 --- a/cli/compose/schema/schema_test.go +++ b/cli/compose/schema/schema_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/gotestyourself/gotestyourself/assert" - is "github.com/gotestyourself/gotestyourself/assert/cmp" ) type dict map[string]interface{} @@ -33,8 +32,7 @@ func TestValidateUndefinedTopLevelOption(t *testing.T) { } err := Validate(config, "3.0") - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "Additional property helicopters is not allowed")) + assert.ErrorContains(t, err, "Additional property helicopters is not allowed") } func TestValidateAllowsXTopLevelFields(t *testing.T) { @@ -77,8 +75,7 @@ func TestValidateInvalidVersion(t *testing.T) { } err := Validate(config, "2.1") - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "unsupported Compose file version: 2.1")) + assert.ErrorContains(t, err, "unsupported Compose file version: 2.1") } type array []interface{} diff --git a/cli/compose/template/template_test.go b/cli/compose/template/template_test.go index 20897109e0..7adef7b13a 100644 --- a/cli/compose/template/template_test.go +++ b/cli/compose/template/template_test.go @@ -1,9 +1,9 @@ package template import ( + "reflect" "testing" - "github.com/docker/cli/internal/test/testutil" "github.com/gotestyourself/gotestyourself/assert" is "github.com/gotestyourself/gotestyourself/assert/cmp" ) @@ -43,8 +43,7 @@ func TestInvalid(t *testing.T) { for _, template := range invalidTemplates { _, err := Substitute(template, defaultMapping) - assert.Check(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), "Invalid template")) + assert.ErrorContains(t, err, "Invalid template") } } @@ -119,9 +118,8 @@ func TestMandatoryVariableErrors(t *testing.T) { for _, tc := range testCases { _, err := Substitute(tc.template, defaultMapping) - assert.Check(t, is.ErrorContains(err, "")) - assert.IsType(t, &InvalidTemplateError{}, err) - testutil.ErrorContains(t, err, tc.expectedError) + assert.ErrorContains(t, err, tc.expectedError) + assert.ErrorType(t, err, reflect.TypeOf(&InvalidTemplateError{})) } } diff --git a/cli/manifest/store/store_test.go b/cli/manifest/store/store_test.go index 44f9c555d3..bd3ba419a7 100644 --- a/cli/manifest/store/store_test.go +++ b/cli/manifest/store/store_test.go @@ -7,6 +7,7 @@ import ( "github.com/docker/cli/cli/manifest/types" "github.com/docker/distribution/reference" + "github.com/google/go-cmp/cmp" "github.com/gotestyourself/gotestyourself/assert" is "github.com/gotestyourself/gotestyourself/assert/cmp" ) @@ -92,19 +93,23 @@ func TestStoreSaveAndGet(t *testing.T) { } for _, testcase := range testcases { - actual, err := store.Get(testcase.listRef, testcase.manifestRef) - if testcase.expectedErr != "" { - assert.Check(t, is.Error(err, testcase.expectedErr)) - assert.Check(t, IsNotFound(err)) - continue - } - if !assert.Check(t, err, testcase.manifestRef.String()) { - continue - } - assert.Check(t, is.DeepEqual(testcase.expected, actual), testcase.manifestRef.String()) + t.Run(testcase.manifestRef.String(), func(t *testing.T) { + actual, err := store.Get(testcase.listRef, testcase.manifestRef) + if testcase.expectedErr != "" { + assert.Check(t, is.Error(err, testcase.expectedErr)) + assert.Check(t, IsNotFound(err)) + return + } + assert.NilError(t, err) + assert.DeepEqual(t, testcase.expected, actual, cmpReferenceNamed) + }) } } +var cmpReferenceNamed = cmp.Transformer("namedref", func(r reference.Named) string { + return r.String() +}) + func TestStoreGetList(t *testing.T) { store, cleanup := newTestStore(t) defer cleanup() diff --git a/cli/required_test.go b/cli/required_test.go index 7b5e87f4d8..c82e3b965f 100644 --- a/cli/required_test.go +++ b/cli/required_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/gotestyourself/gotestyourself/assert" - is "github.com/gotestyourself/gotestyourself/assert/cmp" "github.com/spf13/cobra" ) @@ -123,9 +122,7 @@ func runTestCases(t *testing.T, testCases []testCase) { cmd.SetOutput(ioutil.Discard) err := cmd.Execute() - - assert.Assert(t, is.ErrorContains(err, ""), "Expected an error: %s", tc.expectedError) - assert.Check(t, is.Contains(err.Error(), tc.expectedError)) + assert.ErrorContains(t, err, tc.expectedError) } } diff --git a/internal/test/testutil/assert.go b/internal/test/testutil/assert.go index 7f9ca777fe..cc7cc886d5 100644 --- a/internal/test/testutil/assert.go +++ b/internal/test/testutil/assert.go @@ -2,14 +2,18 @@ package testutil import ( "github.com/gotestyourself/gotestyourself/assert" - is "github.com/gotestyourself/gotestyourself/assert/cmp" ) +type helperT interface { + Helper() +} + // ErrorContains checks that the error is not nil, and contains the expected // substring. -// TODO: replace with testify if https://github.com/stretchr/testify/pull/486 -// is accepted. +// Deprecated: use assert.Assert(t, cmp.ErrorContains(err, expected)) func ErrorContains(t assert.TestingT, err error, expectedError string) { - assert.Assert(t, is.ErrorContains(err, "")) - assert.Check(t, is.Contains(err.Error(), expectedError)) + if ht, ok := t.(helperT); ok { + ht.Helper() + } + assert.ErrorContains(t, err, expectedError) } diff --git a/kubernetes/check_test.go b/kubernetes/check_test.go index 129a982960..792dc3e6a5 100644 --- a/kubernetes/check_test.go +++ b/kubernetes/check_test.go @@ -22,7 +22,7 @@ func TestGetStackAPIVersion(t *testing.T) { for _, test := range tests { version, err := getAPIVersion(test.groups) if test.err { - assert.Assert(t, is.ErrorContains(err, "")) + assert.ErrorContains(t, err, "") } else { assert.NilError(t, err) } diff --git a/templates/templates_test.go b/templates/templates_test.go index a4ee115f28..66fafa2f07 100644 --- a/templates/templates_test.go +++ b/templates/templates_test.go @@ -82,7 +82,7 @@ func TestParseTruncateFunction(t *testing.T) { t.Run("Nil Source Test with template: "+testCase.template, func(t *testing.T) { var c bytes.Buffer - assert.Check(t, is.ErrorContains(tm.Execute(&c, nil), "")) + assert.Check(t, tm.Execute(&c, nil) != nil) assert.Check(t, is.Equal("", c.String())) }) }