add relation stuff ala === and /== #196

chessai · 2018-05-15T19:43:17Z

This PR adds 4 functions, that have similar behaviour to === and /==, except they correspond to different relations.

(<==) ~> (<=)
(>==) ~> (>=)

(>>>) ~> (>)
(<<<) ~> (<)

As for naming, I tried to keep them to three characters while making sure their names didn't conflict with anything popular.

The reason I wanted this is that I was doing some numerical computations and wanted to test that the result of a computation was less than or equal to a particular value.

andrewthad · 2018-05-15T19:50:06Z

The names >>> and <<< conflict with some stuff from Control.Arrow. I find those names confusing, but I like the idea here. Maybe non-infix names would be better here, but that's just my two cents as a very casual user of hedgehog. The project maintainers probably have better vision for this kind of thing.

chessai · 2018-05-16T00:30:46Z

I considered Control.Arrow; but I weigh that pretty low, since it isn't used much. Other people might weigh that higher.

As far as the names being confusing, I agree that they definitely may be. I considered (<<=)_{hedgehog_pr}, but that looks too close to (=<<)_base, and its counterpart (>>=)_{hedgehog_pr} is nominally (in the linguistic sense) equivalent to (>>=)_base. If anyone has any idea for a better name for those, I certainly wouldn't object. I don't know if it's necessary to continue the trend of three characters - it's possible that they could be english character names.

thumphries · 2018-05-16T02:09:19Z

They’re also used in Control.Category, so I will also suggest we find different names or operators. We have not really thought through how many base relations to include as special Hedgehog functions or operators. Now’s probably a good time to think about it. My gut feeling is that it’s worthwhile wherever we can provide a superior counterexample report by doing so.

…

On May 15, 2018, at 5:30 PM, Daniel Cartwright ***@***.***> wrote: I considered Control.Arrow; but I weigh that pretty low, since it isn't used much. Other people might weigh that higher. As far as the names being confusing, I agree that they definitely may be. I considered (<<=)hedgehog_pr, but that looks too close to (=<<)base, and its counterpart (>>=)hedgehog_pr is nominally (in the linguistic sense) equivalent to (>>=)base. If anyone has any idea for a better name for those, I certainly wouldn't object. I don't know if it's necessary to continue the trend of three characters - it's possible that they could be english character names. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

chessai · 2018-05-18T00:47:03Z

@thumphries Would you be in favour of a non-infix operator name?

chessai · 2018-09-28T14:45:43Z

ping

jacobstanley · 2019-03-10T03:50:45Z

Not a fan of this I'm sorry, I'd like to keep operators to a minimum. I think this is prettier anyway:

assert $ x > y

Of course it doesn't do the diff, but that's mainly useful for large data structures and I wouldn't have though comparing those for ordering is common enough to include dedicated functions. Maybe you have an example?

Also there's no way I'd merge operators which clash with both Control.Category and Control.Arrow (i.e. >>> and <<<)

jacobstanley · 2019-04-17T09:10:05Z

I apologize for the tone of the last message I think I was in a bit of a bad mood when I wrote it.

I've been considering this PR since I last replied, and I think I found something I'm happy with. I pushed an update to to your branch which renames the operators to use an @ prefix which I pronounce "assert" and then the standard haskell name for the operator. I kind of like this even though it's a bit ugly because once you learn the rule (prefix with @) then you know how to get all the rest. Because of this, I added @==, and @/= even though they are redundant just for completeness.

I would appreciate everyone else's input on this though.

@chessai great work on hedgehog-classes by the way, I think we're going to try and use it to remove our QuickCheck / checkers dependency from our test code. /cc @moodmosaic

jacobstanley · 2019-04-17T11:00:41Z

As discussed with @amosr @HuwCampbell, I've added function to assert relations directly.

Arguably the operators are no longer required, but they do have the advantage of perhaps slightly nicer failure output. Any opinions either way on whether we should keep the operators given the new relation function is appreciated.

Failure using operators

^ I'm not sure this extra info is a big win?

Failure using `relation`

moodmosaic · 2019-04-17T11:49:51Z

Keeping the function that @jystic added should be good enough, perhaps I'd consider giving it a different name (I think I've actually never seen relation as a function name in any testing tool out there).

Basically we have this already which is good enough

assert $ x > y

perhaps the proposed relation one could be renamed, e.g. → relate, but really not sure about adding more operators 😕

The issue I see with adding more operators is that it makes it hard(er) to draw a line; where do we stop adding more assertion-specific stuff?

I often see assertions packed on separate libraries (see Groovy Power Assert, Unquote, etc). It's just the Unix philosophy at the end; do one thing and do it well...

jacobstanley · 2019-04-17T12:08:25Z

think I've actually never seen relation as a function name in any testing tool out there

That is what it's called though https://en.wikipedia.org/wiki/Relational_operator

moodmosaic · 2019-04-17T12:14:11Z

That is what it's called though https://en.wikipedia.org/wiki/Relational_operator

That's great then 👍

charleso · 2019-04-17T20:00:24Z

hedgehog/src/Hedgehog/Internal/Property.hs

+
+infix 4 @==
+
+-- | Fails the test if the two arguments provided are equal.


"not equal"? (And I think the @/= comment is the wrong way around too)

yeah, you're right oops

i think i might remove the operators anyway after chatting to a few people about this PR

jacobstanley · 2019-04-18T11:42:23Z

hedgehog-example/src/Test/Example/Basic.hs

