From 2e266001c66eecc3f48f71cf28ac15ebf64ab5fb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 13:51:48 +0100 Subject: [PATCH 1/5] cli/command/volume: TestVolumeCreateErrors: assert unhandled errors Signed-off-by: Sebastiaan van Stijn --- cli/command/volume/create_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/command/volume/create_test.go b/cli/command/volume/create_test.go index 959050a28f..de879e81e2 100644 --- a/cli/command/volume/create_test.go +++ b/cli/command/volume/create_test.go @@ -48,7 +48,7 @@ func TestVolumeCreateErrors(t *testing.T) { ) cmd.SetArgs(tc.args) for key, value := range tc.flags { - cmd.Flags().Set(key, value) + assert.Check(t, cmd.Flags().Set(key, value)) } cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) From 8b5e5539e1619706c65b733fae8e03f31f5d9a8b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 14:00:59 +0100 Subject: [PATCH 2/5] cli/command/volume: TestVolumeCreateWithName: minor fixes and improvements - assert unhandled error - use sub-tests - add test-case for conflicting options (both flag and name) - reset command-args to prevent test failing when running from pre-compiled test-binary - use a const and a slightly more unique name for the volume-name - discard stdout/stderr output Signed-off-by: Sebastiaan van Stijn --- cli/command/volume/create_test.go | 42 ++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/cli/command/volume/create_test.go b/cli/command/volume/create_test.go index de879e81e2..2ffe222a9c 100644 --- a/cli/command/volume/create_test.go +++ b/cli/command/volume/create_test.go @@ -57,7 +57,7 @@ func TestVolumeCreateErrors(t *testing.T) { } func TestVolumeCreateWithName(t *testing.T) { - name := "foo" + const name = "my-volume-name" cli := test.NewFakeCli(&fakeClient{ volumeCreateFunc: func(body volume.CreateOptions) (volume.Volume, error) { if body.Name != name { @@ -70,19 +70,37 @@ func TestVolumeCreateWithName(t *testing.T) { }) buf := cli.OutBuffer() + t.Run("using-flags", func(t *testing.T) { + cmd := newCreateCommand(cli) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs([]string{}) + assert.Check(t, cmd.Flags().Set("name", name)) + assert.NilError(t, cmd.Execute()) + assert.Check(t, is.Equal(strings.TrimSpace(buf.String()), name)) + }) - // Test by flags - cmd := newCreateCommand(cli) - cmd.Flags().Set("name", name) - assert.NilError(t, cmd.Execute()) - assert.Check(t, is.Equal(name, strings.TrimSpace(buf.String()))) - - // Then by args buf.Reset() - cmd = newCreateCommand(cli) - cmd.SetArgs([]string{name}) - assert.NilError(t, cmd.Execute()) - assert.Check(t, is.Equal(name, strings.TrimSpace(buf.String()))) + t.Run("using-args", func(t *testing.T) { + cmd := newCreateCommand(cli) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs([]string{name}) + assert.NilError(t, cmd.Execute()) + assert.Check(t, is.Equal(strings.TrimSpace(buf.String()), name)) + }) + + buf.Reset() + t.Run("using-both", func(t *testing.T) { + cmd := newCreateCommand(cli) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs([]string{name}) + assert.Check(t, cmd.Flags().Set("name", name)) + err := cmd.Execute() + assert.Check(t, is.Error(err, `conflicting options: cannot specify a volume-name through both --name and as a positional arg`)) + assert.Check(t, is.Equal(strings.TrimSpace(buf.String()), "")) + }) } func TestVolumeCreateWithFlags(t *testing.T) { From a8265e72bf5c8f8797d993ec2380bd79438ead07 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 14:06:05 +0100 Subject: [PATCH 3/5] cli/command/volume: TestVolumeCreateWithFlags: minor fixes - assert unhandled error - reset command-args to prevent test failing when running from pre-compiled test-binary - use a const and a slightly more unique name for the volume-name - discard stdout/stderr output Signed-off-by: Sebastiaan van Stijn --- cli/command/volume/create_test.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/cli/command/volume/create_test.go b/cli/command/volume/create_test.go index 2ffe222a9c..7c256c3051 100644 --- a/cli/command/volume/create_test.go +++ b/cli/command/volume/create_test.go @@ -104,7 +104,8 @@ func TestVolumeCreateWithName(t *testing.T) { } func TestVolumeCreateWithFlags(t *testing.T) { - expectedDriver := "foo" + const name = "random-generated-name" + const expectedDriver = "foo-volume-driver" expectedOpts := map[string]string{ "bar": "1", "baz": "baz", @@ -113,7 +114,6 @@ func TestVolumeCreateWithFlags(t *testing.T) { "lbl1": "v1", "lbl2": "v2", } - name := "banana" cli := test.NewFakeCli(&fakeClient{ volumeCreateFunc: func(body volume.CreateOptions) (volume.Volume, error) { @@ -136,13 +136,16 @@ func TestVolumeCreateWithFlags(t *testing.T) { }) cmd := newCreateCommand(cli) - cmd.Flags().Set("driver", "foo") - cmd.Flags().Set("opt", "bar=1") - cmd.Flags().Set("opt", "baz=baz") - cmd.Flags().Set("label", "lbl1=v1") - cmd.Flags().Set("label", "lbl2=v2") + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs([]string{}) + assert.Check(t, cmd.Flags().Set("driver", expectedDriver)) + assert.Check(t, cmd.Flags().Set("opt", "bar=1")) + assert.Check(t, cmd.Flags().Set("opt", "baz=baz")) + assert.Check(t, cmd.Flags().Set("label", "lbl1=v1")) + assert.Check(t, cmd.Flags().Set("label", "lbl2=v2")) assert.NilError(t, cmd.Execute()) - assert.Check(t, is.Equal(name, strings.TrimSpace(cli.OutBuffer().String()))) + assert.Check(t, is.Equal(strings.TrimSpace(cli.OutBuffer().String()), name)) } func TestVolumeCreateCluster(t *testing.T) { From 5b8c08d19e3bda645058d66b0364569002ad577b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 14:17:25 +0100 Subject: [PATCH 4/5] cli/command/volume: TestVolumeCreateCluster: minor fixes and refactor - assert unhandled error - use sub-tests - use slightly more unique volume-names - discard stdout/stderr output Signed-off-by: Sebastiaan van Stijn --- cli/command/volume/create_test.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/cli/command/volume/create_test.go b/cli/command/volume/create_test.go index 7c256c3051..e1c5c18eaf 100644 --- a/cli/command/volume/create_test.go +++ b/cli/command/volume/create_test.go @@ -161,19 +161,27 @@ func TestVolumeCreateCluster(t *testing.T) { }, }) - cmd := newCreateCommand(cli) - cmd.Flags().Set("type", "block") - cmd.Flags().Set("group", "gronp") - cmd.Flags().Set("driver", "csi") - cmd.SetArgs([]string{"name"}) + t.Run("csi-volume", func(t *testing.T) { + cmd := newCreateCommand(cli) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + assert.Check(t, cmd.Flags().Set("type", "block")) + assert.Check(t, cmd.Flags().Set("group", "gronp")) + assert.Check(t, cmd.Flags().Set("driver", "csi")) + cmd.SetArgs([]string{"my-csi-volume"}) - assert.NilError(t, cmd.Execute()) + assert.NilError(t, cmd.Execute()) + }) - cmd = newCreateCommand(cli) - cmd.Flags().Set("driver", "notcsi") - cmd.SetArgs([]string{"name"}) + t.Run("non-csi-volume", func(t *testing.T) { + cmd := newCreateCommand(cli) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + assert.Check(t, cmd.Flags().Set("driver", "notcsi")) + cmd.SetArgs([]string{"my-non-csi-volume"}) - assert.NilError(t, cmd.Execute()) + assert.NilError(t, cmd.Execute()) + }) } func TestVolumeCreateClusterOpts(t *testing.T) { From 31b81982860c5edbb4c9ded68f66d8e2472ed3a2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 14:19:04 +0100 Subject: [PATCH 5/5] cli/command/volume: TestVolumeCreateClusterOpts: minor fixes and refactor - assert unhandled error - discard stdout/stderr output Signed-off-by: Sebastiaan van Stijn --- cli/command/volume/create_test.go | 34 ++++++++++++++++--------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/cli/command/volume/create_test.go b/cli/command/volume/create_test.go index e1c5c18eaf..9984f835cb 100644 --- a/cli/command/volume/create_test.go +++ b/cli/command/volume/create_test.go @@ -233,25 +233,27 @@ func TestVolumeCreateClusterOpts(t *testing.T) { }) cmd := newCreateCommand(cli) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) cmd.SetArgs([]string{"name"}) - cmd.Flags().Set("driver", "csi") - cmd.Flags().Set("group", "gronp") - cmd.Flags().Set("scope", "multi") - cmd.Flags().Set("sharing", "onewriter") - cmd.Flags().Set("type", "mount") - cmd.Flags().Set("sharing", "onewriter") - cmd.Flags().Set("required-bytes", "1234") - cmd.Flags().Set("limit-bytes", "567890") + assert.Check(t, cmd.Flags().Set("driver", "csi")) + assert.Check(t, cmd.Flags().Set("group", "gronp")) + assert.Check(t, cmd.Flags().Set("scope", "multi")) + assert.Check(t, cmd.Flags().Set("sharing", "onewriter")) + assert.Check(t, cmd.Flags().Set("type", "mount")) + assert.Check(t, cmd.Flags().Set("sharing", "onewriter")) + assert.Check(t, cmd.Flags().Set("required-bytes", "1234")) + assert.Check(t, cmd.Flags().Set("limit-bytes", "567890")) - cmd.Flags().Set("secret", "key1=secret1") - cmd.Flags().Set("secret", "key2=secret2") + assert.Check(t, cmd.Flags().Set("secret", "key1=secret1")) + assert.Check(t, cmd.Flags().Set("secret", "key2=secret2")) - cmd.Flags().Set("topology-required", "region=R1,zone=Z1") - cmd.Flags().Set("topology-required", "region=R1,zone=Z2") - cmd.Flags().Set("topology-required", "region=R1,zone=Z3") + assert.Check(t, cmd.Flags().Set("topology-required", "region=R1,zone=Z1")) + assert.Check(t, cmd.Flags().Set("topology-required", "region=R1,zone=Z2")) + assert.Check(t, cmd.Flags().Set("topology-required", "region=R1,zone=Z3")) - cmd.Flags().Set("topology-preferred", "region=R1,zone=Z2") - cmd.Flags().Set("topology-preferred", "region=R1,zone=Z3") + assert.Check(t, cmd.Flags().Set("topology-preferred", "region=R1,zone=Z2")) + assert.Check(t, cmd.Flags().Set("topology-preferred", "region=R1,zone=Z3")) - cmd.Execute() + assert.NilError(t, cmd.Execute()) }