Fix ErrParameterSet condition

Signed-off-by: Tobias Brumhard <tobias.brumhard@mail.schwarz>
SchwarzIT · Oct 29, 2021 · 251f6be · 251f6be
1 parent 0e6ee55
commit 251f6be
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 12 deletions.
diff --git a/pkg/gotemplate/new.go b/pkg/gotemplate/new.go
@@ -22,7 +22,7 @@ var (
 	ErrAlreadyExists   = errors.New("already exists")
 	ErrParameterNotSet = errors.New("parameter not set")
 	ErrMalformedInput  = errors.New("malformed input")
-	ErrParameterSet    = errors.New("parameter set but preconditions are not met")
+	ErrParameterSet    = errors.New("parameter set but has no effect in this context")
 )
 
 type ErrTypeMismatch struct {
@@ -103,7 +103,8 @@ func (gt *GT) LoadConfigValuesFromFile(file string) (*OptionValues, error) {
 // nolint: gocritic // option is passed by value to improve usability when iterating a slice of options
 func validateFileOption(option Option, value interface{}, optionValues OptionValues) error {
 	valType := reflect.TypeOf(value)
-	defaultType := reflect.TypeOf(option.Default(&optionValues))
+	defaultVal := option.Default(&optionValues)
+	defaultType := reflect.TypeOf(defaultVal)
 	if valType != defaultType {
 		return &ErrTypeMismatch{
 			Expected: defaultType.Name(),
@@ -115,8 +116,8 @@ func validateFileOption(option Option, value interface{}, optionValues OptionVal
 		return errors.Wrap(ErrMalformedInput, fmt.Sprintf("%s: %s", option.Name(), err.Error()))
 	}
 
-	// if it is set with shouldDisplay not set it means preconditions are not met
-	if !option.ShouldDisplay(&optionValues) {
+	// if it is set to sth else than default with shouldDisplay returning false it means the parameters does not have any effect
+	if value != defaultVal && !option.ShouldDisplay(&optionValues) {
 		return errors.Wrap(ErrParameterSet, option.Name())
 	}
 

diff --git a/pkg/gotemplate/new_test.go b/pkg/gotemplate/new_test.go
@@ -202,18 +202,52 @@ base:
 	})
 
 	t.Run("error if option is set but shouldDisplay returns false", func(t *testing.T) {
-		gt.Options.Base[0] = gotemplate.NewOption(
-			optionName,
+		gt.Options = &gotemplate.Options{
+			Extensions: []gotemplate.Category{
+				{
+					Name: "test",
+					Options: []gotemplate.Option{
+						gotemplate.NewOption(
+							"option",
 							gotemplate.StringValue("description"),
-							gotemplate.StaticValue(true),
+							gotemplate.StaticValue(false),
 							gotemplate.WithShouldDisplay(gotemplate.BoolValue(false)),
-		)
+						),
+					},
+				},
+			},
+		}
 
-		_, err := loadValueFromTestFile(t, gt, fmt.Sprintf(`---
-base:
-    %s: true`, optionName))
+		_, err := loadValueFromTestFile(t, gt, `---
+extensions:
+    test:
+        option: true`)
 
-		assert.ErrorIs(t, err, gotemplate.ErrParameterSet)
+		require.ErrorIs(t, err, gotemplate.ErrParameterSet)
+	})
+
+	t.Run("no error if option is set to default value shouldDisplay returns false", func(t *testing.T) {
+		gt.Options = &gotemplate.Options{
+			Extensions: []gotemplate.Category{
+				{
+					Name: "test",
+					Options: []gotemplate.Option{
+						gotemplate.NewOption(
+							"option",
+							gotemplate.StringValue("description"),
+							gotemplate.StaticValue(true),
+							gotemplate.WithShouldDisplay(gotemplate.BoolValue(false)),
+						),
+					},
+				},
+			},
+		}
+
+		_, err := loadValueFromTestFile(t, gt, `---
+extensions:
+    test:
+        option: true`)
+		require.NoError(t, err)
 	})
 }