testscript: test custom conditions #175
Conversation
Add a test for custom conditions: * two without arguments * two requiring an argument The companion script shows how the condition work when they match, when they don't match, and when they match but are negated.
datacharmer
changed the title
testscript/doc.go
Outdated
| if len(args) < 2 { | ||
| return false, fmt.Errorf("syntax: [is_upper:word]") | ||
| } | ||
| return strings.ToUpper(args[1]) == args[1], nil |
There was a problem hiding this comment.
to make the example with an argument a bit more realistic, how about a variant of exists? Like the built-in command, but being useful as a condition, like [exists:foo/bar] mycmd foo/bar.
case "exists":
_, err := os.Lstat(args[1])
return err == nil, nil
testscript/doc.go
Outdated
| } | ||
| return strings.ToUpper(args[1]) == args[1], nil | ||
| case "always_true": | ||
| return true, nil |
There was a problem hiding this comment.
similar to my other comment, I'd like this to be slightly more realistic: in practice I don't think anyone would write an "always true" condition :)
How about one that's tied to an env var? For example, I've used "slow" or "long" env vars to tell go test to do more extensive testing, kinda like the opposite of go test -short. So you could have a [long] that checks os.Getenv("TEST_LONG") == "true", for instance.
| @@ -120,6 +120,35 @@ when testing.Short() is false. | |||
|
|
|||
| Additional conditions can be added by passing a function to Params.Condition. | |||
|
|
|||
| An example: | |||
There was a problem hiding this comment.
No need for this to be a separate paragraph, IMO. You can attach it to the previous paragraph.
| An example: | ||
|
|
||
| Condition: func(cond string) (bool, error) { | ||
| // Assume condition name and args are separated by colon (":") |
There was a problem hiding this comment.
I think that parsing the condition here is somewhat overkill tbh, and makes the example a bit longer than it needs to be. I'm pretty sure that people writing conditions are up to the task of extrapolating from something a bit simpler.
How about just something like:
Condition: func(cond string) (bool, error) {
// Note: conditions can be more complex than this (for example [exists:/some/file]).
if cond != "someCondition" {
return false, fmt.Errorf("unknown condition")
}
// someCondition is a place-holder for any conditional logic you might want.
return someCondition(), nil
}
?
There was a problem hiding this comment.
I can do that, if everyone agrees. However, I wanted the example to include some text manipulation logic, because I was baffled when I first tried to define a condition. What was unclear to me was that custom commands are defined as a map of functions, while custom conditions are just one function for all.
I think the current example will save users some grief.
There was a problem hiding this comment.
For what it's worth, I also feel this is likely to confuse people. But perhaps what this is really telling us is that the API should be a map of condition names to func(string) (bool, error). What do you think?
| @@ -120,6 +120,35 @@ when testing.Short() is false. | |||
|
|
|||
| Additional conditions can be added by passing a function to Params.Condition. | |||
There was a problem hiding this comment.
Maybe worth mentioning this:
The function will only be called when all built-in conditions have been checked for.
| @@ -0,0 +1,67 @@ | |||
| [!exec:echo] skip | |||
There was a problem hiding this comment.
It would be nice to have a test that for the custom condition function returning an error, but that might be a bit awkward to do.
There was a problem hiding this comment.
I couldn't find a way to do that.
There was a problem hiding this comment.
UPDATE: I found a way to test the errors (see testdata/custom_conditions_errors.txt).
Admittedly, it's a hack. It may not be resilient enough to certify future code update.
|
I found this PR while looking for discussions about custom conditions in testscript. I could not find in this PR any doc that would explain its purpose and use cases; reading the proposed code changes lead me to think that its purpose is different from mine. EDIT: I removed further comments from here, and opened an issue Feature proposal: use environment variables as custom conditions in a txtar script #241 |
Add a test for custom conditions and a brief example in the documentation.