search

vimus / libmpd-haskell

A client library for MPD, the Music Player Daemon
MIT License
36 stars 26 forks source link

Ensure that unicode characters are valid #111

Closed andrzejressel closed 5 years ago

andrzejressel commented 5 years ago

Fixes #104

andrzejressel commented 5 years ago

I've tested it locally using: stack test --test-arguments=--seed=1971057639 --resolver nightly-2018-10-04 --fast, but it passes.

joachifm commented 5 years ago

LGTM. Only problem is it might end up producing empty values, as evinced by the travis failure.

Is it feasible to use something like https://hackage.haskell.org/package/QuickCheck-2.12.6.1/docs/Test-QuickCheck-Modifiers.html#t:UnicodeString? Or otherwise make arbitrary construct valid strings rather than filter invalid elements?

joachifm commented 5 years ago

I wonder if

UTF8.fromString <$> listOf1 arbitraryUnicodeChar -- or arbitraryPrintableChar

would work

andrzejressel commented 5 years ago

arbitraryUnicode will fail, because it includes these invalid chars. The printable should work fine thought.

joachifm commented 5 years ago

Vexingly, there are still edge cases: https://travis-ci.org/vimus/libmpd-haskell/jobs/453998056

My only other suggestion is

listOf1 (arbitrary `suchThat` isValid)

where isValid is from the previous version you proposed. Surely that must work, given that the predicate is suitably constraining?

andrzejressel commented 5 years ago

The worst thing is that it works fine locally even with that seed. I'll add better logging

andrzejressel commented 5 years ago

There is something wrong with generating: https://travis-ci.org/vimus/libmpd-haskell/jobs/454096799

Regarding other errors I think we should remove these old stack resolvers and add -11 and -12 - whatever works with new QuickCheck.

joachifm commented 5 years ago

I agree that the set of stack resolvers can be constrained. Even limiting to default and nightly (the latter as permitted failure) might suffice. (I use nixpkgs for everything so I've really no idea what range of resolvers make sense, I'm happy to defer to your judgement).