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

Report Test.Hedgehog.Seed tests individually #214

Closed
wants to merge 1 commit into from

Conversation

moodmosaic
Copy link
Member

@thumphries, those deterministic tests added in #207, they've been reported as 1 test which is incorrect:

prop_avoid_pathological_gamma_values :: Property
prop_avoid_pathological_gamma_values =
  withTests 1 . property $ do
    for_ asserts $ \a ->
      expected a === actual a
━━━ Test.Hedgehog.Seed ━━━
  ✓ prop_avoid_pathological_gamma_values passed 1 test.
  ✓ 1 succeeded.

It turns out that Group is all we need, since it's really a named collection of property tests:

-- | A named collection of property tests.
--
data Group =
  Group {
      groupName :: !GroupName
    , groupProperties :: ![(PropertyName, Property)]
    }

So I did this, which is actually cool for fully deterministic tests:

checkParallel $ Group "Test.Hedgehog.Seed" $
  -- | Verify that SplitMix avoids pathological γ-values, as discussed by
  --   Melissa E. O'Neill in the post with title Bugs in SplitMix(es), at
  --   http://www.pcg-random.org/posts/bugs-in-splitmix.html
  --
  --   See also:
  --   https://github.com/hedgehogqa/haskell-hedgehog/issues/191
  --
  do
  (expected, actual) <-
    [
      ( Seed 15210016002011668638 12297829382473034411
      , Seed.from 0x61c8864680b583eb
      )
    , ( Seed 11409286845259996466 12297829382473034411
      , Seed.from 0xf8364607e9c949bd
      )
    ...
    ]
  return
    ( "SplitMix avoids pathological γ-values,"
    , withTests 1 . property $ expected === actual
    )
━━━ Test.Hedgehog.Seed ━━━
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ SplitMix avoids pathological γ-values, passed 1 test.
  ✓ 16 succeeded.

/cc @jystic, @thumphries

@thumphries
Copy link
Member

When these fail, the counterexample is not particularly helpful:

  ✗ SplitMix avoids pathological γ-values, failed after 1 test.
  
       ┏━━ test/Test/Hedgehog/Seed.hs ━━━
     9 ┃ tests :: IO Bool
    10 ┃ tests =
    11 ┃   checkParallel $ Group "Test.Hedgehog.Seed" $
    12 ┃     -- | Verify that SplitMix avoids pathological γ-values, as discussed by
    13 ┃     --   Melissa E. O'Neill in the post with title Bugs in SplitMix(es), at
    14 ┃     --   http://www.pcg-random.org/posts/bugs-in-splitmix.html
    15 ┃     --
    16 ┃     --   See also:
    17 ┃     --   https://github.com/hedgehogqa/haskell-hedgehog/issues/191
    18 ┃     --
    19 ┃     do
    20 ┃     (expected, actual) <-
    21 ┃       [
    22 ┃         ( Seed 15210016002011668638 12297829382473034411
    23 ┃         , Seed.from 0x61c8864680b583eb
    24 ┃         )
    25 ┃       , ( Seed 11409286845259996466 12297829382473034411
    26 ┃         , Seed.from 0xf8364607e9c949bd
    27 ┃         )
    28 ┃       , ( Seed 1931727433621677744 12297829382473034411
    29 ┃         , Seed.from 0x88e48f4fcc823717
    30 ┃         )
    31 ┃       , ( Seed 307741759840609752 12297829382473034411
    32 ┃         , Seed.from 0x7f83ab8da2e71dd1
    33 ┃         )
    34 ┃       , ( Seed 8606169619657412120 12297829382473034413
    35 ┃         , Seed.from 0x7957d809e827ff4c
    36 ┃         )
    37 ┃       , ( Seed 13651108307767328632 12297829382473034413
    38 ┃         , Seed.from 0xf8d059aee4c53639
    39 ┃         )
    40 ┃       , ( Seed 125750466559701114 12297829382473034413
    41 ┃         , Seed.from 0x9cd9f015db4e58b7
    42 ┃         )
    43 ┃       , ( Seed 6781260234005250507 12297829382473034413
    44 ┃         , Seed.from 0xf4077b0dbebc73c0
    45 ┃         )
    46 ┃       , ( Seed 15306535823716590088 12297829382473034405
    47 ┃         , Seed.from 0x305cb877109d0686
    48 ┃         )
    49 ┃       , ( Seed 7344074043290227165 12297829382473034405
    50 ┃         , Seed.from 0x359e58eeafebd527
    51 ┃         )
    52 ┃       , ( Seed 9920554987610416076 12297829382473034405
    53 ┃         , Seed.from 0xbeb721c511b0da6d
    54 ┃         )
    55 ┃       , ( Seed 3341781972484278810 12297829382473034405
    56 ┃         , Seed.from 0x86466fd0fcc363a6
    57 ┃         )
    58 ┃       , ( Seed 12360157267739240775 12297829382473034421
    59 ┃         , Seed.from 0xefee3e7b93db3075
    60 ┃         )
    61 ┃       , ( Seed 600595566262245170 12297829382473034421
    62 ┃         , Seed.from 0x79629ee76aa83059
    63 ┃         )
    64 ┃       , ( Seed 1471112649570176389 12297829382473034421
    65 ┃         , Seed.from 0x05d507d05e785673
    66 ┃         )
    67 ┃       , ( Seed 8100917074368564322 12297829382473034421
    68 ┃         , Seed.from 0x76442b62dddf926c
    69 ┃         )
    70 ┃       ]
    71 ┃     return
    72 ┃       ( "SplitMix avoids pathological γ-values,"
    73 ┃       , withTests 1 . property $ expected === actual
       ┃       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
       ┃       │ Failed (- lhs =/= + rhs)
       ┃       │ - Seed 1931727433621677744 12297829382473034411
       ┃       │ + Seed 2860492057171661485 7012680697202162775
    74 ┃       )
    
    This failure can be reproduced by running:
    > recheck (Size 0) (Seed 3343433750760618743 4233547017185020597) SplitMix avoids pathological γ-values,

