commercialhaskell / stackage

Stable Haskell package sets: vetted consistent packages from Hackage
https://www.stackage.org/
MIT License
531 stars 804 forks source link

hashable-1.3.1.0 breaks the ordering in testsuites #5878

Closed juhp closed 3 years ago

juhp commented 3 years ago

hashable-1.3.1.0 break the testsuites of various packages that use aeson (in Stackage Nightly).

For now to try to manage I will temporarily put an upper bound on hashable to workaround this. Please see haskell/aeson#837 for more details and related discussion.

phadej commented 3 years ago

@juhp do you have a list of packages (or have you opened issues in their bug trackers). I guess that without pinging maintainers this won't get resolved by itself any time soon.

juhp commented 3 years ago

Unfortunately not yet: the Stackage buildlog is quite long: >180k lines... I suppose as a first step I could upload the log somewhere to make it visible at least.

juhp commented 3 years ago

I uploaded the affected Stackage Nightly log here: https://petersen.fedorapeople.org/nightly-build.log-2021-02-20.gz (~1.1MB) if one wants to inspect it. I'd like to do some text processing on it but not sure if I can find time to do that soon...

bergmark commented 3 years ago

Here's the full list of test failures, sorry for the delay on this! There may be false positives in here (if so, I apologize for the noise!), all i checked is that a test suite failed. If your package (transitively) uses hashable and the test suite assumes an ordering based on this then please either update your test suite to be ordering-agnostic or bump the hashable lower bound for the test suite. This hashable change may cause actual bugs in your library if it also depends on a specific ordering. At some point we will need bite the bullet and upgrade hashable in stackage and disable any test suites that have not been updated.

This data is from 2021-02-20, so if you have already updated you don't need to do anything else, we'll notice when we upgrade hashable!

See https://petersen.fedorapeople.org/nightly-build.log-2021-02-20.gz if you want to review the actual test failure for your package.

To reproduce, make sure you are building against hashable >= 1.3.1.0

maintainer package test suite
@AndrewRademacher aeson-casing casing failed
@turboMaCk aeson-combinators doctest & spec failed
@newhoggy aeson-lens doctest failed
@brandon-leapyear aeson-schemas aeson-schemas-test failed
@brendanhay amazonka-sqs amazonka-sqs-test failed
@jonathanknowles bech32-th bech32-th-test failed
@centromere cacophony hlint failed
@tomphp cfenv cfenv-test failed
@aiya000 character-cases doctest failed
@chessai chronos doctest failed
@SamProtas composable-associations-aeson doctest & tests failed
@ludat conferer-aeson specs failed
@rblaze credential-store credential-store-test failed
Grandfathered crypto-pubkey test-crypto-pubkey failed
@dfithian datadog datadog-api-test failed
@rblaze dbus dbus_tests failed
@Gabriel439 dhall-lsp-server doctest failed
@k-bx dns network-tests failed
@karun012 doctest-discover doctests failed
@psibi download test failed
@ryota-ka duration doctest failed
@parsonsmatt esqueleto mysql & postgresql failed
@psibi fb runtests failed
@neongreen fmt tests failed
@Gabriel439 foldl doctest failed
@tdammers ginger tests failed
Tom Murphy gingersnap gingersnap-tests failed
Diogo Biazus hasql-notifications hasql-notifications-test failed
@jfischoff hasql-queue unit-tests failed
@nikita-volkov hasql-transaction conflicts-test failed
@vaclavsvejcar headroom doctest failed
@k-bx hedis hedis-test-cluster failed
@lemmih hgeometry doctests failed
@ndmitchell hie-bios bios-tests failed
@alanz hjsmin test-cli failed
@hpdeifel hledger-iadd spec failed
@philderbeast hpack-dhall golden failed
@myfreeweb hspec-expectations-pretty-diff tests failed
@mchaver hspec-golden-aeson test failed
@snoyberg http-client spec failed
@juhp http-directory test failed
@DougBurke hvega test failed
@vaibhavsagar ihaskell hspec failed
@maoe influxdb doctests failed
@tekul jose-jwt tests failed
@ozataman katip test failed
@hamsterdam kawhi tests failed
@bubba lsp-test tests failed
@chrisdone lucid test failed
@lehins massiv doctests failed
@donatello minio-hs minio-hs-test failed
@mrkkrp mmark tests failed
@mrkkrp mmark-ext tests failed
@VictorDenisov mongoDB test failed
@nalchevanidze morpheus-graphql morpheus-test failed
@nalchevanidze morpheus-graphql-core morpheus-test failed
@paul-rouse mysql test failed
@paul-rouse mysql-simple test failed
Grandfathered network spec failed
@chrisdone odbc test failed
@tomjaguarpaw opaleye test failed
@maksbotan openapi3 doctests failed
@LaurentRDC pandoc-plot tests failed
@jfischoff pg-transact pg-transact-test failed
@jfischoff port-utils unit-test failed
@jfischoff postgresql-libpq-notify test failed
@dylex postgresql-typed hdbc failed
@robx postgrest spec & spec-querycost failed
@parsonsmatt prairie prairie-test failed
@ndmitchell rattle rattle-test failed
@lemmih reanimate spec failed
@lemmih reanimate-svg w3c-spec failed
@wereHamster rethinkdb-client-driver spec failed
@polarina sdl2 sdl-space failed
@maksbotan servant-openapi3 doctests failed
@koterpillar serverless-haskell tests failed
@ndmitchell shake shake-test failed
Grandfathered skein runtests failed
@igrep skews mock-ws-server-test failed
@rickeyski slack-api tests failed
Grandfathered snap-core testsuite failed
@echatav squeal-postgresql doctest & properties & spec failed
@fizruk swagger2 doctests failed
@bartavelle tar-conduit tests failed
@jfischoff tmp-postgres test failed
@chshersh @vrom911 tomland tomland-test failed
@mbg wai-rate-limit-redis wai-rate-limit-redis-tests failed
bergmark commented 3 years ago

