Prettified shouldBe

[bitemyapp]

Cf. yesodweb/yesod#1259

Why?

Before:

After:

Takes up more space, but is more readable. YMMV.

I'd like to figure out a good (SYB/Generics driven, ideally) diff solution for a future PR, suggestions to this end would be appreciated.

[soenkehahn]

This is awesome!

Regarding a better diff solution: I would think that a pretty-printed output like done in this PR combined with a textual diff would go a long way in making test failures more readable. (@sol made an effort to have textual diffs, not sure about the state of that.)

[soenkehahn]

One thing that I'm worrying about is this: We should be very careful when adding new dependencies to hspec. Since lots of packages depend (in the tests) on hspec, this has a huge impact. Also consider that some CI solutions don't have caching and build all hspec deps in every run.

Added (transitive) dependencies for this PR are:

• haskell-lexer
• pretty
• pretty-show

[bitemyapp]

@soenkehahn pretty-show is the smallest and most effective library I could find to do this, particularly given the constraint that I wanted to keep it within what (Eq a, Show a) => a can accomplish. From that it's not hard to see why it has to lex Haskell datatypes to prettify them correctly.

Regarding a better diff solution: I would think that a pretty-printed output like done in this PR combined with a textual diff would go a long way in making test failures more readable. (@sol made an effort to have textual diffs, not sure about the state of that.)

I haven't found anything good or generic yet, which is unfortunate. I could content myself to a textual diff as long as it was intelligent about including context and was pure Haskell but I haven't found that yet. One of the libraries I found shelled out to colordiff :\

[soenkehahn]

@bitemyapp: Re dependencies: Yeah, makes sense. Adding more class constraints would indeed be bad. I'd like to know how long it takes to build the additional dependencies. (I'm planning to measure this, if you don't get to it first.)

[bitemyapp]

@soenkehahn go for it.

[sol]

Guys, it would be awesome to have something like this on master.

This needs to be rebased, and we still need to measure the build time impact.

Anybody wants to help with this?

[tfausak]

I'm not sure why this branch has conflicts with master now. My guess is the Cabal file.

A current diff

diff --git a/hspec-expectations.cabal b/hspec-expectations.cabal
index 4bab9bf..0d422f1 100644
--- a/hspec-expectations.cabal
+++ b/hspec-expectations.cabal
@@ -1,21 +1,21 @@
--- This file has been generated from package.yaml by hpack version 0.14.1.
+-- This file has been generated from package.yaml by hpack version 0.14.0.
 --
 -- see: https://github.com/sol/hpack

-name:             hspec-expectations
-version:          0.7.2
-synopsis:         Catchy combinators for HUnit
-description:      Catchy combinators for HUnit: <https://github.com/hspec/hspec-expectations#readme>
-bug-reports:      https://github.com/hspec/hspec-expectations/issues
-license:          MIT
-license-file:     LICENSE
-copyright:        (c) 2011-2015 Simon Hengel
-author:           Simon Hengel <sol@typeful.net>
-maintainer:       Simon Hengel <sol@typeful.net>
-build-type:       Simple
-category:         Testing
-cabal-version:    >= 1.10
-homepage:         https://github.com/hspec/hspec-expectations#readme
+name:           hspec-expectations
+version:        0.7.2
+synopsis:       Catchy combinators for HUnit
+description:    Catchy combinators for HUnit: <https://github.com/hspec/hspec-expectations#readme>
+category:       Testing
+homepage:       https://github.com/hspec/hspec-expectations#readme
+bug-reports:    https://github.com/hspec/hspec-expectations/issues
+author:         Simon Hengel <sol@typeful.net>
+maintainer:     Simon Hengel <sol@typeful.net>
+copyright:      (c) 2011-2015 Simon Hengel
+license:        MIT
+license-file:   LICENSE
+build-type:     Simple
+cabal-version:  >= 1.10

 source-repository head
   type: git
@@ -28,6 +28,7 @@ library
   build-depends:
       base == 4.*
     , HUnit
+    , pretty-show
   exposed-modules:
       Test.Hspec.Expectations
       Test.Hspec.Expectations.Contrib
@@ -46,6 +47,7 @@ test-suite spec
   build-depends:
       base == 4.*
     , HUnit
+    , pretty-show
     , nanospec
   other-modules:
       Test.Hspec.Expectations.MatcherSpec
diff --git a/package.yaml b/package.yaml
index e86779f..9483150 100644
--- a/package.yaml
+++ b/package.yaml
@@ -15,6 +15,7 @@ ghc-options: -Wall
 dependencies:
   - base == 4.*
   - HUnit
+  - pretty-show

 library:
   source-dirs: src
diff --git a/src/Test/Hspec/Expectations.hs b/src/Test/Hspec/Expectations.hs
index 2fd326c..18ebdd5 100644
--- a/src/Test/Hspec/Expectations.hs
+++ b/src/Test/Hspec/Expectations.hs
@@ -57,6 +57,7 @@ import           Data.List
 import           Control.Monad (unless)

 import           Test.Hspec.Expectations.Matcher
+import           Text.Show.Pretty (ppShow)

 #ifdef HAS_SOURCE_LOCATIONS

@@ -85,7 +86,7 @@ infix 1 `shouldNotBe`, `shouldNotSatisfy`, `shouldNotContain`, `shouldNotReturn`
 -- @actual \`shouldBe\` expected@ sets the expectation that @actual@ is equal
 -- to @expected@.
 with_loc(shouldBe, (Show a, Eq a) => a -> a -> Expectation)
-actual `shouldBe` expected = expectTrue ("expected: " ++ show expected ++ "\n but got: " ++ show actual) (actual == expected)
+actual `shouldBe` expected = expectTrue ("expected: " ++ ppShow expected ++ "\n but got: " ++ ppShow actual) (actual == expected)

 -- |
 -- @v \`shouldSatisfy\` p@ sets the expectation that @p v@ is @True@.

Anyway, I applied the same changes to the current master. It took about 30 seconds to build pretty-show (and its dependencies) on my machine. After that, it had no effect on the build time of hspec-expectations.

[bitemyapp]

Sidebar: pretty-show doesn't seem to handle [(RecordType, RecordType)] well, but it doesn't make it any harder to read either, so any failures on the part of pretty-show appear to be benign.

[sol]

With a recent change to HUnit it's now possible to implement this kind of behavior in Hspec formatters. And I think this is what we want. Specifically, we want command-line options --pretty and --no-pretty that enabled/disabled this feature. Initially the default should be --no-pretty so that we can experiment with it. In a later release we will then change the default to --pretty.

I implemented --diff in a similar way: hspec/hspec@fe5b8a6

@bitemyapp Would you be willing to port this code over to hspec-core?

[sol]

Closing in favor of hspec/hspec#294

[bitemyapp]

@sol fine by me, thank you.