hedgehogqa / haskell-hedgehog-classes

Hedgehog will eat your typeclass bugs
BSD 3-Clause "New" or "Revised" License
56 stars 17 forks source link

ordLaws fails with a generically derived Ord instance for a product of naturals #17

Closed ocharles closed 5 years ago

ocharles commented 5 years ago

I find the following quite surprising, and I'm not really sure what to do with the test failure:

Prelude Hedgehog.Classes Numeric.Natural Hedgehog.Gen Hedgehog.Range> data NatPair = NatPair Natural Natural deriving (Eq, Ord, Show)
Prelude Hedgehog.Classes Numeric.Natural Hedgehog.Gen Hedgehog.Range> lawsCheck (ordLaws (NatPair <$> integral (linear 1 100) <*> integral (linear 1 100)))
  ✓ <interactive> passed 100 tests.
  ✗ <interactive> failed after 3 tests and 3 shrinks.

    ━━━ Context ━━━
    When testing the Transitivity law(†), for the Ord typeclass, the following test failed: 

    x <= y && y <= z ≡ x <= z, where
      x = NatPair 1 1
      y = NatPair 3 1
      z = NatPair 1 1

    The reduced test is: 
      False ≡ True

    The law in question: 
      (†) Transitivity Law: x <= y && y <= z ≡ x <= z
    ━━━━━━━━━━━━━━━

    This failure can be reproduced by running:
    > recheck (Size 2) (Seed 17929284001805393683 15151267920088307561) <property>

  ✓ <interactive> passed 100 tests.
  ✓ <interactive> passed 100 tests.
False
ocharles commented 5 years ago

In fact, I don't even need a pair, the test doesn't even work with naturals:

> lawsCheck (ordLaws (integral @_ @Natural (linear 1 100)))
  ✓ <interactive> passed 100 tests.
  ✗ <interactive> failed after 3 tests.

    ━━━ Context ━━━
    When testing the Transitivity law(†), for the Ord typeclass, the following test failed: 

    x <= y && y <= z ≡ x <= z, where
      x = 1
      y = 2
      z = 1

    The reduced test is: 
      False ≡ True

    The law in question: 
      (†) Transitivity Law: x <= y && y <= z ≡ x <= z
    ━━━━━━━━━━━━━━━

    This failure can be reproduced by running:
    > recheck (Size 2) (Seed 3253908977522463743 17379856752577018227) <property>

  ✓ <interactive> passed 100 tests.
  ✓ <interactive> passed 100 tests.
chessai commented 5 years ago

Something is very wrong here.

ocharles commented 5 years ago

I think the equality should be an implication. Transitivity is:

x <= y && y <= z implies x <= z

chessai commented 5 years ago

You're right. I just pushed a50eaa453a79a75ef8346e3ffba5bf4085e6545e, which after adding some tests seems to fix the issue. Would you mind trying that out? I need to make a release with a fix.

ocharles commented 5 years ago

Certainly. Just building now, will report back.

chessai commented 5 years ago

Thank you very much!

ocharles commented 5 years ago

Works perfectly for my case, thank you!

chessai commented 5 years ago

Awesome!