From f1a0fd5a68bde7aeb805891821c789442caa0a02 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 29 Apr 2025 10:44:57 -0500 Subject: [PATCH 1/4] exploring an idea --- provider/parameter_test.go | 196 +++++++++++++++++++++++++++++++++++++ provider/table_test.go | 56 +++++++++++ 2 files changed, 252 insertions(+) create mode 100644 provider/table_test.go diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 4b52d943..210b86f3 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -686,6 +686,183 @@ data "coder_parameter" "region" { } } +//nolint:paralleltest,tparallel // Parameters load values from env vars +func TestParameterValidationEnforcement(t *testing.T) { + for _, tc := range []struct { + Name string + Config string + Value string + ExpectError *regexp.Regexp + Check func(state *terraform.ResourceState) + }{ + // Empty + { + Name: "EmptyString", + Config: ` + data "coder_parameter" "parameter" { + name = "parameter" + type = "string" + } +`, + ExpectError: nil, + Check: func(state *terraform.ResourceState) { + attrs := state.Primary.Attributes + for key, value := range map[string]interface{}{ + "default": "", + "value": "", + "optional": "false", + } { + require.Equal(t, value, attrs[key]) + } + }, + }, + { + Name: "EmptyNumber", + Config: ` + data "coder_parameter" "parameter" { + name = "parameter" + type = "number" + } +`, + ExpectError: nil, + Check: func(state *terraform.ResourceState) { + attrs := state.Primary.Attributes + for key, value := range map[string]interface{}{ + "default": "", + "value": "", + "optional": "false", + } { + require.Equal(t, value, attrs[key]) + } + }, + }, + // EmptyWithOption + { + Name: "EmptyWithOption", + Config: ` + data "coder_parameter" "parameter" { + name = "parameter" + type = "number" + + option { + name = "option" + value = "5" + } + } +`, + ExpectError: nil, + Check: func(state *terraform.ResourceState) { + attrs := state.Primary.Attributes + for key, value := range map[string]interface{}{ + "default": "", + "value": "", + "optional": "false", + } { + require.Equal(t, value, attrs[key]) + } + }, + }, + // DefaultSet + { + Name: "DefaultSet", + Config: ` + data "coder_parameter" "parameter" { + name = "parameter" + type = "number" + default = "5" + } +`, + ExpectError: nil, + Check: func(state *terraform.ResourceState) { + attrs := state.Primary.Attributes + for key, value := range map[string]interface{}{ + "default": "5", + "value": "5", + "optional": "true", + } { + require.Equal(t, value, attrs[key]) + } + }, + }, + { + Name: "DefaultSetInOption", + Config: ` + data "coder_parameter" "parameter" { + name = "parameter" + type = "number" + default = "5" + option { + name = "option" + value = "5" + } + } +`, + ExpectError: nil, + Check: func(state *terraform.ResourceState) { + attrs := state.Primary.Attributes + for key, value := range map[string]interface{}{ + "default": "5", + "value": "5", + "optional": "true", + } { + require.Equal(t, value, attrs[key]) + } + }, + }, + { + Name: "DefaultSetOutOption", + Config: ` + data "coder_parameter" "parameter" { + name = "parameter" + type = "number" + default = "2" + option { + name = "option" + value = "5" + } + } +`, + ExpectError: nil, + Check: func(state *terraform.ResourceState) { + attrs := state.Primary.Attributes + for key, value := range map[string]interface{}{ + "default": "5", + "value": "5", + "optional": "true", + } { + require.Equal(t, value, attrs[key]) + } + }, + }, + } { + tc := tc + //nolint:paralleltest,tparallel // Parameters load values from env vars + t.Run(tc.Name, func(t *testing.T) { + if tc.Value != "" { + t.Setenv(provider.ParameterEnvironmentVariable("parameter"), tc.Value) + } + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: tc.Config, + ExpectError: tc.ExpectError, + Check: func(state *terraform.State) error { + require.Len(t, state.Modules, 1) + require.Len(t, state.Modules[0].Resources, 1) + param := state.Modules[0].Resources["data.coder_parameter.parameter"] + require.NotNil(t, param) + if tc.Check != nil { + tc.Check(param) + } + return nil + }, + }}, + }) + }) + } +} + func TestValueValidatesType(t *testing.T) { t.Parallel() for _, tc := range []struct { @@ -798,6 +975,25 @@ func TestValueValidatesType(t *testing.T) { Value: `[]`, MinDisabled: true, MaxDisabled: true, + }, { + Name: "ValidListOfStrings", + Type: "list(string)", + Value: `["first","second","third"]`, + MinDisabled: true, + MaxDisabled: true, + }, { + Name: "InvalidListOfStrings", + Type: "list(string)", + Value: `["first","second","third"`, + MinDisabled: true, + MaxDisabled: true, + Error: regexp.MustCompile("is not valid list of strings"), + }, { + Name: "EmptyListOfStrings", + Type: "list(string)", + Value: `[]`, + MinDisabled: true, + MaxDisabled: true, }} { tc := tc t.Run(tc.Name, func(t *testing.T) { diff --git a/provider/table_test.go b/provider/table_test.go new file mode 100644 index 00000000..c17db585 --- /dev/null +++ b/provider/table_test.go @@ -0,0 +1,56 @@ +package provider_test + +import ( + "strconv" + "strings" + "testing" +) + +func TestMDtable(t *testing.T) { + // Copy and paste the table to https://www.tablesgenerator.com/markdown_tables for easier editing + table := strings.TrimSpace(` +| Name | Type | Input Value | Default | Options | Validation | -> | Output Value | Optional | Error | +|------------------|---------------|-------------|-----------|---------|------------|----|--------------|----------|-------| +| Empty | string,number | undefined | undefined | | undefined | | "" | false | - | +| EmptyWithOptions | number | undefined | undefined | | undefined | | "" | false | - | +| DefaultSet | number | undefined | 5 | | undefined | | 5 | true | - | +`) + + type row struct { + Name string + Types []string + InputValue string + Default string + Options string + Validation string + OutputValue string + Optional bool + Error string + } + + rows := make([]row, 0) + lines := strings.Split(table, "\n") + for _, line := range lines[2:] { + columns := strings.Split(line, "|") + columns = columns[1 : len(columns)-2] + for i := range columns { + columns[i] = strings.TrimSpace(columns[i]) + } + + optional, err := strconv.ParseBool(columns[8]) + if err != nil { + t.Fatalf("failed to parse optional column %q: %v", columns[8], err) + } + rows = append(rows, row{ + Name: columns[0], + Types: strings.Split(columns[1], ","), + InputValue: columns[2], + Default: columns[3], + Options: columns[4], + Validation: columns[5], + OutputValue: columns[7], + Optional: optional, + Error: columns[9], + }) + } +} From 9af5f68174431392d9ffaf2f7733dd0b43777c2b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 30 Apr 2025 10:30:37 -0500 Subject: [PATCH 2/4] test: add unit test to document validation behavior --- provider/parameter_test.go | 369 +++++++++++++++++++++---------------- provider/table_test.go | 56 ------ 2 files changed, 207 insertions(+), 218 deletions(-) delete mode 100644 provider/table_test.go diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 210b86f3..7fde13ae 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -3,6 +3,7 @@ package provider_test import ( "fmt" "regexp" + "strconv" "strings" "testing" @@ -686,180 +687,224 @@ data "coder_parameter" "region" { } } +// TestParameterValidationEnforcement tests various parameter states and the +// validation enforcement that should be applied to them. The table is described +// by a markdown table. This is done so that the test cases can be more easily +// edited and read. +// +// Copy and paste the table to https://www.tablesgenerator.com/markdown_tables for easier editing +// //nolint:paralleltest,tparallel // Parameters load values from env vars func TestParameterValidationEnforcement(t *testing.T) { - for _, tc := range []struct { + table := strings.TrimSpace(` +| Name | Type | Input Value | Default | Options | Validation | -> | Output Value | Optional | Error | +|---------------|---------------|-------------|---------|-------------------|------------|----|--------------|----------|--------------| +| | Empty Vals | | | | | | | | | +| Emty | string,number | | | | | | "" | false | | +| EmtyOpts | string,number | | | 1,2,3 | | | "" | false | | +| EmtyRegex | string | | | | world | | | | regex error | +| EmtyMin | number | | | | 1-10 | | | | 1 < < 10 | +| EmtyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | +| EmtyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | +| EmtyRegexOk | string | | | | .* | | "" | false | | +| | | | | | | | | | | +| | Default Set | No inputs | | | | | | | | +| NumDef | number | | 5 | | | | 5 | true | | +| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | +| NumDefInv | number | | 5 | | 10- | | 5 | | 10 < 5 < 0 | +| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | +| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | +| | | | | | | | | | | +| StrDef | string | | hello | | | | hello | true | | +| StrDefInv | string | | hello | | world | | | | regex error | +| StrDefOpts | string | | a | a,b,c | | | a | true | | +| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | +| StrDefOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | +| | | | | | | | | | | +| | Input Vals | | | | | | | | | +| NumIns | number | 3 | 5 | | | | 3 | true | | +| | | | | | | | | | | +| | | | | | | | | | | +`) + + type row struct { Name string - Config string - Value string - ExpectError *regexp.Regexp - Check func(state *terraform.ResourceState) - }{ - // Empty - { - Name: "EmptyString", - Config: ` - data "coder_parameter" "parameter" { - name = "parameter" - type = "string" - } -`, - ExpectError: nil, - Check: func(state *terraform.ResourceState) { - attrs := state.Primary.Attributes - for key, value := range map[string]interface{}{ - "default": "", - "value": "", - "optional": "false", - } { - require.Equal(t, value, attrs[key]) - } - }, - }, - { - Name: "EmptyNumber", - Config: ` - data "coder_parameter" "parameter" { - name = "parameter" - type = "number" + Types []string + InputValue string + Default string + Options []string + Validation *provider.Validation + OutputValue string + Optional bool + Error *regexp.Regexp + } + + rows := make([]row, 0) + lines := strings.Split(table, "\n") + validMinMax := regexp.MustCompile("^[0-9]*-[0-9]*$") + for _, line := range lines[2:] { + columns := strings.Split(line, "|") + columns = columns[1 : len(columns)-1] + for i := range columns { + // Trim the whitespace from all columns + columns[i] = strings.TrimSpace(columns[i]) + } + + if columns[0] == "" { + continue // Skip rows with empty names + } + + optional, err := strconv.ParseBool(columns[8]) + if columns[8] != "" { + // Value does not matter if not specified + require.NoError(t, err) + } + + var rerr *regexp.Regexp + if columns[9] != "" { + rerr, err = regexp.Compile(columns[9]) + if err != nil { + t.Fatalf("failed to parse error column %q: %v", columns[9], err) } -`, - ExpectError: nil, - Check: func(state *terraform.ResourceState) { - attrs := state.Primary.Attributes - for key, value := range map[string]interface{}{ - "default": "", - "value": "", - "optional": "false", - } { - require.Equal(t, value, attrs[key]) - } - }, - }, - // EmptyWithOption - { - Name: "EmptyWithOption", - Config: ` - data "coder_parameter" "parameter" { - name = "parameter" - type = "number" - - option { - name = "option" - value = "5" + } + var options []string + if columns[4] != "" { + options = strings.Split(columns[4], ",") + } + + var validation *provider.Validation + if columns[5] != "" { + // Min-Max validation should look like: + // 1-10 :: min=1, max=10 + // -10 :: max=10 + // 1- :: min=1 + if validMinMax.MatchString(columns[5]) { + parts := strings.Split(columns[5], "-") + min, _ := strconv.ParseInt(parts[0], 10, 64) + max, _ := strconv.ParseInt(parts[1], 10, 64) + validation = &provider.Validation{ + Min: int(min), + MinDisabled: parts[0] == "", + Max: int(max), + MaxDisabled: parts[1] == "", + Monotonic: "", + Regex: "", + Error: "{min} < {value} < {max}", } - } -`, - ExpectError: nil, - Check: func(state *terraform.ResourceState) { - attrs := state.Primary.Attributes - for key, value := range map[string]interface{}{ - "default": "", - "value": "", - "optional": "false", - } { - require.Equal(t, value, attrs[key]) + } else { + validation = &provider.Validation{ + Min: 0, + MinDisabled: true, + Max: 0, + MaxDisabled: true, + Monotonic: "", + Regex: columns[5], + Error: "regex error", } - }, - }, - // DefaultSet - { - Name: "DefaultSet", - Config: ` - data "coder_parameter" "parameter" { - name = "parameter" - type = "number" - default = "5" } -`, - ExpectError: nil, - Check: func(state *terraform.ResourceState) { - attrs := state.Primary.Attributes - for key, value := range map[string]interface{}{ - "default": "5", - "value": "5", - "optional": "true", - } { - require.Equal(t, value, attrs[key]) - } - }, - }, - { - Name: "DefaultSetInOption", - Config: ` - data "coder_parameter" "parameter" { - name = "parameter" - type = "number" - default = "5" - option { - name = "option" - value = "5" + } + + rows = append(rows, row{ + Name: columns[0], + Types: strings.Split(columns[1], ","), + InputValue: columns[2], + Default: columns[3], + Options: options, + Validation: validation, + OutputValue: columns[7], + Optional: optional, + Error: rerr, + }) + } + + stringLiteral := func(s string) string { + if s == "" { + return `""` + } + return fmt.Sprintf("%q", s) + } + + for rowIndex, row := range rows { + for _, rt := range row.Types { + //nolint:paralleltest,tparallel // Parameters load values from env vars + t.Run(fmt.Sprintf("%d|%s:%s", rowIndex, row.Name, rt), func(t *testing.T) { + if row.InputValue != "" { + t.Setenv(provider.ParameterEnvironmentVariable("parameter"), row.InputValue) } - } -`, - ExpectError: nil, - Check: func(state *terraform.ResourceState) { - attrs := state.Primary.Attributes - for key, value := range map[string]interface{}{ - "default": "5", - "value": "5", - "optional": "true", - } { - require.Equal(t, value, attrs[key]) + + var cfg strings.Builder + cfg.WriteString("data \"coder_parameter\" \"parameter\" {\n") + cfg.WriteString("\tname = \"parameter\"\n") + cfg.WriteString(fmt.Sprintf("\ttype = \"%s\"\n", rt)) + if row.Default != "" { + cfg.WriteString(fmt.Sprintf("\tdefault = %s\n", stringLiteral(row.Default))) } - }, - }, - { - Name: "DefaultSetOutOption", - Config: ` - data "coder_parameter" "parameter" { - name = "parameter" - type = "number" - default = "2" - option { - name = "option" - value = "5" + + for _, opt := range row.Options { + cfg.WriteString("\toption {\n") + cfg.WriteString(fmt.Sprintf("\t\tname = %s\n", stringLiteral(opt))) + cfg.WriteString(fmt.Sprintf("\t\tvalue = %s\n", stringLiteral(opt))) + cfg.WriteString("\t}\n") } - } -`, - ExpectError: nil, - Check: func(state *terraform.ResourceState) { - attrs := state.Primary.Attributes - for key, value := range map[string]interface{}{ - "default": "5", - "value": "5", - "optional": "true", - } { - require.Equal(t, value, attrs[key]) + + if row.Validation != nil { + cfg.WriteString("\tvalidation {\n") + if !row.Validation.MinDisabled { + cfg.WriteString(fmt.Sprintf("\t\tmin = %d\n", row.Validation.Min)) + } + if !row.Validation.MaxDisabled { + cfg.WriteString(fmt.Sprintf("\t\tmax = %d\n", row.Validation.Max)) + } + if row.Validation.Monotonic != "" { + cfg.WriteString(fmt.Sprintf("\t\tmonotonic = \"%s\"\n", row.Validation.Monotonic)) + } + if row.Validation.Regex != "" { + cfg.WriteString(fmt.Sprintf("\t\tregex = %q\n", row.Validation.Regex)) + } + cfg.WriteString(fmt.Sprintf("\t\terror = %q\n", row.Validation.Error)) + cfg.WriteString("\t}\n") } - }, - }, - } { - tc := tc - //nolint:paralleltest,tparallel // Parameters load values from env vars - t.Run(tc.Name, func(t *testing.T) { - if tc.Value != "" { - t.Setenv(provider.ParameterEnvironmentVariable("parameter"), tc.Value) - } - resource.Test(t, resource.TestCase{ - ProviderFactories: coderFactory(), - IsUnitTest: true, - Steps: []resource.TestStep{{ - Config: tc.Config, - ExpectError: tc.ExpectError, - Check: func(state *terraform.State) error { - require.Len(t, state.Modules, 1) - require.Len(t, state.Modules[0].Resources, 1) - param := state.Modules[0].Resources["data.coder_parameter.parameter"] - require.NotNil(t, param) - if tc.Check != nil { - tc.Check(param) - } - return nil - }, - }}, + + cfg.WriteString("}\n") + + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: cfg.String(), + ExpectError: row.Error, + Check: func(state *terraform.State) error { + require.Len(t, state.Modules, 1) + require.Len(t, state.Modules[0].Resources, 1) + param := state.Modules[0].Resources["data.coder_parameter.parameter"] + require.NotNil(t, param) + + if row.Default == "" { + _, ok := param.Primary.Attributes["default"] + require.False(t, ok, "default should not be set") + } else { + require.Equal(t, strings.Trim(row.Default, `"`), param.Primary.Attributes["default"]) + } + + if row.OutputValue == "" { + _, ok := param.Primary.Attributes["value"] + require.False(t, ok, "output value should not be set") + } else { + require.Equal(t, strings.Trim(row.OutputValue, `"`), param.Primary.Attributes["value"]) + } + + for key, expected := range map[string]string{ + "optional": strconv.FormatBool(row.Optional), + } { + require.Equal(t, expected, param.Primary.Attributes[key]) + } + + return nil + }, + }}, + }) }) - }) + } } } diff --git a/provider/table_test.go b/provider/table_test.go deleted file mode 100644 index c17db585..00000000 --- a/provider/table_test.go +++ /dev/null @@ -1,56 +0,0 @@ -package provider_test - -import ( - "strconv" - "strings" - "testing" -) - -func TestMDtable(t *testing.T) { - // Copy and paste the table to https://www.tablesgenerator.com/markdown_tables for easier editing - table := strings.TrimSpace(` -| Name | Type | Input Value | Default | Options | Validation | -> | Output Value | Optional | Error | -|------------------|---------------|-------------|-----------|---------|------------|----|--------------|----------|-------| -| Empty | string,number | undefined | undefined | | undefined | | "" | false | - | -| EmptyWithOptions | number | undefined | undefined | | undefined | | "" | false | - | -| DefaultSet | number | undefined | 5 | | undefined | | 5 | true | - | -`) - - type row struct { - Name string - Types []string - InputValue string - Default string - Options string - Validation string - OutputValue string - Optional bool - Error string - } - - rows := make([]row, 0) - lines := strings.Split(table, "\n") - for _, line := range lines[2:] { - columns := strings.Split(line, "|") - columns = columns[1 : len(columns)-2] - for i := range columns { - columns[i] = strings.TrimSpace(columns[i]) - } - - optional, err := strconv.ParseBool(columns[8]) - if err != nil { - t.Fatalf("failed to parse optional column %q: %v", columns[8], err) - } - rows = append(rows, row{ - Name: columns[0], - Types: strings.Split(columns[1], ","), - InputValue: columns[2], - Default: columns[3], - Options: columns[4], - Validation: columns[5], - OutputValue: columns[7], - Optional: optional, - Error: columns[9], - }) - } -} From 2d51bff8064a2b44b087fcadae36ce937d2cd2d8 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 30 Apr 2025 10:54:45 -0500 Subject: [PATCH 3/4] add cases --- provider/parameter_test.go | 90 ++++++++++++++++++++++++++------------ 1 file changed, 61 insertions(+), 29 deletions(-) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 7fde13ae..56fde622 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -696,35 +696,61 @@ data "coder_parameter" "region" { // //nolint:paralleltest,tparallel // Parameters load values from env vars func TestParameterValidationEnforcement(t *testing.T) { + // Some interesting observations: + // - Validation logic does not apply to the value of 'options' + // - [NumDefInvOpt] So an invalid option can be present and selected, but would fail + // - Validation logic does not apply to the default if a value is given + // - [NumIns/DefInv] So the default can be invalid if an input value is valid. + // The value is therefore not really optional, but it is marked as such. + // - [NumInsNotOptsVal | NumsInsNotOpts] values do not need to be in the option set? + table := strings.TrimSpace(` -| Name | Type | Input Value | Default | Options | Validation | -> | Output Value | Optional | Error | -|---------------|---------------|-------------|---------|-------------------|------------|----|--------------|----------|--------------| -| | Empty Vals | | | | | | | | | -| Emty | string,number | | | | | | "" | false | | -| EmtyOpts | string,number | | | 1,2,3 | | | "" | false | | -| EmtyRegex | string | | | | world | | | | regex error | -| EmtyMin | number | | | | 1-10 | | | | 1 < < 10 | -| EmtyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | -| EmtyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | -| EmtyRegexOk | string | | | | .* | | "" | false | | -| | | | | | | | | | | -| | Default Set | No inputs | | | | | | | | -| NumDef | number | | 5 | | | | 5 | true | | -| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | -| NumDefInv | number | | 5 | | 10- | | 5 | | 10 < 5 < 0 | -| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | -| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | -| | | | | | | | | | | -| StrDef | string | | hello | | | | hello | true | | -| StrDefInv | string | | hello | | world | | | | regex error | -| StrDefOpts | string | | a | a,b,c | | | a | true | | -| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | -| StrDefOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | -| | | | | | | | | | | -| | Input Vals | | | | | | | | | -| NumIns | number | 3 | 5 | | | | 3 | true | | -| | | | | | | | | | | -| | | | | | | | | | | +| Name | Type | Input Value | Default | Options | Validation | -> | Output Value | Optional | Error | +|---------------------|---------------|-------------|---------|-------------------|------------|----|--------------|----------|--------------| +| | Empty Vals | | | | | | | | | +| Emty | string,number | | | | | | "" | false | | +| EmtyOpts | string,number | | | 1,2,3 | | | "" | false | | +| EmtyRegex | string | | | | world | | | | regex error | +| EmtyMin | number | | | | 1-10 | | | | 1 < < 10 | +| EmtyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | +| EmtyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | +| EmtyRegexOk | string | | | | .* | | "" | false | | +| | | | | | | | | | | +| | Default Set | No inputs | | | | | | | | +| NumDef | number | | 5 | | | | 5 | true | | +| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | +| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | +| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | +| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | +| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | +| | | | | | | | | | | +| StrDef | string | | hello | | | | hello | true | | +| StrDefInv | string | | hello | | world | | | | regex error | +| StrDefOpts | string | | a | a,b,c | | | a | true | | +| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | +| StrDefOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | +| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | +| | | | | | | | | | | +| | Input Vals | | | | | | | | | +| NumIns | number | 3 | | | | | 3 | false | | +| NumInsDef | number | 3 | 5 | | | | 3 | true | | +| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | +| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | +| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | +| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | 3 | true | | +| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | +| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | 3 | true | | +| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | 3 | false | | +| | | | | | | | | | | +| StrIns | string | c | | | | | c | false | | +| StrInsDef | string | c | e | | | | c | true | | +| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | +| NumIns=DefInv | string | e | e | | [a-c] | | | | regex error | +| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | +| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | c | true | | +| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | +| StrInsNotOpts | string | c | e | a,b,d,e | | | c | true | | +| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | c | false | | `) type row struct { @@ -832,6 +858,12 @@ func TestParameterValidationEnforcement(t *testing.T) { t.Setenv(provider.ParameterEnvironmentVariable("parameter"), row.InputValue) } + if row.Error != nil { + if row.OutputValue != "" { + t.Errorf("output value %q should not be set if error is set", row.OutputValue) + } + } + var cfg strings.Builder cfg.WriteString("data \"coder_parameter\" \"parameter\" {\n") cfg.WriteString("\tname = \"parameter\"\n") @@ -896,7 +928,7 @@ func TestParameterValidationEnforcement(t *testing.T) { for key, expected := range map[string]string{ "optional": strconv.FormatBool(row.Optional), } { - require.Equal(t, expected, param.Primary.Attributes[key]) + require.Equal(t, expected, param.Primary.Attributes[key], "optional") } return nil From cec2d970ad44291b21fd40c51eca69336ef561f0 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 30 Apr 2025 11:39:13 -0500 Subject: [PATCH 4/4] extract tests to a file --- provider/parameter_test.go | 61 ++++-------------------- provider/testdata/parameter_table.md | 70 ++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 51 deletions(-) create mode 100644 provider/testdata/parameter_table.md diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 56fde622..37a46796 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -2,6 +2,7 @@ package provider_test import ( "fmt" + "os" "regexp" "strconv" "strings" @@ -703,55 +704,8 @@ func TestParameterValidationEnforcement(t *testing.T) { // - [NumIns/DefInv] So the default can be invalid if an input value is valid. // The value is therefore not really optional, but it is marked as such. // - [NumInsNotOptsVal | NumsInsNotOpts] values do not need to be in the option set? - - table := strings.TrimSpace(` -| Name | Type | Input Value | Default | Options | Validation | -> | Output Value | Optional | Error | -|---------------------|---------------|-------------|---------|-------------------|------------|----|--------------|----------|--------------| -| | Empty Vals | | | | | | | | | -| Emty | string,number | | | | | | "" | false | | -| EmtyOpts | string,number | | | 1,2,3 | | | "" | false | | -| EmtyRegex | string | | | | world | | | | regex error | -| EmtyMin | number | | | | 1-10 | | | | 1 < < 10 | -| EmtyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | -| EmtyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | -| EmtyRegexOk | string | | | | .* | | "" | false | | -| | | | | | | | | | | -| | Default Set | No inputs | | | | | | | | -| NumDef | number | | 5 | | | | 5 | true | | -| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | -| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | -| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | -| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | -| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | -| | | | | | | | | | | -| StrDef | string | | hello | | | | hello | true | | -| StrDefInv | string | | hello | | world | | | | regex error | -| StrDefOpts | string | | a | a,b,c | | | a | true | | -| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | -| StrDefOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | -| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | -| | | | | | | | | | | -| | Input Vals | | | | | | | | | -| NumIns | number | 3 | | | | | 3 | false | | -| NumInsDef | number | 3 | 5 | | | | 3 | true | | -| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | -| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | -| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | -| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | 3 | true | | -| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | -| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | 3 | true | | -| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | 3 | false | | -| | | | | | | | | | | -| StrIns | string | c | | | | | c | false | | -| StrInsDef | string | c | e | | | | c | true | | -| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | -| NumIns=DefInv | string | e | e | | [a-c] | | | | regex error | -| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | -| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | c | true | | -| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | -| StrInsNotOpts | string | c | e | a,b,d,e | | | c | true | | -| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | c | false | | -`) + table, err := os.ReadFile("testdata/parameter_table.md") + require.NoError(t, err) type row struct { Name string @@ -766,7 +720,7 @@ func TestParameterValidationEnforcement(t *testing.T) { } rows := make([]row, 0) - lines := strings.Split(table, "\n") + lines := strings.Split(string(table), "\n") validMinMax := regexp.MustCompile("^[0-9]*-[0-9]*$") for _, line := range lines[2:] { columns := strings.Split(line, "|") @@ -867,7 +821,12 @@ func TestParameterValidationEnforcement(t *testing.T) { var cfg strings.Builder cfg.WriteString("data \"coder_parameter\" \"parameter\" {\n") cfg.WriteString("\tname = \"parameter\"\n") - cfg.WriteString(fmt.Sprintf("\ttype = \"%s\"\n", rt)) + if rt == "multi-select" || rt == "tag-select" { + cfg.WriteString(fmt.Sprintf("\ttype = \"%s\"\n", "list(string)")) + cfg.WriteString(fmt.Sprintf("\tform_type = \"%s\"\n", rt)) + } else { + cfg.WriteString(fmt.Sprintf("\ttype = \"%s\"\n", rt)) + } if row.Default != "" { cfg.WriteString(fmt.Sprintf("\tdefault = %s\n", stringLiteral(row.Default))) } diff --git a/provider/testdata/parameter_table.md b/provider/testdata/parameter_table.md new file mode 100644 index 00000000..4c9ee458 --- /dev/null +++ b/provider/testdata/parameter_table.md @@ -0,0 +1,70 @@ +| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | Error | +|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|--------------| +| | Empty Vals | | | | | | | | | +| Empty | string,number | | | | | | "" | false | | +| EmptyList | list(string) | | | | | | "" | false | | +| EmptyMulti | tag-select | | | | | | "" | false | | +| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | +| EmptyRegex | string | | | | world | | | | regex error | +| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | +| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | +| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | +| EmptyRegexOk | string | | | | .* | | "" | false | | +| | | | | | | | | | | +| | Default Set | No inputs | | | | | | | | +| NumDef | number | | 5 | | | | 5 | true | | +| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | +| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | +| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | +| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | +| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | +| | | | | | | | | | | +| StrDef | string | | hello | | | | hello | true | | +| StrDefInv | string | | hello | | world | | | | regex error | +| StrDefOpts | string | | a | a,b,c | | | a | true | | +| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | +| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | +| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | +| | | | | | | | | | | +| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | +| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | +| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | +| | | | | | | | | | | +| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | +| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | +| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | +| | | | | | | | | | | +| | Input Vals | | | | | | | | | +| NumIns | number | 3 | | | | | 3 | false | | +| NumInsDef | number | 3 | 5 | | | | 3 | true | | +| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | +| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | +| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | +| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | 3 | true | | +| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | +| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | 3 | true | | +| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | 3 | false | | +| | | | | | | | | | | +| StrIns | string | c | | | | | c | false | | +| StrInsDef | string | c | e | | | | c | true | | +| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | +| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | +| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | +| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | c | true | | +| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | +| StrInsNotOpts | string | c | e | a,b,d,e | | | c | true | | +| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | c | false | | +| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | +| | | | | | | | | | | +| | list(string) | | | | | | | | | +| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | +| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | +| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | +| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | +| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | ["c"] | true | | +| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | ["c"] | false | | +| | | | | | | | | | | +| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | +| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | ["c"] | true | | +| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | ["c"] | false | | +| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file