Throttled pings (please see comment above): @LaurentRDC @bartavelle @chshersh @vrom911 @dylex @echatav @fizruk @igrep @koterpillar @mbg @polarina @rickeyski @robx @wereHamster

bartavelle commented 3 years ago

You might want to also ping @lehins for tar-conduit!

phadej commented 3 years ago

lucid is already fixed.

mbg commented 3 years ago

I am a little confused about wai-rate-limit-redis being included in this list - it is listed under expected-test-failures in any case since it requires a Redis server to run, so that might be a false negative? Sorry if I have missed something else.

I have a daily CI run for this too: https://github.com/mbg/wai-rate-limit/actions/workflows/stackage-nightly.yml which hasn't failed.

tomjaguarpaw commented 3 years ago

Similar to opaleye. Its tests require a Postgres server. You can see it running correctly with hashable == 1.3.1.0 at https://github.com/tomjaguarpaw/haskell-opaleye/runs/2102755933?check_suite_focus=true

(There is one transient failure. These are almost unavoidable in Opaleye tests.)

LaurentRDC commented 3 years ago

For pandoc-plot the tests pass with hashable 1.3.1.0 on my machine. Similar to wai-rate-limit-redis, it is listed under expected test failures, which might be due to plotting toolkits not being installed on the host.

robx commented 3 years ago

In the logs, I see a number of lines like

mysql: unexpected test build success mysql-simple: unexpected test build success

I'm guessing those might be the packages that are marked to be skipped? If so, they should probably be removed here.

(The build log does some weird overwriting, the following seems to work to extract the list of packages with that message: gzcat nightly-build.log-2021-02-20.gz | grep unexpected\ test\ build\ success | col -b | awk -F: '{print $1}')

bergmark commented 3 years ago

Hmm, perhaps this list includes a bunch of packages with expected-test-failures. I thought I filtered these out but I may have done that incorrectly! No need to do anything if you don't think you need to :-)

mrkkrp commented 3 years ago

The test suites of mmark and mmark-ext pass with hashable-1.3.1.0. mmark depends on hashable directly, so I bumped the lower bound accordingly. mmark-ext doesn't, so I didn't do anything. I published mmark-0.0.7.3 and mmark-ext-0.2.1.3 on Hackage.

hpdeifel commented 3 years ago

The test-suite of the newest hledger-iadd release now passes with hashable-1.3.1.0

vaclavsvejcar commented 3 years ago

I fixed the failing doctest inheadroom so it's now passing with hashable-1.3.1.0, thanks for letting me know!

turboMaCk commented 3 years ago

I've just released aeson-combinators 0.0.5.0 - tests should work with both versions there. I did not update doctest but that one is behind cabal flag which is off by default so I think it should not cause problems. Anyway feel free to ping me if it does.