+    relation x (<=) 'r'
+
+    relate y (<) 87
+    relate x (<=) 'r'


Would appreciate some comments on which name people like best or some other one? The main point of this is to provide a diff, otherwise you'd just use assert and annotate.

Also is the infix style crazy? it's kind of non-haskellish but it does read pretty nice.

no strong opinions, but comparison is my favourite. I like the infix style, I think it makes the meaning pretty obvious.

I like diff and relate, I like how they sound, diff this with that, relate this with that.

One thing @moodmosaic and I discussed with comparison is that it's not a verb, like if we could use compare that would be awesome but it's already taken by Ord

chessai · 2019-04-18T12:48:45Z

I apologize for the tone of the last message I think I was in a bit of a bad mood when I wrote it.

I didn't pick up on any rudeness, so it's ok.

Thanks for picking up some work on the branch

hedgehog/src/Hedgehog/Internal/Property.hs

moodmosaic · 2019-04-19T08:30:28Z

hedgehog/src/Hedgehog/Internal/Property.hs

+
+relate :: (MonadTest m, Show a, Show b, HasCallStack) => a -> (a -> b -> Bool) -> b -> m ()
+relate = diff
+


See comment at https://github.com/hedgehogqa/haskell-hedgehog/pull/196/files#r276932847.

I think diff sounds more clear than all the others, short and sweet, diff this with that 🚀 🚀 🚀

jacobstanley · 2019-04-21T20:07:21Z

Looks like diff is the winner 📜

I am still willing to change it if someone has better ideas / good arguments as long as it's prior to the next release.

moodmosaic · 2019-04-21T20:28:27Z

👍

moodmosaic · 2019-04-23T10:05:37Z

hedgehog/src/Hedgehog/Internal/Tripping.hs

-import           Hedgehog.Internal.Source
+import           Hedgehog.Internal.Property (MonadTest, Diff(..), success, failWith)
+import           Hedgehog.Internal.Show (valueDiff, mkValue, showPretty)
+import           Hedgehog.Internal.Source (HasCallStack, withFrozenCallStack)


Good catch 👍

add relation stuff ala === and /==

05d0da2

jacobstanley force-pushed the add-more-rels branch from 6c5d9dc to d36bbea Compare April 17, 2019 09:07

jacobstanley force-pushed the add-more-rels branch from d36bbea to 9985ae0 Compare April 17, 2019 09:21

charleso reviewed Apr 17, 2019

View reviewed changes

jacobstanley reviewed Apr 18, 2019

View reviewed changes

moodmosaic reviewed Apr 19, 2019

View reviewed changes

hedgehog/src/Hedgehog/Internal/Property.hs Show resolved Hide resolved

moodmosaic reviewed Apr 19, 2019

View reviewed changes

Remove operators in favour of diff function.

9c2da54

jacobstanley force-pushed the add-more-rels branch from e0fea5b to 9c2da54 Compare April 21, 2019 20:05

jacobstanley merged commit 143995b into hedgehogqa:master Apr 21, 2019

jacobstanley mentioned this pull request Apr 21, 2019

Added ability to use comparison functions other than == #112

Closed

moodmosaic reviewed Apr 23, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add relation stuff ala === and /== #196

add relation stuff ala === and /== #196

chessai commented May 15, 2018

andrewthad commented May 15, 2018

chessai commented May 16, 2018

thumphries commented May 16, 2018 via email

chessai commented May 18, 2018

chessai commented Sep 28, 2018

jacobstanley commented Mar 10, 2019

jacobstanley commented Apr 17, 2019 •

edited

Loading

jacobstanley commented Apr 17, 2019 •

edited

Loading

moodmosaic commented Apr 17, 2019

jacobstanley commented Apr 17, 2019

moodmosaic commented Apr 17, 2019

charleso Apr 17, 2019

jacobstanley Apr 18, 2019

jacobstanley Apr 18, 2019

amosr Apr 19, 2019

moodmosaic Apr 19, 2019

jacobstanley Apr 19, 2019

chessai commented Apr 18, 2019

moodmosaic Apr 19, 2019

moodmosaic Apr 19, 2019

jacobstanley commented Apr 21, 2019

moodmosaic commented Apr 21, 2019

moodmosaic Apr 23, 2019


		infix 4 @==

		-- \| Fails the test if the two arguments provided are equal.


		relate :: (MonadTest m, Show a, Show b, HasCallStack) => a -> (a -> b -> Bool) -> b -> m ()
		relate = diff

add relation stuff ala === and /== #196

add relation stuff ala === and /== #196

Conversation

chessai commented May 15, 2018

andrewthad commented May 15, 2018

chessai commented May 16, 2018

thumphries commented May 16, 2018 via email

chessai commented May 18, 2018

chessai commented Sep 28, 2018

jacobstanley commented Mar 10, 2019

jacobstanley commented Apr 17, 2019 • edited Loading

jacobstanley commented Apr 17, 2019 • edited Loading

Failure using operators

Failure using relation

moodmosaic commented Apr 17, 2019

jacobstanley commented Apr 17, 2019

moodmosaic commented Apr 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chessai commented Apr 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobstanley commented Apr 21, 2019

moodmosaic commented Apr 21, 2019

Choose a reason for hiding this comment

jacobstanley commented Apr 17, 2019 •

edited

Loading

jacobstanley commented Apr 17, 2019 •

edited

Loading

Failure using `relation`