Merge pull request helm-unittest#556 from gofogo/issue-555

issue-555: document selector defensive nil guard

pkg/unittest/.snapshots/TestV3RunnerOkWithDocumentSelector

@@ -13,7 +13,7 @@
 
 Charts:      1 passed, 1 total
 Test Suites: 7 passed, 7 total
-Tests:       9 passed, 9 total
+Tests:       10 passed, 10 total
 Snapshot:    1 passed, 1 total
 Time:        XX.XXXms
 

pkg/unittest/assertion.go

@@ -41,7 +41,6 @@ func (a *Assertion) Assert(
 	// Ensure assertion is succeeding or failing based on templates to test.
 	assertionPassed := false
 	failInfo := make([]string, 0)
-
 	selectedDocsByTemplate, indexError := a.selectDocumentsForAssertion(a.getDocumentsByDefaultTemplates(templatesResult))
 	selectedTemplates := a.getKeys(selectedDocsByTemplate)
 
@@ -212,18 +211,13 @@ func (a *Assertion) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	}
 
 	if documentSelector, ok := assertDef["documentSelector"].(map[string]interface{}); ok {
-		documentSelectorPath := documentSelector["path"].(string)
-		documentSelectorValue := documentSelector["value"]
-		documentSelectorMatchMany := documentSelector["matchMany"] == true
-		documentSelectorSkipEmptyTemplates := documentSelector["skipEmptyTemplates"] == true
-
-		a.DocumentSelector = &valueutils.DocumentSelector{
-			Path:               documentSelectorPath,
-			Value:              documentSelectorValue,
-			MatchMany:          documentSelectorMatchMany,
-			SkipEmptyTemplates: documentSelectorSkipEmptyTemplates,
+		s, err := valueutils.NewDocumentSelector(documentSelector)
+		if err != nil {
+			return err
 		}
+		a.DocumentSelector = s
 	}
+
 	if err := a.constructValidator(assertDef); err != nil {
 		return err
 	}

pkg/unittest/test_runner_test.go

@@ -309,3 +309,16 @@ func TestV3RunnerOk_With_FailFast_NoPanic(t *testing.T) {
 		})
 	}
 }
+
+func TestV3RunnerOkWithDocumentSelect(t *testing.T) {
+	buffer := new(bytes.Buffer)
+	runner := TestRunner{
+		Printer:   printer.NewPrinter(buffer, nil),
+		TestFiles: []string{testTestFiles},
+	}
+	passed := runner.RunV3([]string{testV3WithDocumentSelectorChart})
+	assert.True(t, passed, buffer.String())
+	fmt.Println(buffer.String())
+	assert.Contains(t, buffer.String(), "Test Suites: 7 passed, 7 total")
+	assert.Contains(t, buffer.String(), "Tests:       10 passed, 10 total")
+}

pkg/unittest/test_suite.go

