From be327e744380573663a41912e9614732061436ef Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 3 Nov 2020 11:54:44 -0800 Subject: [PATCH 1/4] add tests for ElementSetter --- kyaml/yaml/fns_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index e7dda5f71..046e6e370 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -97,6 +97,76 @@ func TestGetElementByKey(t *testing.T) { assert.Equal(t, "f: g\n", assertNoErrorString(t)(rn.String())) } +func TestElementSetter(t *testing.T) { + orig := MustParse(` +- a: b +- scalarValue +- c: d +# null will be removed +- null +`) + + // ElementSetter will update node, so make a copy + node := orig.Copy() + // Remove an element + rn, err := node.Pipe(ElementSetter{Key: "a", Value: "b"}) + assert.NoError(t, err) + assert.Nil(t, rn) + assert.Equal(t, `- scalarValue +- c: d +`, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Set an element + newElement := NewMapRNode(&map[string]string{ + "e": "f", + }) + rn, err = node.Pipe(ElementSetter{ + Key: "a", + Value: "b", + Element: newElement.YNode(), + }) + assert.NoError(t, err) + assert.Equal(t, rn, newElement) + assert.Equal(t, `- e: f +- scalarValue +- c: d +`, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Set an element with scalar + newElement = NewScalarRNode("foo") + rn, err = node.Pipe(ElementSetter{ + Key: "a", + Value: "b", + Element: newElement.YNode(), + }) + assert.NoError(t, err) + assert.Equal(t, rn, newElement) + assert.Equal(t, `- foo +- scalarValue +- c: d +`, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Append an element + newElement = NewMapRNode(&map[string]string{ + "e": "f", + }) + rn, err = node.Pipe(ElementSetter{ + Key: "x", + Value: "y", + Element: newElement.YNode(), + }) + assert.NoError(t, err) + assert.Equal(t, rn, newElement) + assert.Equal(t, `- a: b +- scalarValue +- c: d +- e: f +`, assertNoErrorString(t)(node.String())) +} + func TestElementMatcherWithNoValue(t *testing.T) { node, err := Parse(` - a: c From 3fed68b694200a96e4241967f037738b68f895c5 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 3 Nov 2020 13:09:12 -0800 Subject: [PATCH 2/4] clarify the comments --- kyaml/yaml/fns.go | 8 +++++++- kyaml/yaml/fns_test.go | 9 +++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 45b107618..ad69d3bef 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -41,8 +41,14 @@ func (a ElementAppender) Filter(rn *RNode) (*RNode, error) { return nil, nil } -// ElementSetter sets the value for an Element in an associative list. ElementSetter +// ElementSetter sets the value for an Element in an associative list. ElementSetter // will remove any elements which are empty. +// ElementSetter will append, replace or delete an element in an associative list. +// To append, user a key-value pair that doesn't exist in the sequence. Note: this +// behavior is intended to handle the case that not matching element found. It's +// not designed for this purpose. To append an element, please use ElementAppender. +// To replace, set the key-value pair and a non-nil Element. +// To delete, set the key-value pair and leave the Element as nil. type ElementSetter struct { Kind string `yaml:"kind,omitempty"` diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index 046e6e370..dc6ae3d25 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -108,7 +108,7 @@ func TestElementSetter(t *testing.T) { // ElementSetter will update node, so make a copy node := orig.Copy() - // Remove an element + // Remove an element, because ElementSetter.Element is left nil. rn, err := node.Pipe(ElementSetter{Key: "a", Value: "b"}) assert.NoError(t, err) assert.Nil(t, rn) @@ -117,7 +117,7 @@ func TestElementSetter(t *testing.T) { `, assertNoErrorString(t)(node.String())) node = orig.Copy() - // Set an element + // Set an element, replacing 'a: b' with 'e: f' newElement := NewMapRNode(&map[string]string{ "e": "f", }) @@ -134,7 +134,7 @@ func TestElementSetter(t *testing.T) { `, assertNoErrorString(t)(node.String())) node = orig.Copy() - // Set an element with scalar + // Set an element with scalar, {"a": "b"} to "foo" newElement = NewScalarRNode("foo") rn, err = node.Pipe(ElementSetter{ Key: "a", @@ -149,7 +149,8 @@ func TestElementSetter(t *testing.T) { `, assertNoErrorString(t)(node.String())) node = orig.Copy() - // Append an element + // Append an element, {"x": "y"} is not in the list + // so the element will be appended. newElement = NewMapRNode(&map[string]string{ "e": "f", }) From 6bed2752340cff20246a1640e8249f76ac5874b5 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 3 Nov 2020 15:23:34 -0800 Subject: [PATCH 3/4] add more tests for ElementSetter --- kyaml/yaml/fns.go | 5 ++--- kyaml/yaml/fns_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index ad69d3bef..bec46eeb0 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -41,10 +41,9 @@ func (a ElementAppender) Filter(rn *RNode) (*RNode, error) { return nil, nil } -// ElementSetter sets the value for an Element in an associative list. ElementSetter -// will remove any elements which are empty. +// ElementSetter sets the value for an Element in an associative list. // ElementSetter will append, replace or delete an element in an associative list. -// To append, user a key-value pair that doesn't exist in the sequence. Note: this +// To append, user a key-value pair that doesn't exist in the sequence. this // behavior is intended to handle the case that not matching element found. It's // not designed for this purpose. To append an element, please use ElementAppender. // To replace, set the key-value pair and a non-nil Element. diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index dc6ae3d25..fef18557f 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -116,6 +116,55 @@ func TestElementSetter(t *testing.T) { - c: d `, assertNoErrorString(t)(node.String())) + node = orig.Copy() + // Nothing happens because no element is matched + rn, err = node.Pipe(ElementSetter{Key: "a", Value: "zebra"}) + assert.NoError(t, err) + assert.Nil(t, rn) + assert.Equal(t, `- a: b +- scalarValue +- c: d +`, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Return error because ElementSetter doesn't support a single key + // when there is a scalar value in the list + rn, err = node.Pipe(ElementSetter{Key: "a"}) + assert.EqualError(t, err, "wrong Node Kind for expected: MappingNode was ScalarNode: value: {scalarValue}") + + node = MustParse(` +- a: b +- c: d +`) + // {a: b} is removed since the value is omitted and only key is used + // to match and no Element specified. + rn, err = node.Pipe(ElementSetter{Key: "a"}) + assert.NoError(t, err) + assert.Nil(t, rn) + assert.Equal(t, `- c: d +`, assertNoErrorString(t)(node.String())) + + node = MustParse(` +- a: b +- c: d +`) + // Return error because ElementSetter will assume all elements are scalar when + // there is only value provided. + rn, err = node.Pipe(ElementSetter{Value: "b"}) + assert.EqualError(t, err, "wrong Node Kind for expected: ScalarNode was MappingNode: value: {a: b}") + + node = MustParse(` +- a +- b +`) + // b is removed since ElementSetter use the value "b" to match the + // scalar values. + rn, err = node.Pipe(ElementSetter{Value: "b"}) + assert.NoError(t, err) + assert.Nil(t, rn) + assert.Equal(t, `- a +`, assertNoErrorString(t)(node.String())) + node = orig.Copy() // Set an element, replacing 'a: b' with 'e: f' newElement := NewMapRNode(&map[string]string{ From c803ca83a4f0bbe7e4b3adb300a474110e0a8b68 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 3 Nov 2020 15:29:01 -0800 Subject: [PATCH 4/4] fix linter error --- kyaml/yaml/fns_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index fef18557f..1b8628c97 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -129,7 +129,7 @@ func TestElementSetter(t *testing.T) { node = orig.Copy() // Return error because ElementSetter doesn't support a single key // when there is a scalar value in the list - rn, err = node.Pipe(ElementSetter{Key: "a"}) + _, err = node.Pipe(ElementSetter{Key: "a"}) assert.EqualError(t, err, "wrong Node Kind for expected: MappingNode was ScalarNode: value: {scalarValue}") node = MustParse(` @@ -150,7 +150,7 @@ func TestElementSetter(t *testing.T) { `) // Return error because ElementSetter will assume all elements are scalar when // there is only value provided. - rn, err = node.Pipe(ElementSetter{Value: "b"}) + _, err = node.Pipe(ElementSetter{Value: "b"}) assert.EqualError(t, err, "wrong Node Kind for expected: ScalarNode was MappingNode: value: {a: b}") node = MustParse(`