mihaimaruseac commented 3 years ago

new one:

hashable-1.3.0.0 (changelog) (Grandfathered dependencies, Stackage upper bounds) is out of bounds for:

igrep commented 3 years ago

My package skews should be added to expected-failures since https://github.com/iij-ii/direct-hs/issues/101.

wereHamster commented 3 years ago

I had a look at the log and I don't see any failure in the rethinkdb-client-driver package, only warnings during compilation. Running the tests requires a RethinkDB database and I doubt it stackage has such a setup (please correct me if I'm wrong). Indeed, the rethinkdb-client-driver package is in expected-test-failures.

mrkkrp commented 3 years ago

@mihaimaruseac I addressed the problem exactly in the way that had been suggested: by bumping the lower bound of hashable. Is there something I need to do in addition to that? I think both mmark and mmark-ext can be safely enabled once hashable-1.3.1.0 is on Stackage.

chshersh commented 3 years ago

tomland-1.3.3.0 was released to Hackage with the fixes to test cases and the increased the lower bound for hashable to 1.3.1.0

dfithian commented 3 years ago

I ran the datadog tests locally against the explicit hashable dependency. They succeeded. build-constraints.yaml has datadog marked as an expected test failure, so I don't think I have to do anything else. Please let me know if I am misunderstanding the problem.

phadej commented 3 years ago

@chshersh and others, did you made sure that your tests are resilient against further changes in hashable hash functions. You shouldn't rely on it being stable hash implementation. (Nor you cannot depend that unordered-containers doesn't change its use of hash functions either - i.e. print someHashMap is just bad).

I'm (not seriously) considering having different parameters on different platforms, so this would be more in your face. (It kind of is, if you test on 32bit systems too).

vaclavsvejcar commented 3 years ago

@chshersh and others, did you made sure that your tests are resilient against further changes in hashable hash functions. You shouldn't rely on it being stable hash implementation. (Nor you cannot depend that unordered-containers doesn't change its use of hash functions either - i.e. print someHashMap is just bad).

I'm (not seriously) considering having different parameters on different platforms, so this would be more in your face. (It kind of is, if you test on 32bit systems too).

I just released headroom-0.4.1.0 to Hackage aiming this issue. In case of Headroom the issue was in doctest, where you really can't do anything else than comparing strings, so I took different approach that should be more futureproof and I'll keep this in mind for future changes.

chshersh commented 3 years ago

@phadej Yes, we made sure. Specifically, in tomland we use sorting in tests.

robx commented 3 years ago

For what it's worth, postgrest tests did break, and are fixed in https://github.com/PostgREST/postgrest/pull/1770, so thanks for the heads up!

This shouldn't affect stackage though, since the tests are skipped due to the PostgreSQL dependency.

phadej commented 3 years ago

@chshersh great! (I was confused why you then bumped the lowerbound?)

mihaimaruseac commented 3 years ago

Unfortunately cannot yet remove the upper bound and test if things got fixed since

hashable-1.3.1.0 (changelog) (Grandfathered dependencies) is out of bounds for:

turboMaCk commented 3 years ago

@chshersh and others, did you made sure that your tests are resilient against further changes in hashable hash functions. You shouldn't rely on it being stable hash implementation.

yes. I want to support ghcjs in (all versions of) my lib. Luckily if your code is stupid all you need is stone and sticks https://github.com/turboMaCk/aeson-combinators/commit/a0bf7294f9ad4ee67074c93e3bf865826efc2675

k-bx commented 3 years ago

Both dns and hedis work fine, probably some error due to test setup.

ludat commented 3 years ago

I just released conferer-aeson 1.1.0.1 that fixes this issue.

and the library wasn't really broken before, I just had to tighten some semantics that weren't very well defined previously.

Also, thanks for letting everyone know, this would have definitely taken me quite more time without this issue :clap:

tekul commented 3 years ago

I released jose-jwt 0.9.1 which should fix this. Thanks for the warning!

SamProtas commented 3 years ago

composable-associations-aeson-0.1.0.1 released on hackage with docs/tests/doctests that no longer rely on HashMap ordering.

aiya000 commented 3 years ago

@bergmark

Hi :D Thank you for notifying to me!

I tried reproduce doctest failed :point_down:

https://github.com/commercialhaskell/stackage/issues/5878#issuecomment-798478764

But I cannot reproduce at here - hashable-1.3.1.0...

In above branch, I added hashable-1.3.1.0 into extra-deps of stack.yaml. I also tried below, but...

$ stack build         # OK
$ stack test :doctest # OK

Please tell me my wrong x(

Thanks!

bergmark commented 3 years ago

@aiya000 I don't think this was related to hashable for character-cases, but the tests fail for me locally:

$ stack unpack character-cases
Unpacked character-cases (from Hackage) to /Users/adam.bergmark/repos/3rd/character-cases-0.1.0.6/
$ cd character-cases-0.1.0.6/
$ stack test
$ stack init --resolver nightly
Looking for .cabal or package.yaml files to use to init the project.
Using cabal packages:
- ./

Selected resolver: nightly-2021-03-17
Selected resolver: nightly-2021-03-17
Initialising configuration using resolver: nightly-2021-03-17
Total number of user packages considered: 1
Writing configuration to file: stack.yaml
All done.
adam.bergmark ~/r/3/character-cases-0.1.0.6 /master $ stack test
character-cases> configure (lib + test)
Configuring character-cases-0.1.0.6...
character-cases> build (lib + test)
Preprocessing library for character-cases-0.1.0.6..
Building library for character-cases-0.1.0.6..
[1 of 4] Compiling Cases.Megaparsec
[2 of 4] Compiling Data.Char.Cases
[3 of 4] Compiling Data.String.Cases
[4 of 4] Compiling Paths_character_cases
Preprocessing test suite 'doctest' for character-cases-0.1.0.6..
Building test suite 'doctest' for character-cases-0.1.0.6..
[1 of 5] Compiling Cases.Megaparsec
[2 of 5] Compiling Data.Char.Cases
[3 of 5] Compiling Data.String.Cases
[4 of 5] Compiling Main
[5 of 5] Compiling Paths_character_cases
Linking .stack-work/dist/x86_64-osx/Cabal-3.2.1.0/build/doctest/doctest ...
character-cases> copy/register
Installing library in /Users/adam.bergmark/repos/3rd/character-cases-0.1.0.6/.stack-work/install/x86_64-osx/0adf80aebcae3f7ebfe156181adbc51440c2e273742b573987de95632af96cf0/8.10.4/lib/x86_64-osx-ghc-8.10.4/character-cases-0.1.0.6-JjLrB1XY8mY5NJzfl14Sdj
Registering library for character-cases-0.1.0.6..
character-cases> test (suite: doctest)

src/Data/String/Cases.hs:120: failure in expression `[camelQ|camel|]'
expected: "camel"
 but got: Camel (AlphaLower C_) [AlphaNumAlpha (AlphaLower A_),AlphaNumAlpha (AlphaLower M_),AlphaNumAlpha (AlphaLower E_),AlphaNumAlpha (AlphaLower L_)]
          ^

Examples: 37  Tried: 36  Errors: 0  Failures: 1

character-cases> Test suite doctest failed
Completed 2 action(s).
Test suite failure for package character-cases-0.1.0.6
    doctest:  exited with: ExitFailure 1
Logs printed to console
mrkkrp commented 3 years ago

When can we expect the new version of hashable to be included in nightlies?

vaibhavsagar commented 3 years ago

I just ran the IHaskell test suite with hashable-1.3.1.0 and it passed successfully, I think this was a false positive since the test setup for IHaskell is a little involved (i.e. it requires the ihaskell executable to be in the $PATH). You can reproduce this with Nix by running nix-build --check release-9.0.nix -A passthru.haskellPackages.ihaskell from this branch and you can use the ihaskell Cachix cache to download the dependencies if you like.

DougBurke commented 3 years ago

I have finally got around to fixing the tests in hvega - I've just pushed version 0.11.0.1 and am about 80.927% sure it works...

mrkkrp commented 3 years ago

When can we expect the new version of hashable to be included in nightlies?

bergmark commented 3 years ago

When can we expect the new version of hashable to be included in nightlies?

When we switch nightlies to GHC 9, hopefully next week

aiya000 commented 3 years ago

Hi! Removing my library from stackage is OK :D

Thank you!

bergmark commented 3 years ago

Closing this due to the GHC 9 upgrade. Packages mentioned here may have been disabled, please see https://github.com/commercialhaskell/stackage/issues/6072 for more information.