@@ -267,7 +267,6 @@ func (s *TestSuite) RunV3(
 func (s *TestSuite) polishTestJobsPathInfo() {
 	log.WithField(common.LOG_TEST_SUITE, "polish-test-jobs-path-info").Debug("suite '", s.Name, "' total tests ", len(s.Tests))
 	for _, test := range s.Tests {
-
 		if test != nil {
 			test.chartRoute = s.chartRoute
 			test.definitionFile = s.definitionFile

pkg/unittest/test_suite_test.go

@@ -1248,6 +1248,51 @@ tests:
 	a.ErrorContains(err, "Assertion type `notSupportedAssert` is invalid")
 }
 
+func TestV3ParseTestMultipleSuitesDocumentSelectorWithPoisonInAssertIgnored(t *testing.T) {
+	suiteDoc := `
+suite: test suite with assert that not supported
+templates:
+  - "*.yaml"
+tests:
+  - it: should error when not supported assert is found
+    documentSelector:
+     skipEmptyTemplates: true # this is a poison pill
+    asserts:
+      - hasDocuments:
+          count: 1
+`
+	a := assert.New(t)
+	file := path.Join("_scratch", "assert-not-supported.yaml")
+	a.Nil(writeToFile(suiteDoc, file))
+	defer os.RemoveAll(file)
+
+	_, err := ParseTestSuiteFile(file, "basic", true, []string{})
+	a.NoError(err)
+}
+
+func TestV3ParseTestMultipleSuitesDocumentSelectorWithPoisonInTestNotIgnored(t *testing.T) {
+	suiteDoc := `
+suite: test suite with assert that not supported
+templates:
+  - deployment.yaml
+tests:
+  - it: should error when not supported assert is found
+    asserts:
+      - hasDocuments:
+          count: 1
+        documentSelector:
+          skipEmptyTemplates: true
+`
+	a := assert.New(t)
+	file := path.Join("_scratch", "assert-not-supported.yaml")
+	a.Nil(writeToFile(suiteDoc, file))
+	defer os.RemoveAll(file)
+
+	_, err := ParseTestSuiteFile(file, "basic", true, []string{})
+	a.Error(err)
+	a.ErrorContains(err, "empty 'documentSelector.path' not supported")
+}
+
 func TestV3ParseTestMultipleSuites_With_FailFast(t *testing.T) {
 	suiteDoc := `
 suite: test suite with partial chart metadata

pkg/unittest/testdata/chart-document-selector/Chart.yaml

@@ -0,0 +1,6 @@
+apiVersion: v2
+name: document-selector
+version: 1.0.0
+description: simple chart to validate different aspects of documentSelector
+keywords:
+  - helm template test  pkg/unittest/testdata/chart-document-selector --output-dir _scratch

pkg/unittest/testdata/chart-document-selector/templates/cfg01.yaml

@@ -0,0 +1,7 @@
+{{- if .Values.enabled }}
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: second-config-map
+{{- end}}
+

pkg/unittest/testdata/chart-document-selector/templates/cfg02.yaml

@@ -0,0 +1,5 @@
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: second-cfg

pkg/unittest/testdata/chart-document-selector/tests/case1-error_test.yaml

@@ -0,0 +1,11 @@
+suite: document selector erroring first suite
+templates:
+  - "*"
+tests:
+  - it: partial document selector in asserts
+    asserts:
+      - exists:
+          path: kind
+        documentSelector:
+          # this throw an error as currently documentSelector require path and value
+          skipEmptyTemplates: true

pkg/unittest/testdata/chart-document-selector/tests/case1-ok_test.yaml

@@ -0,0 +1,26 @@
+suite: document selector currently working behaviour
+templates:
+  - "*"
+tests:
+  - it: assert selector to skip empty
+    asserts:
+      - exists:
+          path: kind
+        documentSelector:
+          path: kind
+          value: ConfigMap
+          skipEmptyTemplates: true
+
+---
+suite: document selector second suite
+templates:
+  - "*"
+tests:
+  - it: assert selector to skip empty
+    documentSelector:
+      path: kind
+      value: ConfigMap
+      skipEmptyTemplates: true
+    asserts:
+      - exists:
+          path: kind

pkg/unittest/testdata/chart-document-selector/tests/case2-error_test.yaml

@@ -0,0 +1,13 @@
+suite: document selector erroring second suite
+templates:
+  - "*.yaml"
+tests:
+  - it: partial document selector in asserts. documentSelector.value is missing
+    asserts:
+      - exists:
+          path: kind
+        documentSelector:
+          path: kind
+          # this throw an error as currently documentSelector require path and value
+          skipEmptyTemplates: true
+

pkg/unittest/testdata/chart-document-selector/tests/case3-error_test.yaml

@@ -0,0 +1,11 @@
+suite: document selector erroring
+templates:
+  - "*.yaml"
+tests:
+  - it: partial document selector in tests. documentSelector.skipEmptyTemplates is ignored
+    documentSelector:
+      skipEmptyTemplates: true
+    asserts:
+# - asserts[0] exists fail Template document-selector/templates/cfg01.yaml Path kind expected to exists
+      - exists:
+          path: kind

pkg/unittest/testdata/chart-document-selector/tests/case4-error_test.yaml

@@ -0,0 +1,12 @@
+suite: document selector erroring second suite
+templates:
+  - "*.yaml"
+tests:
+  - it: partial document selector in asserts. documentSelector.value is missing
+    asserts:
+      - exists:
+          path: kind
+        documentSelector:
+          # document selector is empty and ignored
+          # current error is "Path: kind expected to exists"
+

pkg/unittest/utils_test.go

@@ -148,3 +148,61 @@ func TestV3RunnerWith_Fixture_Chart_YamlSeparator(t *testing.T) {
 	assert.Contains(t, buffer.String(), "Test Suites: 5 passed, 5 total")
 	assert.Contains(t, buffer.String(), "Tests:       6 passed, 6 total")
 }
+
+func TestV3RunnerWith_Fixture_Chart_DocumentSelector(t *testing.T) {
+	cases := []struct {
+		chart      string
+		testFlavor string
+		expected   []string
+	}{
+		{
+			chart:      "testdata/chart-document-selector",
+			testFlavor: "case1-error",
+			expected: []string{
+				"### Error:  empty 'documentSelector.path' not supported",
+			},
+		},
+		{
+			chart:      "testdata/chart-document-selector",
+			testFlavor: "case2-error",
+			expected: []string{
+				"### Error:  empty 'documentSelector.value' not supported",
+			},
+		},
+		{
+			chart:      "testdata/chart-document-selector",
+			testFlavor: "case3-error",
+			expected: []string{
+				"Template:\tdocument-selector/templates/cfg01.yaml",
+				"Path:\tkind expected to exist",
+			},
+		},
+		{
+			chart:      "testdata/chart-document-selector",
+			testFlavor: "case4-error",
+			expected: []string{
+				"Path:\tkind expected to exists",
+			},
+		},
+		{
+			chart:      "testdata/chart-document-selector",
+			testFlavor: "case1-ok",
+			expected: []string{
+				"Test Suites: 2 passed, 2 total",
+			},
+		},
+	}
+	for _, tt := range cases {
+		t.Run(fmt.Sprintf("chart %s with %s", tt.chart, tt.testFlavor), func(t *testing.T) {
+			buffer := new(bytes.Buffer)
+			runner := TestRunner{
+				Printer:   printer.NewPrinter(buffer, nil),
+				TestFiles: []string{fmt.Sprintf("tests/%s_test.yaml", tt.testFlavor)},
+			}
+			_ = runner.RunV3([]string{tt.chart})
+			for _, e := range tt.expected {
+				assert.Contains(t, buffer.String(), e)
+			}
+		})
+	}
+}

pkg/unittest/valueutils/documentselector.go

@@ -5,6 +5,7 @@ import (
 	"reflect"
 
 	"github.com/helm-unittest/helm-unittest/internal/common"
+	log "github.com/sirupsen/logrus"
 )
 
 type DocumentSelector struct {
@@ -14,6 +15,28 @@ type DocumentSelector struct {
 	Value              interface{}
 }
 
+func NewDocumentSelector(documentSelector map[string]interface{}) (*DocumentSelector, error) {
+	path, ok := documentSelector["path"].(string)
+	if !ok {
+		log.WithField("selector", "document-selector").Debugln("documentSelector.path is empty")
+		return nil, errors.New("empty 'documentSelector.path' not supported")
+	}
+	value := documentSelector["value"]
+	if value == nil {
+		log.WithField("selector", "document-selector").Debugln("documentSelector.value is nil")
+		return nil, errors.New("empty 'documentSelector.value' not supported")
+	}
+	matchMany := documentSelector["matchMany"] == true
+	skipEmptyTemplates := documentSelector["skipEmptyTemplates"] == true
+
+	return &DocumentSelector{
+		Path:               path,
+		Value:              value,
+		MatchMany:          matchMany,
+		SkipEmptyTemplates: skipEmptyTemplates,
+	}, nil
+}
+
 func (ds DocumentSelector) SelectDocuments(documentsByTemplate map[string][]common.K8sManifest) (map[string][]common.K8sManifest, error) {
 
 	matchingDocuments := map[string][]common.K8sManifest{}
@@ -37,12 +60,11 @@ func (ds DocumentSelector) SelectDocuments(documentsByTemplate map[string][]comm
 			matchingDocuments[template] = filteredManifests
 		}
 	}
-
 	return matchingDocuments, nil
 }
 
 func (ds DocumentSelector) selectDocuments(docs []common.K8sManifest) ([]common.K8sManifest, error) {
-	selectedDocs := []common.K8sManifest{}
+	var selectedDocs []common.K8sManifest
 
 	for _, doc := range docs {
 		var indexError error

pkg/unittest/valueutils/documentselector_test.go

@@ -179,3 +179,57 @@ func TestFindDocumentIndicesMatchManyAndDontSkipEmptyTemplatesNOk(t *testing.T)
 	a.EqualError(err, "document not found")
 	a.Equal(expectedManifests, actualManifests)
 }
+
+func TestNewSafeDocumentSelector_Success(t *testing.T) {
+	tests := []struct {
+		name             string
+		input            map[string]interface{}
+		expectedSelector *DocumentSelector
+	}{
+		{
+			name: "all fields set",
+			input: map[string]interface{}{
+				"path":               "metadata.name",
+				"value":              "foo",
+				"matchMany":          true,
+				"skipEmptyTemplates": true,
+			},
+			expectedSelector: &DocumentSelector{
+				Path:               "metadata.name",
+				Value:              "foo",
+				MatchMany:          true,
+				SkipEmptyTemplates: true,
+			},
+		},
+		{
+			name: "only required fields set",
+			input: map[string]interface{}{
+				"path":  "metadata.name",
+				"value": "foo",
+			},
+			expectedSelector: &DocumentSelector{
+				Path:               "metadata.name",
+				Value:              "foo",
+				MatchMany:          false,
+				SkipEmptyTemplates: false,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var actualSelector, err = NewDocumentSelector(tt.input)
+			assert.Nil(t, err)
+			assert.Equal(t, tt.expectedSelector, actualSelector)
+		})
+	}
+}
+
+func TestNewDocumentSelectorMissingPath(t *testing.T) {
+	input := map[string]interface{}{
+		"value": "foo",
+	}
+	selector, err := NewDocumentSelector(input)
+	assert.NotNil(t, err)
+	assert.Nil(t, selector)
+}