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

Use displayException for HUnitFailure #55

Open
adamgundry opened this issue May 2, 2022 · 5 comments
Open

Use displayException for HUnitFailure #55

adamgundry opened this issue May 2, 2022 · 5 comments

Comments

@adamgundry
Copy link

The Exception HUnitFailure instance does not currently implement displayException. It would be nice if it did, and if performTestCase called displayException rather than show when rendering an exception.

Related tasty issue: UnkindPartition/tasty#327

@sol
Copy link
Member

sol commented May 4, 2022

It would be nice ... if performTestCase called displayException rather than show when rendering an exception.

performTestCase does not call show on HUnitFailure. Are you suggesting to call show on exceptions other than HUnitFailure? If yes, can you motivate how this is preferable from a user's perspective (bonus points for actual code that I can play around with).

@adamgundry
Copy link
Author

Yes. Here's an example:

#!/usr/bin/env cabal
{- cabal:
build-depends: base, HUnit
-}

import Control.Exception
import Test.HUnit

-- User code defines an exception type, with a displayException implementation
-- that explains how it should be presented to users
data MyException = MyException String
  deriving Show

instance Exception MyException where
  displayException (MyException _) = "Some nice message"

-- HUnit test suite where one of the tests accidentally throws an error
main = runTestTT $ TestCase (throwIO (MyException "Some tedious internal details"))

At the moment, this test suite fails with

### Error:
MyException "Some tedious internal details"

whereas (I claim) it would be better to see "Some nice message" instead.

The point here is that displayException gives a standard way to pretty-print unknown exceptions, without the test runner needing to know about the exception type.

@sol
Copy link
Member

sol commented May 8, 2022

Yes. Here's an example:

#!/usr/bin/env cabal

{- cabal:

build-depends: base, HUnit

-}



import Control.Exception

import Test.HUnit



-- User code defines an exception type, with a displayException implementation

-- that explains how it should be presented to users

data MyException = MyException String

  deriving Show



instance Exception MyException where

  displayException (MyException _) = "Some nice message"



-- HUnit test suite where one of the tests accidentally throws an error

main = runTestTT $ TestCase (throwIO (MyException "Some tedious internal details"))

At the moment, this test suite fails with


### Error:

MyException "Some tedious internal details"

whereas (I claim) it would be better to see "Some nice message" instead.

The point here is that displayException gives a standard way to pretty-print unknown exceptions, without the test runner needing to know about the exception type.

Is this motivated by practical need for it. Say, did you run into a situation where you would have preferred the internal details to be omitted. I would be very interested to learn in what kind of situation one would not want to know all the details that led to a test failure.

@adamgundry
Copy link
Author

The practical motivation arises from refactoring a commercial Haskell codebase, where we're trying to make test failure messages easier to diagnose. One aspect of this includes attaching backtraces to exceptions (using HasCallStack), where we really want to pretty-print the backtrace rather than show its internal structure. Another case that arises is where a HTTP client library throws an exception containing lots of details about the failed request, but we want to focus attention on the reason for the failure (e.g. the system under test didn't respond to a connection attempt, or it did respond but generated an error response, or ...).

Note that displayException by default is the same as show anyway, so unless someone actively overrides it, it will include all the same details.

@sol
Copy link
Member

sol commented May 12, 2022

@adamgundry ok, for exceptions that provide a lot of context, like failed HTTP requests, I see how that can be convenient in some situations.

Now, what are the cons? Let’s assume, you write some code and you have a test case that fails with an exception. If you want to handle that exception in your code then you need to know the type of that exception. Your best bet is to rely on the Show instance to figure out the type**.

So this is a trade-off where I’m not exactly sure where the sweet spot is.

Hspec does not use performTestCase, so personally I don’t care that much. Are you going to use it? If yes, that’s in support of changing the existing behavior. However, if you’re not going to use it for tasty then I don’t think it’s worth introducing a breaking change.

As for displayException for HUnitFailure, I think that won’t hurt. PRs welcome.

** This works with most third-party libraries, but not so much for exceptions from base (as they don’t derive Show 🤦).

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

No branches or pull requests

2 participants