mgsloan / store

Fast binary serialization in Haskell
MIT License
109 stars 35 forks source link

Tests for Ratio fail. #23

Open kantp opened 8 years ago

kantp commented 8 years ago

The tests for Ratio Int8 and Ratio Int64 fail with an arithmetic overflow. This is rather strange, given that the implementations of poke, peek, and size do not seem to be performing any kind of arithmetic.

Failures:

  test/Data/StoreSpec.hs:237:
  1) Data.Store, Manually listed polymorphic store instances, Roundtrips (GHC.Real.Ratio GHC.Int.Int8)
       uncaught exception: ArithException (arithmetic overflow)

  test/Data/StoreSpec.hs:237:
  2) Data.Store, Manually listed polymorphic store instances, Roundtrips (GHC.Real.Ratio GHC.Int.Int64)
       uncaught exception: ArithException (arithmetic overflow)
mgsloan commented 8 years ago

Yes, this is why we have a submodule dep on smallcheck. For some reason roman has not accepted this PR https://github.com/feuerbach/smallcheck/pull/34

This is among the reasons I'm considering moving away from using smallcheck - https://github.com/fpco/store/issues/17

kantp commented 8 years ago

Oh, I see. That's inconvenient.

mgsloan commented 8 years ago

We actually should maybe consider changing the instance for Ratio to use Storable. We have two choices:

I think when this decision was made I was working on making all the unit tests pass. I think the other behavior is likely more sensible

bitonic commented 8 years ago

I've mitigated this in e9aa2be52fa9e8c37b74f06522507604bd135d85 . This became necessary because tests were failing while building docker images.

bitonic commented 8 years ago

@mgsloan I think that this means that the smallcheck submodule is not needed anymore.

mgsloan commented 8 years ago

Yup, feel free to remove it!