Skip to content

Add implicit param HasCallStack to assertions#1421

Merged
snoyberg merged 1 commit intoyesodweb:masterfrom
sestrella:add_has_call_stack_to_assertions
Jul 20, 2017
Merged

Add implicit param HasCallStack to assertions#1421
snoyberg merged 1 commit intoyesodweb:masterfrom
sestrella:add_has_call_stack_to_assertions

Conversation

@sestrella
Copy link
Copy Markdown
Contributor

yesod-test current error messages looks like follows:

./Yesod/Test.hs:351: 
  1) testing status code
       Expected status was 401 but received status was 400

The error message is not easy to localize since it points to the module and line number where the assertion is defined instead of the place where it is called.

Adding HasCallStack to each assertion fix the error message since it passes the CallStack to the HUnit matchers used by yesod-test under the hood. Here is an example of how an error message looks before and after the changes introduced by this PR:

Before

./Yesod/Test.hs:351: 
  1) testing status code
       Expected status was 401 but received status was 400

After

test/Handler/FooSpec.hs:24: 
  1) testing status code
       Expected status was 401 but received status was 400

@snoyberg
Copy link
Copy Markdown
Member

Would you be able to switch this over to using conditional compilation for older GHCs? I'd rather that than add another dependency.

@sestrella
Copy link
Copy Markdown
Contributor Author

sestrella commented Jul 19, 2017

@snoyberg thank you for the comment. I guess I could copy some of the content (just HasCallStack type alias) of Data.CallStack defined at call-stack into Yesod.Test module. Data.CallStack module looks like follows:

#if MIN_VERSION_base(4,8,1)
import qualified GHC.Stack as GHC
#endif

#if MIN_VERSION_base(4,9,0)
import           GHC.Stack (HasCallStack)
#elif MIN_VERSION_base(4,8,1)
type HasCallStack = (?callStack :: GHC.CallStack)
#else
import GHC.Exts (Constraint)
type HasCallStack = (() :: Constraint)
#endif

type CallStack = [(String, SrcLoc)]

callStack :: HasCallStack => CallStack
#if MIN_VERSION_base(4,9,0)
callStack = drop 1 $ GHC.getCallStack GHC.callStack
#elif MIN_VERSION_base(4,8,1)
callStack = drop 2 $ GHC.getCallStack ?callStack
#else
callStack = []
#endif

However, I'm actually exposing a hidden package rather than adding a new dependency since call-stack is a dependency of HUnit already. What do you think?

@sestrella
Copy link
Copy Markdown
Contributor Author

@snoyberg taking a second look, I found this change is not compatible with older stack resolvers such as lts-3 since call-stack is not part of it. I'm going to add the type alias HasCallStack to Yesod.Test using conditional compilation as you originally suggested.

@sestrella
Copy link
Copy Markdown
Contributor Author

@snoyberg all changes are in place.

@snoyberg
Copy link
Copy Markdown
Member

Looks good. Could you also do a minor version bump and update the changelog?

@sestrella
Copy link
Copy Markdown
Contributor Author

@snoyberg sure thing! I just added a new entry to the CHANGELOG and bumped the minor version. Thank you for taking a look.

@snoyberg snoyberg merged commit eb3c570 into yesodweb:master Jul 20, 2017
@snoyberg
Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants