Add expect_syntax_error spec helper#1188
Conversation
amomchilov
force-pushed
the
expect_syntax_error
branch
from
096c2a1 to
7849c33
Compare
| MSpecRun.main | ||
| end | ||
|
|
||
| def expect_syntax_error(ruby_src) |
There was a problem hiding this comment.
Is this the correct place to put a helper like this? Or should it go in mspec?
There was a problem hiding this comment.
It definitely feels like the wrong place.
Adding expect_syntax_error in MSpec feels wrong too, first MSpec never use the term expect and it seems far too specific.
You could define it in some fixtures file under language maybe.
But I'm not sure I see the value of this helper, the current code works fine, it's a bit verbose but it's also consistent with all other matchers for expected exceptions.
There was a problem hiding this comment.
IMO duplication in tests is rarely a problem, especially if it's something pretty trivial like this.
It's often even a feature as it makes easier to find out where the error happened and have all the context around vs in N helper methods called around.
There was a problem hiding this comment.
IMO the existing code added a ton of noise that distract from the actual example code, and turns it into a huge -> ({} { ) _} }) {} {} ) sigil soup. But that's a matter of personal taste, so we can it as-is
2 participants
This was a really common pattern that I needed to repeat several times in #1187.
I noticed it repeated a bunch (69 times), and could be simplified really nicely with this little helper method.