- Notifications
You must be signed in to change notification settings - Fork22
test: unit test to document validation behavior of parameters#387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| }, { | ||
| 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, | ||
| }} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Added some extra test cases down here too
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| 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) | ||
| } | ||
| } | ||
| 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}", | ||
| } | ||
| } else { | ||
| validation = &provider.Validation{ | ||
| Min: 0, | ||
| MinDisabled: true, | ||
| Max: 0, | ||
| MaxDisabled: true, | ||
| Monotonic: "", | ||
| Regex: columns[5], | ||
| Error: "regex error", | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
suggestion: Extract this to atestutil package maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Since it cannot be reused outside the test, I am going to leave it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Some comments below, but these are optional for a new unit test.
ac2f9bf intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Locking in the behavior of validations before I change things. My goal is to move all validation logic into this provider. At present, some of it is done in
coder/coder.If you see#381, adding more strict validation would be a regression in behavior. So I might have to add 2 validation modes so that template import can still be done with relaxed valiation.
Regardless, this test will help illustrate what I change when I add stricter validation.
I confirmed this test passes on a commit 3 weeks ago, before any of my refactors.
Some things we should probably fix or document:
coder/coderNOTE: Monotonic is not yet checked, it should be added into the terraform