hedgehogqa / haskell-hedgehog

Release with confidence, state-of-the-art property testing for Haskell.
670 stars 109 forks source link

output quality depends on $PWD #517

Open samuel-gelineau-at-well-dot opened 3 months ago

samuel-gelineau-at-well-dot commented 3 months ago

in multi-package projects, it is common for each package to be placed in a separate sub-directory. the quality of the output depends on whether the tests are run from the root of the project or from the sub-directory of the package which contains the tests:

$ cd ~PROJECT_ROOT
$ cabal run toy-exe
━━━ Toy ━━━
  ✗ prop_failing failed at Main.hs:14:3
    after 1 test.
    shrink path: 1:

    forAll0 =
      0

    forAll1 =
      0

    This failure can be reproduced by running:
    > recheckAt (Seed 1302999818739436304 15345910533038617549) "1:" prop_failing

  ✗ 1 failed.

vs the much better looking:

$ cd $PROJECT_ROOT/my-package
$ cabal run toy-exe
━━━ Toy ━━━
  ✗ prop_failing failed at Main.hs:14:3
    after 1 test.
    shrink path: 1:

       ┏━━ Main.hs ━━━
     8 ┃ prop_failing :: Property
     9 ┃ prop_failing = property $ do
    10 ┃   x <- forAll $ Gen.int (Range.linear 0 10)
       ┃   │ 0
    11 ┃   y <- forAll $ Gen.int (Range.linear 0 10)
       ┃   │ 0
    12 ┃   assert (x == x)
    13 ┃   assert (y == y)
    14 ┃   assert (x < y)
       ┃   ^^^^^^^^^^^^^^
    15 ┃   assert (x <= x)
    16 ┃   assert (y <= y)

    This failure can be reproduced by running:
    > recheckAt (Seed 7577190907837601680 195968907661934709) "1:" prop_failing

  ✗ 1 failed.

For easy reproducibility, here is the file structure I have used to produce the output above. The only thing which matters is that my toy.cabal file is in $PROJECT_ROOT/my-package instead of $PROJECT_ROOT.

$ tree $PROJECT_ROOT
$PROJECT_ROOT
├──
├── my-package
│   ├── Main.hs
│   └── toy.cabal
└── cabal.project

$ cat $PROJECT_ROOT/cabal.project
packages: my-package

$ cat $PROJECT_ROOT/my-package/toy.cabal
cabal-version:      3.0
name:               toy
version:            0.1.0.0
build-type:         Simple

executable toy-exe
    main-is:          Main.hs
    build-depends:    base
                    , hedgehog == 1.4
    hs-source-dirs:   .
    default-language: Haskell2010

$ cat $PROJECT_ROOT/my-package/Main.hs
{-# LANGUAGE OverloadedStrings #-}
module Main where

import Hedgehog
import qualified Hedgehog.Gen as Gen
import qualified Hedgehog.Range as Range

prop_failing :: Property
prop_failing = property $ do
  x <- forAll $ Gen.int (Range.linear 0 10)
  y <- forAll $ Gen.int (Range.linear 0 10)
  assert (x == x)
  assert (y == y)
  assert (x < y)
  assert (x <= x)
  assert (y <= y)

main :: IO Bool
main
  = checkParallel
  $ Group "Toy"
    [ ("prop_failing", prop_failing)
    ]
moodmosaic commented 3 months ago

Thanks for the detailed report ❤️

I think that with Cabal, running tests from a sub-directory with its own .cabal file might affect how paths are resolved.

This could link error outputs directly to the source files in the sub-directory, potentially explaining the detailed error reporting when tests are run from $PROJECT_ROOT/my-package.

However, I couldn’t find specific documentation confirming this exact behavior, so it might very well be possible to fix this inside hedgehog itself.

samuel-gelineau-at-well-dot commented 3 months ago

it might very well be possible to fix this inside hedgehog itself.

I am guessing that hedgehog records the relative path of the source code at compile time, and thus whether this relative path resolves to a file or not depends on the $PWD. recording the absolute path instead of the relative path should fix the problem.

moodmosaic commented 2 months ago

Sounds reasonable and pull requests welcome 💯 👍

sol commented 2 months ago

recording the absolute path instead of the relative path should fix the problem.

I think this would require Template Haskell, I'm not aware of a way to achieve this with the HasCallStack-machinery.

Maybe just open a Cabal issue instead. When cabal test executes the test executable it could easily set the CWD to the package root. Better address it once at the root than requiring every test library to hack its way around it.

moodmosaic commented 2 months ago

Agreed with @sol, if it's possible to fix this upstream it'll be better so that the whole community may benefit for this.