We can't re-check just one of the tests, despite what Hedgehog says. It expects the PropertyName to be a bound name. We can only re-run the whole set. We also can't recreate the test case from the footnotes, because it only shows the expected/actual values, not the inputs.

I suggest using zipWith to produce a slightly more informative property description. It would also be helpful to use footnote to log the inputs.

@thumphries
Copy link
Member

I could also just merge this if you like, since this is in the test tree and you did all the work to get this test passing in the first place. 😄 Up to you!

Before

    ━━━ Test.Hedgehog.Seed ━━━
      ✓ prop_avoid_pathological_gamma_values passed 1 test.
      ✓ 1 succeeded.

After

    ━━━ Test.Hedgehog.Seed ━━━
      ✓ prop_avoid_pathological_gamma_values → Seed 15210016002011668638 12297829382473034411, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 11409286845259996466 12297829382473034411, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 1931727433621677744 12297829382473034411, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 307741759840609752 12297829382473034411, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 8606169619657412120 12297829382473034413, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 13651108307767328632 12297829382473034413, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 125750466559701114 12297829382473034413, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 6781260234005250507 12297829382473034413, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 15306535823716590088 12297829382473034405, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 7344074043290227165 12297829382473034405, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 9920554987610416076 12297829382473034405, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 3341781972484278810 12297829382473034405, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 12360157267739240775 12297829382473034421, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 600595566262245170 12297829382473034421, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 1471112649570176389 12297829382473034421, passed 1 test.
      ✓ prop_avoid_pathological_gamma_values → Seed 8100917074368564322 12297829382473034421, passed 1 test.
      ✓ 16 succeeded.
@moodmosaic
Copy link
Member Author

I suggest using zipWith to produce a slightly more informative property description

Hmm, but we have a [(PropertyName, Property)] already since that do block takes place inside the list monad.

It would also be helpful to use footnote to log the inputs.

Absolutely. (Done, force pushed.)

I could also just merge this if you like

Sure, go ahead if the image below looks good with the footnotes (warning, large image below):

image

, withTests 1 . property $ do
footnote $ "expected: " ++ show expected
footnote $ "actual: " ++ show actual
expected === actual
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks a bit ugly to me having all these inlined (lines 74/75 up to 77). Happy to move those out separately if you also feel the same.

Copy link
Member Author

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave this a second thought and I think we should not merge it, so I'm closing it. I actually like the fact that we can use Hedgehog to run fully deterministic tests, but the output looks a bit weird.

  • We can only recheck the whole set
  • We can add footnotes but that won't allow us to recreate a particular test case from the set

@thumphries, thanks for your feedback 👍

@moodmosaic moodmosaic closed this Aug 10, 2018
@moodmosaic moodmosaic deleted the topic/seed-tests branch January 6, 2020 16:43
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

Successfully merging this pull request may close these issues.

2 participants