search

fakedata-haskell / fakedata

Haskell Library for producing quality fake data
BSD 3-Clause "New" or "Revised" License
149 stars 22 forks source link

Fix test failures for nightly builds #34

Closed TristanCacqueray closed 3 years ago

TristanCacqueray commented 3 years ago

This change fixes this cabal test error: ghc: could not execute: hspec-discover

Fixes #33

TristanCacqueray commented 3 years ago

There are still 42 test failures with incorrect data, e.g.:

  test/OtherSpec.hs:70:7:
  40) Other, Faker Generate, App author
       expected: "Schmidt, Breitenberg and Lowe"
        but got: "Goldner, Will and Muller"

Should we fix the expected value? It seems like setDeterministic is not working...

psibi commented 3 years ago

@TristanCacqueray Thanks for the PR!

Should we fix the expected value? It seems like setDeterministic is not working...

The reason the failure is happening is because new version of random has changed the internals. So we should fix the failure so that the test case works for both the newer version of random library as well as older version of random library.

For the above failure, I would fix it like this:

x == "Schmidt, Breitenberg and Lowe" || x == "Goldner, Will and Muller"
TristanCacqueray commented 3 years ago

@psibi would it be possible to find a compatible seed instead and configure it with a CPP macro?

psibi commented 3 years ago

@TristanCacqueray If I understand, the underlying algorithm itself has changed between random versions. But if you can find a seed that works for both the case, then sure we can go ahead with that. :-)

TristanCacqueray commented 3 years ago

I looked for such seed with https://gist.github.com/TristanCacqueray/64795ba9444af8fe0314e44e59f14942 , but could not find one from 0 to 510_000. And even if such seed exists, it may not work for all the tests. So manually fixing the test seems like the best solution.

psibi commented 3 years ago

Seems it needs some adjustment in the CI scripts to make it pass (different stack.yaml for different resolvers). I'm going ahead and merging this - that can be fixed separately. Thanks @TristanCacqueray for the PR!