-
Notifications
You must be signed in to change notification settings - Fork 2
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Navigate the callstack better when creating custom assertions #12
Conversation
for ok { | ||
var fw *frameWrapper | ||
if fw, ok = localT.(*frameWrapper); ok { | ||
numFrames++ |
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.
@krancour let me suggest an addition:
- Make
frameWrapper
exported - Make the number of frames to traverse configurable on the newly exported
FrameWrapper
- Increment by that number of frames 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.
So I have now amended the commit with the following modifications:
withFrameWrapper(t)
now wraps t
with a frameWrapper
, which now includes a numFrames
attribute. If t
is a frameWrapper
to begin with, .withFrameWrapper(t)
returns a new frameWrapper
with an incremented numFrames
, wrapping the t
referenced by the original frameWrapper
. In short-- the result is there's never more than one layer of wrapping.
Re: making frameWrapper
exported, I can and will if that is what you want, of course, but I'm wondering if you can explain the reasoning? fwiw, withFrameWrapper(t)
just returns the exported Tester
interface, which afaik is all that any caller should care about. i.e. I'm assuming we don't want people actually coding directly to frameWrapper
type.
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.
@krancour IMO the way you have it now is great! I like the fact that one can recursively wrap Tester
s, which can easily parallels the layout of code-under-test's stack. 👍
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.
Thanks. 😄
@arschles just in case it gets overlooked, I commented on the outdated diff above. |
Also, fwiw, CI failures appear unrelated to this change. |
Closes #11
This adds a new bit of functionality that makes it possible to create test helpers (custom assertions, really) without worrying that the call stack won't be unwound far enough when the source of the assertion failure is displayed.
See addition to
examples/simple_test.go
for sample usage.