Skip to content
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

Add implicit param HasCallStack to assertions #1421

Merged
merged 1 commit into from Jul 20, 2017

Conversation

@sestrella
Copy link
Contributor

commented Jul 19, 2017

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

This comment has been minimized.

Copy link
Member

commented Jul 19, 2017

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

@sestrella

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2017

@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

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2017

@snoyberg all changes are in place.

@snoyberg

This comment has been minimized.

Copy link
Member

commented Jul 20, 2017

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

@sestrella

This comment has been minimized.

Copy link
Contributor Author

commented Jul 20, 2017

@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

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@snoyberg

This comment has been minimized.

Copy link
Member

commented Jul 20, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.