Catching invalid step functions #61
Conversation
| t tester | ||
| steps []stepDef | ||
| options SuiteOptions | ||
| stepsErrors []error |
There was a problem hiding this comment.
Steps could be validated in Run as well. What's the reason for caching errors?
There was a problem hiding this comment.
to make the Run function as small as possible. There's no another way of adding steps than using AddStep*.
The second reason why I did it is that (maybe in the future) we'll want to add more checks before calling the Run. Then, the only thing we'll need to do is to rename it.
There was a problem hiding this comment.
Hm...I still think it might better be to run checks in the beginning of Run. Eg. validateSteps(). It would keep both Run and AddStep* clean.
There was a problem hiding this comment.
I think it's not so simple. When working with regex's, we have to compile them. If we do it in AddStep then we have to or use MustCompile (which throws a panic) or regular Compile but we have to store the error somewhere. It means, there have to be some if err != nil in the function.
Or... store strings (not compiled) and try to compile them in the validateSteps() function. But IMO it would complicate things even more.
By doing it this way, we're at least consistent with the behaviour.
There was a problem hiding this comment.
Ah, okay, I see. Well, it's an internal detail, so it should be fine.
gobdd.go
Outdated
| @@ -91,8 +92,17 @@ type stepDef struct { | |||
| f interface{} | |||
| } | |||
|
|
|||
| type tester interface { | |||
There was a problem hiding this comment.
This type should be exported, since it's used in an exported function. I would probably keep calling it T or TestingT (as seen in testify).
There was a problem hiding this comment.
good point! changed 👍
Fixes #48
FYI @sagikazarmark