-
Notifications
You must be signed in to change notification settings - Fork 109
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
Pretty printing for HUnitFailure
#327
Comments
This affects me too. It's particularly problematic when the FWIW I'm currently hacking my way around that with some -- Catches a HUnit test failure, if the test fails.
assertionToMaybe :: HU.Assertion -> IO (Maybe HU.HUnitFailure)
assertionToMaybe = flip E.catches [E.Handler $ return . Just] . (>> return Nothing)
testCounterexample :: String -> HU.Assertion -> HU.Assertion
testCounterexample msg = maybe testSuccess (E.throw . adjustMsg) <=< assertionToMaybe
where
joinMsg :: String -> String
joinMsg rest = msg ++ "; " ++ rest
adjustMsg :: HU.HUnitFailure -> HU.HUnitFailure
adjustMsg (HU.HUnitFailure loc (HU.Reason txt)) =
unsafePerformIO $ do
putStrLn ('\n' : joinMsg txt)
return $ HU.HUnitFailure loc (HU.Reason "")
adjustMsg (HU.HUnitFailure loc (HU.ExpectedButGot pref x y)) =
HU.HUnitFailure loc (HU.ExpectedButGot (maybe (Just msg) (Just . joinMsg) pref) x y) It's not perfect, but at least it preserves the newlines of the |
Yes, it would be nice if |
Hey, sorry for the silence. See https://github.com/UnkindPartition/UnkindPartition/blob/main/README.md. I trust highly all 3 of you, so I'm going to add you as contributors to the repo. Please take care of it while I'm away. And feel free to ask @ocharles or hackage admins to add you as maintainers there. |
Also added @Bodigrim, whom I also trust and who's been helping a lot with tasty. |
Thanks @UnkindPartition; I'll set up a PR on this next week :) |
Roman, I pray that you get through this terrible situation unharmed. |
OMG 😨 ! I just read this. I make Andreas words mine, my thoughts are with you Roman. |
So I started working on this today but I couldn't reproduce the problem. Eventually I discovered that @andreasabel and I did something wrong. What we did is some variation on: import qualified Test.HUnit as Raw
import qualified Test.Tasty.HUnit as Tasty
t1 = Tasty.testCase "Some Test" $ Raw.assertFailure "Omg" The issue comes from the fact that The solution is to not depend on |
Following the discussion at UnkindPartition/tasty#327 (comment) @andreasabel wrote: There is a `Show` instance for `HUnitFailure` which gives the _Haskell_ representation of this exception. So far so good. I am looking for a function that _pretty-prints_ the exception, for presentation to the user. It should look nicer than what `Show` gives me: ``` Exception: HUnitFailure (Just (SrcLoc {srcLocPackage = "main", srcLocModule = "Main", srcLocFile = "tests/PackageTestMain.hs", srcLocStartLine = 127, srcLocStartCol = 3, srcLocEndLine = 127, srcLocEndCol = 17})) (Reason "Expected success but got: \"correct-package-0.1.0.0/correct-package.cabal:11:34: \\nunexpected Unknown cabal spec version specified: 1.10123456\\nexpecting \\\".\\\", \\\"-\\\", white space, \\\"&&\\\" or \\\"|| \\\"\\n\"") ``` @VictorCMiraldo wrote: I started working on this today but I couldn't reproduce the problem. Eventually I discovered that @andreasabel and I did something wrong. What we did is some variation on: ```haskell import qualified Test.HUnit as Raw import qualified Test.Tasty.HUnit as Tasty t1 = Tasty.testCase "Some Test" $ Raw.assertFailure "Omg" ``` The issue comes from the fact that `Raw.HUnitAssertion` is a different type altogether from [`Tasty.HUnitAssertion`](https://github.com/UnkindPartition/tasty/blob/16289a77495eb8279c5e544886ef52503becd148/hunit/Test/Tasty/HUnit/Orig.hs#L136). Because `Tasty.Assertion == Raw.Assertion == IO ()`, and failures are communicated through exceptions, we're bypassing the `try` on [`Tasty.HUnit.run`](https://github.com/UnkindPartition/tasty/blob/master/hunit/Test/Tasty/HUnit.hs#L102) and flagging the test as failed through a random exception, since that method is `try`ing on `Tasty.HUnitAssertion`. The solution is to *not* depend on `HUnit`, depend exclusively on `tasty-hunit` and use the functions from there. Your test failures will be printed properly then.
@VictorCMiraldo : Thanks for the excellent analysis and the cookbook! Indeed, this fixes my issue, see: |
Following the discussion at UnkindPartition/tasty#327 (comment) @andreasabel wrote: There is a `Show` instance for `HUnitFailure` which gives the _Haskell_ representation of this exception. So far so good. I am looking for a function that _pretty-prints_ the exception, for presentation to the user. It should look nicer than what `Show` gives me: ``` Exception: HUnitFailure (Just (SrcLoc {srcLocPackage = "main", srcLocModule = "Main", srcLocFile = "tests/PackageTestMain.hs", srcLocStartLine = 127, srcLocStartCol = 3, srcLocEndLine = 127, srcLocEndCol = 17})) (Reason "Expected success but got: \"correct-package-0.1.0.0/correct-package.cabal:11:34: \\nunexpected Unknown cabal spec version specified: 1.10123456\\nexpecting \\\".\\\", \\\"-\\\", white space, \\\"&&\\\" or \\\"|| \\\"\\n\"") ``` @VictorCMiraldo wrote: I started working on this today but I couldn't reproduce the problem. Eventually I discovered that @andreasabel and I did something wrong. What we did is some variation on: ```haskell import qualified Test.HUnit as Raw import qualified Test.Tasty.HUnit as Tasty t1 = Tasty.testCase "Some Test" $ Raw.assertFailure "Omg" ``` The issue comes from the fact that `Raw.HUnitAssertion` is a different type altogether from [`Tasty.HUnitAssertion`](https://github.com/UnkindPartition/tasty/blob/16289a77495eb8279c5e544886ef52503becd148/hunit/Test/Tasty/HUnit/Orig.hs#L136). Because `Tasty.Assertion == Raw.Assertion == IO ()`, and failures are communicated through exceptions, we're bypassing the `try` on [`Tasty.HUnit.run`](https://github.com/UnkindPartition/tasty/blob/master/hunit/Test/Tasty/HUnit.hs#L102) and flagging the test as failed through a random exception, since that method is `try`ing on `Tasty.HUnitAssertion`. The solution is to *not* depend on `HUnit`, depend exclusively on `tasty-hunit` and use the functions from there. Your test failures will be printed properly then.
I'll close this then, it wasn't really an issue to begin with :) I'll eventually send a PR to update the documentation of |
A better solution would be to depend on upstream Alternatively, it may make sense to rename For historic context, I'm not sure for what reason @UnkindPartition decided to ship his own copy of HUnit, but at that time HUnit was not on GitHub and possibly he was concerned about its maintenance. That's not the case anymore, and hasn't been for the last 7 years. @bergmark and myself are maintaining |
That's a reasonable suggestion @sol, I could not find any info on the commit that dropped the dependency on Let's give some time for @UnkindPartition to have a chance to chime in. I'm happy to review/shepherd a pr bringing |
I'd rather refrain from architectural changes without @UnkindPartition's explicit consent. It was a conscious choice, and I believe the intention was to evolve API separately (see #262 and comments about deprecated functions). Thanks to modular architecture, one can use Also remember that PVP essentially discourages re-exports from other packages, because otherwise it's difficult to maintain a stable interface. |
There were two reasons afair (my bad for not mentioning them in the commit message):
(1) is no longer relevant, white (2) still is. However, I realize that this decision has caused some confusion and pain over the years, so if the consensus here (esp. among the new maintainers) is to switch back, so be it. Yet another option would be to make use of the extensible exceptions system such that the packages wouldn't need to depend on each other. E.g. insert |
A good example of the latter would be quickcheck btw. There are people who use hunit and/or hspec operators inside monadic QC properties, not realizing that it's not quite what they want. If quickcheck also started to treat that Proxy exception in a special way, that usage pattern could start working as expected without introducing extra dependencies for quickcheck. |
First of all, @UnkindPartition, like the others I want to wish you all the best in what must be an incredibly difficult time. On more technical matters I'm reopening this issue because while #327 (comment) is correct, I still think tasty/hunit/Test/Tasty/HUnit/Steps.hs Lines 29 to 30 in efe0ad2
Line 116 in efe0ad2
show on exceptions (modulo the special handling for (tasty 's version of) HUnitFailure ). Using displayException instead would give the more user-friendly output regardless of whether the specific exception being thrown was a HUnitFailure from tasty , from HUnit , or a completely different exception.
|
Since @UnkindPartition has agreed that it is possible to switch back to If we reach a consensus, I'm happy to implement this. |
I do not have an opinion on this, so +1 by default. |
I think the dependency footprint of |
I have concerns about re-exports from other packages, but AFK, will be back next week. |
My concern is that a specific version of a package should provide a stable API, whatever build plan. This means that for re-exported functions each version of the proposed Yes, |
I thought it's only the case when the instances are orphans, and in that case it would be a major bump anyway? If we depend on Moreover, if we were to run out of maintenance resources, we get a restrictive upper bound, which I don't think is worse (nor necessarily better) than an outdated API, which would be the case if we don't depend on |
No. Imagine re-exporting |
Getting back to this discussion, I must admit that I phased out It seems that everyone else were supportive of switching to vanilla |
Closing, the future of |
There is a
Show
instance forHUnitFailure
which gives the Haskell representation of this exception. So far so good.I am looking for a function that pretty-prints the exception, for presentation to the user.
It should look nicer than what
Show
gives me:The text was updated successfully, but these errors were encountered: