From bd2be279a735f287234981c325db3676f4221529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Qu=C3=A9r=C3=A9?= Date: Tue, 11 Feb 2020 16:55:20 +0100 Subject: [PATCH 1/7] feat(args): add support for empty slice un/marshahaling --- internal/args/errors.go | 9 ++++++++ internal/args/marshal.go | 6 +++++ internal/args/marshal_test.go | 18 ++++++++++++++- internal/args/unmarshal.go | 41 ++++++++++++++++++++++++++++++--- internal/args/unmarshal_test.go | 29 +++++++++++++++++++++++ 5 files changed, 99 insertions(+), 4 deletions(-) diff --git a/internal/args/errors.go b/internal/args/errors.go index 5bb245a487..91e673f13f 100644 --- a/internal/args/errors.go +++ b/internal/args/errors.go @@ -161,6 +161,15 @@ func (e *CannotUnmarshalError) Error() string { return fmt.Sprintf("%T is not unmarshalable: %s", e.Dest, e.Err) } +type ConflictArgError struct { + ArgName1 string + ArgName2 string +} + +func (e *ConflictArgError) Error() string { + return fmt.Sprintf("arguments '%s' and '%s' cannot be used simultaneously", e.ArgName1, e.ArgName2) +} + // missingIndices returns a string of all the missing indices between index and length. // e.g.: missingIndices(index=5, length=0) should return "0,1,2,3" // e.g.: missingIndices(index=5, length=2) should return "2,3" diff --git a/internal/args/marshal.go b/internal/args/marshal.go index 42c3b6c2f9..6dd45f440b 100644 --- a/internal/args/marshal.go +++ b/internal/args/marshal.go @@ -123,6 +123,12 @@ func marshal(src reflect.Value, keys []string) (args []string, err error) { case reflect.Slice: // If type is a slice: + + // If we explicitly try to marshal a non-nil empty slice we marshal it as args=none + if src.Len() == 0 && !src.IsNil() { + return append(args, marshalKeyValue(keys, "none")), nil + } + // We loop through all items and marshal them with key = key.0, key.1, .... args := []string(nil) for i := 0; i < src.Len(); i++ { diff --git a/internal/args/marshal_test.go b/internal/args/marshal_test.go index eb589ebe09..712b49e145 100644 --- a/internal/args/marshal_test.go +++ b/internal/args/marshal_test.go @@ -101,6 +101,17 @@ func TestMarshal(t *testing.T) { }, })) + t.Run("empty-slice", run(TestCase{ + data: &Slice{ + Strings: []string{}, + StringsPtr: []*string{}, + }, + expected: []string{ + "strings=none", + "strings-ptr=none", + }, + })) + t.Run("well-known-types", run(TestCase{ data: &WellKnownTypes{ Size: 20 * scw.GB, @@ -247,11 +258,16 @@ func TestMarshalValue(t *testing.T) { expected: "", })) - t.Run("typed-nil", run(TestCase{ + t.Run("nil-slice", run(TestCase{ data: []string(nil), expected: "", })) + t.Run("empty-slice", run(TestCase{ + data: []string{}, + expected: "none", + })) + t.Run("custom-func", run(TestCase{ data: height(42), expected: "42cm", diff --git a/internal/args/unmarshal.go b/internal/args/unmarshal.go index 7dec290a38..b1d0f3041f 100644 --- a/internal/args/unmarshal.go +++ b/internal/args/unmarshal.go @@ -75,6 +75,7 @@ func UnmarshalStruct(args []string, data interface{}) error { // Loop through all arguments for _, kv := range argsSlice { argName, argValue := kv[0], kv[1] + argNameWords := strings.Split(argName, ".") // Make sure argument name is correct. // We enforce this check to avoid not well formatted argument name to work by "accident" @@ -102,10 +103,39 @@ func UnmarshalStruct(args []string, data interface{}) error { Err: &DuplicateArgError{}, } } + + // We check that we did not already handle an argument value set on a parent + // Example `cluster.volume.size=12 cluster=premium` cannot be valid as both args are in conflict. + // We loop through all possible parent and check that they are not present in processedArgNames + // If argName=cluster.volume.0.size we will check ["cluster", "cluster.volume" "cluster.volume.0"] + // Duplicate args are handle above. + for index := range argNameWords { + processedArgName := strings.Join(argNameWords[:index+1], ".") + if processedArgNames[processedArgName] { + return &ConflictArgError{ + ArgName1: processedArgName, + ArgName2: argName, + } + } + } + + // We check that we did not already handle an argument value set on a child + // Example `cluster=premium cluster.volume.size=12` cannot be valid as both args are in conflict. + // We loop through all processedArgNames and if argName starts we one of them we consider it as duplicate + // If processedArgNames=["cluster"] and argName=cluster.volume.size we return an error because `cluster.volume.size` as `cluster.` prefix + for processedArgName := range processedArgNames { + if strings.HasPrefix(processedArgName+".", argName) { + return &ConflictArgError{ + ArgName1: processedArgName, + ArgName2: argName, + } + } + } + processedArgNames[argName] = true // Set will recursively find the correct field to set. - err := set(dest, strings.Split(argName, "."), argValue) + err := set(dest, argNameWords, argValue) if err != nil { return &UnmarshalArgError{ ArgName: argName, @@ -177,14 +207,19 @@ func set(dest reflect.Value, argNameWords []string, value string) error { case reflect.Slice: // If type is a slice: - // We check if argNameWords[0] is an number to handle cases like keys.0.value=12 + + // We handle special case where array=none creates an empty slice. + if len(argNameWords) == 0 && value == "none" { + dest.Set(reflect.MakeSlice(dest.Type(), 0, 0)) + return nil + } // We cannot handle slice without an index notation. if len(argNameWords) == 0 { return &MissingIndexOnArrayError{} } - // Make sure index is a positive integer. + // We check if argNameWords[0] is an positive integer to handle cases like keys.0.value=12 index, err := strconv.ParseUint(argNameWords[0], 10, 64) if err != nil { return &InvalidIndexError{Index: argNameWords[0]} diff --git a/internal/args/unmarshal_test.go b/internal/args/unmarshal_test.go index e34e56cacc..e38133331b 100644 --- a/internal/args/unmarshal_test.go +++ b/internal/args/unmarshal_test.go @@ -185,6 +185,35 @@ func TestUnmarshalStruct(t *testing.T) { }, })) + t.Run("empty-slice", run(TestCase{ + args: []string{ + "strings=none", + "strings-ptr=none", + }, + expected: &Slice{ + Strings: []string{}, + StringsPtr: []*string{}, + }, + })) + + t.Run("simple-parent-child-conflict", run(TestCase{ + args: []string{ + "strings=none", + "strings.0=none", + }, + error: "arguments 'strings' and 'strings.0' cannot be used simultaneously", + data: &Slice{}, + })) + + t.Run("simple-child-parent-conflict", run(TestCase{ + args: []string{ + "strings.0=none", + "strings=none", + }, + error: "arguments 'strings.0' and 'strings' cannot be used simultaneously", + data: &Slice{}, + })) + t.Run("well-known-types", run(TestCase{ args: []string{ "size=20gb", From 6650b77715690e7b160c664e16e4c4a0c2fe206e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Qu=C3=A9r=C3=A9?= Date: Tue, 11 Feb 2020 16:57:24 +0100 Subject: [PATCH 2/7] add const --- internal/args/args.go | 2 ++ internal/args/marshal.go | 2 +- internal/args/unmarshal.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/args/args.go b/internal/args/args.go index 595e3ac15f..abd818bc1c 100644 --- a/internal/args/args.go +++ b/internal/args/args.go @@ -9,6 +9,8 @@ import ( // validArgNameRegex regex to check that args words are lower-case or digit starting and ending with a letter. var validArgNameRegex = regexp.MustCompile(`^([a-z][a-z0-9-]*)(\.[a-z0-9-]*)*$`) +const emptySliceValue = "none" + // RawArgs allows to retrieve a simple []string using UnmarshalStruct() type RawArgs []string diff --git a/internal/args/marshal.go b/internal/args/marshal.go index 6dd45f440b..4a3467be00 100644 --- a/internal/args/marshal.go +++ b/internal/args/marshal.go @@ -126,7 +126,7 @@ func marshal(src reflect.Value, keys []string) (args []string, err error) { // If we explicitly try to marshal a non-nil empty slice we marshal it as args=none if src.Len() == 0 && !src.IsNil() { - return append(args, marshalKeyValue(keys, "none")), nil + return append(args, marshalKeyValue(keys, emptySliceValue)), nil } // We loop through all items and marshal them with key = key.0, key.1, .... diff --git a/internal/args/unmarshal.go b/internal/args/unmarshal.go index b1d0f3041f..1a22f0741e 100644 --- a/internal/args/unmarshal.go +++ b/internal/args/unmarshal.go @@ -209,7 +209,7 @@ func set(dest reflect.Value, argNameWords []string, value string) error { // If type is a slice: // We handle special case where array=none creates an empty slice. - if len(argNameWords) == 0 && value == "none" { + if len(argNameWords) == 0 && value == emptySliceValue { dest.Set(reflect.MakeSlice(dest.Type(), 0, 0)) return nil } From fb6ccacd7b818a91271498111c9c0f9c42797f87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Qu=C3=A9r=C3=A9?= Date: Thu, 13 Feb 2020 12:01:31 +0100 Subject: [PATCH 3/7] fix test --- internal/args/args_test.go | 5 +++++ internal/args/unmarshal.go | 31 +++++++++++-------------------- internal/args/unmarshal_test.go | 11 +++++++++++ 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/internal/args/args_test.go b/internal/args/args_test.go index c05f734550..445fea1d32 100644 --- a/internal/args/args_test.go +++ b/internal/args/args_test.go @@ -129,3 +129,8 @@ func unmarshalHeight(value string, dest interface{}) error { _, err := fmt.Sscanf(value, "%dcm", h) return err } + +type SamePrefixArgName struct { + IP string + IPv6 string +} diff --git a/internal/args/unmarshal.go b/internal/args/unmarshal.go index 1a22f0741e..cbd7f3b3e9 100644 --- a/internal/args/unmarshal.go +++ b/internal/args/unmarshal.go @@ -104,34 +104,25 @@ func UnmarshalStruct(args []string, data interface{}) error { } } - // We check that we did not already handle an argument value set on a parent - // Example `cluster.volume.size=12 cluster=premium` cannot be valid as both args are in conflict. - // We loop through all possible parent and check that they are not present in processedArgNames - // If argName=cluster.volume.0.size we will check ["cluster", "cluster.volume" "cluster.volume.0"] - // Duplicate args are handle above. - for index := range argNameWords { - processedArgName := strings.Join(argNameWords[:index+1], ".") - if processedArgNames[processedArgName] { - return &ConflictArgError{ - ArgName1: processedArgName, - ArgName2: argName, - } - } - } - - // We check that we did not already handle an argument value set on a child + // We check that we did not already handle an argument value set on a child or a parent // Example `cluster=premium cluster.volume.size=12` cannot be valid as both args are in conflict. - // We loop through all processedArgNames and if argName starts we one of them we consider it as duplicate - // If processedArgNames=["cluster"] and argName=cluster.volume.size we return an error because `cluster.volume.size` as `cluster.` prefix + // Example `cluster.volume.size=12 cluster=premium` should also be invalid. for processedArgName := range processedArgNames { - if strings.HasPrefix(processedArgName+".", argName) { + // We put the longest argName in long and the shortest in short. + short, long := argName, processedArgName + if len(long) < len(short) { + short, long = long, short + } + + // We check if the longest starts with short+"." + // If it does this mean we have a conflict. + if strings.HasPrefix(long, short+".") { return &ConflictArgError{ ArgName1: processedArgName, ArgName2: argName, } } } - processedArgNames[argName] = true // Set will recursively find the correct field to set. diff --git a/internal/args/unmarshal_test.go b/internal/args/unmarshal_test.go index e38133331b..46a38b25b6 100644 --- a/internal/args/unmarshal_test.go +++ b/internal/args/unmarshal_test.go @@ -408,6 +408,17 @@ func TestUnmarshalStruct(t *testing.T) { })) } + t.Run("common-prefix-args", run(TestCase{ + args: []string{ + "ip=ip", + "ipv6=ipv6", + }, + expected: &SamePrefixArgName{ + IP: "ip", + IPv6: "ipv6", + }, + })) +} func TestIsUmarshalableValue(t *testing.T) { type TestCase struct { expected bool From 59f948b99e2e997b786bda6bbaf43cbb624baf17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Qu=C3=A9r=C3=A9?= Date: Thu, 13 Feb 2020 13:47:44 +0100 Subject: [PATCH 4/7] address comments --- internal/args/args_test.go | 1 + internal/args/errors.go | 2 ++ internal/args/marshal_test.go | 8 ++++++++ internal/args/unmarshal.go | 4 ++-- internal/args/unmarshal_test.go | 8 ++++++++ 5 files changed, 21 insertions(+), 2 deletions(-) diff --git a/internal/args/args_test.go b/internal/args/args_test.go index 445fea1d32..1ff49f3520 100644 --- a/internal/args/args_test.go +++ b/internal/args/args_test.go @@ -25,6 +25,7 @@ type Basic struct { type Slice struct { Strings []string StringsPtr []*string + SlicePtr *[]string Basics []Basic } diff --git a/internal/args/errors.go b/internal/args/errors.go index 91e673f13f..b824c0a237 100644 --- a/internal/args/errors.go +++ b/internal/args/errors.go @@ -161,6 +161,8 @@ func (e *CannotUnmarshalError) Error() string { return fmt.Sprintf("%T is not unmarshalable: %s", e.Dest, e.Err) } +// ConflictArgError is return when two args that are in conflict with each other are used together. +// e.g cluster=prod cluster.name=test are conflicting args type ConflictArgError struct { ArgName1 string ArgName2 string diff --git a/internal/args/marshal_test.go b/internal/args/marshal_test.go index 712b49e145..b1cb5afa8e 100644 --- a/internal/args/marshal_test.go +++ b/internal/args/marshal_test.go @@ -15,6 +15,8 @@ func TestMarshal(t *testing.T) { } stringPtr := "test" + slicePtr := []string{"0", "1", "2"} + emptySlicePtr := []string{} run := func(testCase TestCase) func(t *testing.T) { return func(t *testing.T) { @@ -76,6 +78,7 @@ func TestMarshal(t *testing.T) { data: &Slice{ Strings: []string{"1", "2", "", "3"}, StringsPtr: []*string{&stringPtr, &stringPtr}, + SlicePtr: &slicePtr, Basics: []Basic{ { String: "test", @@ -94,6 +97,9 @@ func TestMarshal(t *testing.T) { "strings.3=3", "strings-ptr.0=test", "strings-ptr.1=test", + "slice-ptr.0=0", + "slice-ptr.1=1", + "slice-ptr.2=2", "basics.0.string=test", "basics.0.int=42", "basics.2.string=test", @@ -104,11 +110,13 @@ func TestMarshal(t *testing.T) { t.Run("empty-slice", run(TestCase{ data: &Slice{ Strings: []string{}, + SlicePtr: &emptySlicePtr, StringsPtr: []*string{}, }, expected: []string{ "strings=none", "strings-ptr=none", + "slice-ptr=none", }, })) diff --git a/internal/args/unmarshal.go b/internal/args/unmarshal.go index cbd7f3b3e9..7b906557b3 100644 --- a/internal/args/unmarshal.go +++ b/internal/args/unmarshal.go @@ -199,7 +199,7 @@ func set(dest reflect.Value, argNameWords []string, value string) error { case reflect.Slice: // If type is a slice: - // We handle special case where array=none creates an empty slice. + // When array=none we creates an empty slice. if len(argNameWords) == 0 && value == emptySliceValue { dest.Set(reflect.MakeSlice(dest.Type(), 0, 0)) return nil @@ -210,7 +210,7 @@ func set(dest reflect.Value, argNameWords []string, value string) error { return &MissingIndexOnArrayError{} } - // We check if argNameWords[0] is an positive integer to handle cases like keys.0.value=12 + // We check if argNameWords[0] is a positive integer to handle cases like keys.0.value=12 index, err := strconv.ParseUint(argNameWords[0], 10, 64) if err != nil { return &InvalidIndexError{Index: argNameWords[0]} diff --git a/internal/args/unmarshal_test.go b/internal/args/unmarshal_test.go index 46a38b25b6..ab0950b620 100644 --- a/internal/args/unmarshal_test.go +++ b/internal/args/unmarshal_test.go @@ -17,6 +17,8 @@ func TestUnmarshalStruct(t *testing.T) { } stringPtr := "test" + slicePtr := []string{"0", "1", "2"} + emptySlicePtr := []string{} run := func(testCase TestCase) func(t *testing.T) { return func(t *testing.T) { @@ -164,6 +166,9 @@ func TestUnmarshalStruct(t *testing.T) { "strings.3=test", "strings-ptr.0=test", "strings-ptr.1=test", + "slice-ptr.0=0", + "slice-ptr.1=1", + "slice-ptr.2=2", "basics.0.string=test", "basics.0.int=42", "basics.1.string=test", @@ -172,6 +177,7 @@ func TestUnmarshalStruct(t *testing.T) { expected: &Slice{ Strings: []string{"1", "2", "3", "test"}, StringsPtr: []*string{&stringPtr, &stringPtr}, + SlicePtr: &slicePtr, Basics: []Basic{ { String: "test", @@ -188,10 +194,12 @@ func TestUnmarshalStruct(t *testing.T) { t.Run("empty-slice", run(TestCase{ args: []string{ "strings=none", + "slice-ptr=none", "strings-ptr=none", }, expected: &Slice{ Strings: []string{}, + SlicePtr: &emptySlicePtr, StringsPtr: []*string{}, }, })) From 274a651ae3c126cb4bb6e8294910ed87b218f0d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Qu=C3=A9r=C3=A9?= Date: Thu, 13 Feb 2020 16:09:40 +0100 Subject: [PATCH 5/7] address comments --- internal/args/marshal.go | 14 +++++++++----- internal/args/marshal_test.go | 4 +--- internal/args/unmarshal.go | 15 +++++++++------ internal/args/unmarshal_test.go | 22 +++++++++++----------- 4 files changed, 30 insertions(+), 25 deletions(-) diff --git a/internal/args/marshal.go b/internal/args/marshal.go index 4a3467be00..e70e460f53 100644 --- a/internal/args/marshal.go +++ b/internal/args/marshal.go @@ -118,17 +118,21 @@ func marshal(src reflect.Value, keys []string) (args []string, err error) { if src.IsNil() { return nil, nil } + + // When: + // - dest is a pointer to a slice + // - The slice is empty + // we return slice=none + if src.Elem().Kind() == reflect.Slice && src.Elem().Len() == 0 { + return append(args, marshalKeyValue(keys, emptySliceValue)), nil + } + // If type is a pointer we Marshal pointer.Elem() return marshal(src.Elem(), keys) case reflect.Slice: // If type is a slice: - // If we explicitly try to marshal a non-nil empty slice we marshal it as args=none - if src.Len() == 0 && !src.IsNil() { - return append(args, marshalKeyValue(keys, emptySliceValue)), nil - } - // We loop through all items and marshal them with key = key.0, key.1, .... args := []string(nil) for i := 0; i < src.Len(); i++ { diff --git a/internal/args/marshal_test.go b/internal/args/marshal_test.go index b1cb5afa8e..3dd80779e4 100644 --- a/internal/args/marshal_test.go +++ b/internal/args/marshal_test.go @@ -16,7 +16,7 @@ func TestMarshal(t *testing.T) { stringPtr := "test" slicePtr := []string{"0", "1", "2"} - emptySlicePtr := []string{} + emptySlicePtr := []string(nil) run := func(testCase TestCase) func(t *testing.T) { return func(t *testing.T) { @@ -114,8 +114,6 @@ func TestMarshal(t *testing.T) { StringsPtr: []*string{}, }, expected: []string{ - "strings=none", - "strings-ptr=none", "slice-ptr=none", }, })) diff --git a/internal/args/unmarshal.go b/internal/args/unmarshal.go index 7b906557b3..a4ea6fd3fd 100644 --- a/internal/args/unmarshal.go +++ b/internal/args/unmarshal.go @@ -193,18 +193,21 @@ func set(dest reflect.Value, argNameWords []string, value string) error { dest.Set(reflect.New(dest.Type().Elem())) } + // When: + // - dest is a pointer to a slice + // - there is no more argNameWords left + // - value == none + // we let the slice empty and return + if dest.Elem().Kind() == reflect.Slice && len(argNameWords) == 0 && value == emptySliceValue { + return nil + } + // Call set with the pointer.Elem() return set(dest.Elem(), argNameWords, value) case reflect.Slice: // If type is a slice: - // When array=none we creates an empty slice. - if len(argNameWords) == 0 && value == emptySliceValue { - dest.Set(reflect.MakeSlice(dest.Type(), 0, 0)) - return nil - } - // We cannot handle slice without an index notation. if len(argNameWords) == 0 { return &MissingIndexOnArrayError{} diff --git a/internal/args/unmarshal_test.go b/internal/args/unmarshal_test.go index ab0950b620..a0585749c1 100644 --- a/internal/args/unmarshal_test.go +++ b/internal/args/unmarshal_test.go @@ -18,7 +18,7 @@ func TestUnmarshalStruct(t *testing.T) { stringPtr := "test" slicePtr := []string{"0", "1", "2"} - emptySlicePtr := []string{} + emptySlicePtr := []string(nil) run := func(testCase TestCase) func(t *testing.T) { return func(t *testing.T) { @@ -193,32 +193,32 @@ func TestUnmarshalStruct(t *testing.T) { t.Run("empty-slice", run(TestCase{ args: []string{ - "strings=none", + //"strings=none", "slice-ptr=none", - "strings-ptr=none", + //"strings-ptr=none", }, expected: &Slice{ - Strings: []string{}, + Strings: []string(nil), SlicePtr: &emptySlicePtr, - StringsPtr: []*string{}, + StringsPtr: []*string(nil), }, })) t.Run("simple-parent-child-conflict", run(TestCase{ args: []string{ - "strings=none", - "strings.0=none", + "slice-ptr=none", + "slice-ptr.0=none", }, - error: "arguments 'strings' and 'strings.0' cannot be used simultaneously", + error: "arguments 'slice-ptr' and 'slice-ptr.0' cannot be used simultaneously", data: &Slice{}, })) t.Run("simple-child-parent-conflict", run(TestCase{ args: []string{ - "strings.0=none", - "strings=none", + "slice-ptr.0=none", + "slice-ptr=none", }, - error: "arguments 'strings.0' and 'strings' cannot be used simultaneously", + error: "arguments 'slice-ptr.0' and 'slice-ptr' cannot be used simultaneously", data: &Slice{}, })) From b67553d7e19aecfd68f177efef32ea00a4208fbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Qu=C3=A9r=C3=A9?= Date: Thu, 13 Feb 2020 16:17:39 +0100 Subject: [PATCH 6/7] address comments --- internal/args/marshal_test.go | 3 +-- internal/args/unmarshal_test.go | 13 +++++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/internal/args/marshal_test.go b/internal/args/marshal_test.go index 3dd80779e4..12c416983c 100644 --- a/internal/args/marshal_test.go +++ b/internal/args/marshal_test.go @@ -16,7 +16,6 @@ func TestMarshal(t *testing.T) { stringPtr := "test" slicePtr := []string{"0", "1", "2"} - emptySlicePtr := []string(nil) run := func(testCase TestCase) func(t *testing.T) { return func(t *testing.T) { @@ -110,7 +109,7 @@ func TestMarshal(t *testing.T) { t.Run("empty-slice", run(TestCase{ data: &Slice{ Strings: []string{}, - SlicePtr: &emptySlicePtr, + SlicePtr: scw.StringsPtr(nil), StringsPtr: []*string{}, }, expected: []string{ diff --git a/internal/args/unmarshal_test.go b/internal/args/unmarshal_test.go index a0585749c1..814c7d83bc 100644 --- a/internal/args/unmarshal_test.go +++ b/internal/args/unmarshal_test.go @@ -18,7 +18,6 @@ func TestUnmarshalStruct(t *testing.T) { stringPtr := "test" slicePtr := []string{"0", "1", "2"} - emptySlicePtr := []string(nil) run := func(testCase TestCase) func(t *testing.T) { return func(t *testing.T) { @@ -193,17 +192,23 @@ func TestUnmarshalStruct(t *testing.T) { t.Run("empty-slice", run(TestCase{ args: []string{ - //"strings=none", "slice-ptr=none", - //"strings-ptr=none", }, expected: &Slice{ Strings: []string(nil), - SlicePtr: &emptySlicePtr, + SlicePtr: scw.StringsPtr(nil), StringsPtr: []*string(nil), }, })) + t.Run("none-on-non-pointer-slice", run(TestCase{ + args: []string{ + "strings=none", + }, + error: "cannot unmarshal arg 'strings=none': missing index on the array", + data: &Slice{}, + })) + t.Run("simple-parent-child-conflict", run(TestCase{ args: []string{ "slice-ptr=none", From e63b9384ed50d6e727c317eac5485cc50533c553 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Qu=C3=A9r=C3=A9?= Date: Thu, 13 Feb 2020 16:25:41 +0100 Subject: [PATCH 7/7] fix --- internal/args/unmarshal_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/args/unmarshal_test.go b/internal/args/unmarshal_test.go index 814c7d83bc..3a4d1a45b4 100644 --- a/internal/args/unmarshal_test.go +++ b/internal/args/unmarshal_test.go @@ -419,7 +419,6 @@ func TestUnmarshalStruct(t *testing.T) { }, }, })) -} t.Run("common-prefix-args", run(TestCase{ args: []string{