Allow for different test and property names

[mbg]

This PR changes testProperty to take an extra parameter which allows users to specify a name for the property that's different from the name of the test. Currently the output we get from hedgehog for a failing test wrongly instructs users to e.g.:

This failure can be reproduced by running:
> recheck (Size 3) (Seed 2994512763059724157 10152725542272428131) badReverse involutive fails

This would obviously never work. hedgehog seems to encourage/assume that the property name is set to the name of the function that implements it, so this allows users of tasty-hedgehog to specify that separately from the test name. With the changes, we can now get the more useful:

This failure can be reproduced by running:
> recheck (Size 1) (Seed 3784549250809935497 16296630620216742993) prop_badReverse_involutive

[andreabedini]

@gwils ?

[gwils]

@andreabedini Do you want to be a maintainer of tasty-hedgehog? I'd be happy for you to merge this PR and make a new release.

[andreabedini]

Thanks for the offer @gwils, I am not sure I would be the right person. Maybe @mbg would be interested? He's been using tasty-hedgehog way more than me, I think.

[mbg]

I would be happy to volunteer for this 🙂

[gwils]

Done.
Looks like there are conflicts? Please fix them, then bump the major version number according to PVP (probably the second digit). After that, feel free to merge the PR and I can do the Hackage release. What is your Hackage username?

[gwils]

Regarding this PR, what do you think of having two versions of these functions? One where both names are the same (the current behaviour) and one where they can vary (as in this PR)?
That way, we could make this change in a backwards-compatible way, by having new names for the new functions.

[mbg]

What is your Hackage username?

Same as on here: mbg.

Regarding this PR, what do you think of having two versions of these functions? One where both names are the same (the current behaviour) and one where they can vary (as in this PR)?
That way, we could make this change in a backwards-compatible way, by having new names for the new functions.

I had some time to think about this recently and thought of another possible approach: we could combine the TestName and PropertyName into one type along with an IsString instance, e.g.:

data HedgehogName = HN TestName PropertyName

instance IsString HedgehogName where
  fromString name = HN name (fromString name)

This way existing code and behaviour would not need to change, except for having to enable OverloadedStrings. Users who do want to specify the property name explicitly can do so by using the HN constructor explicitly. Downside would be that nobody is forced to do it while we still require the OverloadedStrings extension.

Do you have any thoughts on that? I can implement either this or the two different functions (or both) and rebase everything to be up-to-date.

[gwils]

I would prefer the two different functions approach. This usage of IsString is definitely clever, but I don't like it so much, because the two different kinds of usage would like at first glance like they shouldn't both typecheck. It's just a bit "weird" for my taste.

[mbg]

I have moved the changes into a new testPropertyNamed function and rebased the branch on master. I have also duplicated the tests for testPropertyNamed. That's all in 2d18c8d.

In ac441fb, I have added some changes to discourage the use of testProperty. Happy to remove that commit if you would prefer to keep testProperty as the default.

[mbg]

(If you are happy with ac441fb, I should probably update the README as well.)

[gwils]

Ok, thanks, I didn't fully understand everything that was going on here until I read your haddock string and deprecation message. I'm the on the same page now.
Yep, this looks great. Thank you for your perseverance and sorry it took over a year to make something happen.

[gwils]

I'll make a hackage release of this within about 48 hours and add you as a hackage maintainer while I'm there.

[andreabedini]

<3

[gwils]

@mbg I've compiled and run this now. Can you please open a new PR adding a no-deprecation-warning GHC pragma to our test file, so that our test suite doesn't generate warnings?

[mbg]

Ok, thanks, I didn't fully understand everything that was going on here until I read your haddock string and deprecation message. I'm the on the same page now.

That's great to hear! Sorry that the original explanation in the PR description didn't make this clear!

Can you please open a new PR adding a no-deprecation-warning GHC pragma to our test file, so that our test suite doesn't generate warnings?

Done in #52.

[gwils]

Released to hackage as tasty-hedgehog 1.2.0.0 and I've made you a maintainer on hackage @mbg

[mbg]

Awesome, thanks!

[JTeeuwissen]

Should testPropertyNamed be used from now on?
This requires an import from Hedgehog.Internal.Property to use the PropertyName constructor.

[mbg]

Yes, the intention is for testPropertyNamed to be used instead of testProperty. The Hedgehog documentation notes that PropertyName values "Should be constructed using OverloadedStrings". I assume that their intention is for the exact representation to be an implementation detail. So you should only import Hedgehog.Internal.Property if you want to avoid OverloadedStrings at all cost.