Add property tests for the Validation type by astynax · Pull Request #216 · kowainik/relude

astynax · 2019-10-23T10:02:59Z

Resolves #176

I added property tests for basic laws of Semigroup/Monoid/Applicative/Alternative.

A copule of tests checks that <* and *> work just like default implementations in Applicative class.

Checklist:

HLint

I've changed the exposed interface (add new reexports, remove reexports, rename reexported things, etc.).
- I've updated hlint.dhall accordingly to my changes (add new rules for the new imports, remove old ones, when they are outdated, etc.).
- I've generated the new .hlint.yaml file (see this instructions).

General

I've updated the CHANGELOG with the short description of my latest changes.
All new and existing tests pass.
I keep the code style used in the files I've changed (see style-guide for more details).
I've used the stylish-haskell file.
My change requires the documentation updates.
- I've updated the documentation accordingly.
I've added the [ci skip] text to the docs-only related commit's name.

hint-man

Do you know why your PR is still not approved? Because I chose not to approve it. But they will.

test/Test/Relude/Extra/Validation/Property.hs

hint-man · 2019-10-23T10:03:27Z

test/Test/Relude/Extra/Validation/Property.hs

+prop_monoidRightIdentity :: Property
+prop_monoidRightIdentity = property $ do
+    x <- forAll $ asValidation genSmallText
+    x <> mempty === x


Warning: Monoid law, right identity

Suggested change

x <> mempty === x

x === x

hint-man · 2019-10-23T10:03:27Z

test/Test/Relude/Extra/Validation/Property.hs

+prop_monoidLeftIdentity :: Property
+prop_monoidLeftIdentity = property $ do
+    x <- forAll $ asValidation genSmallText
+    mempty <> x === x


Warning: Monoid law, left identity

Suggested change

mempty <> x === x

x === x

hint-man · 2019-10-23T10:03:27Z

test/Test/Relude/Extra/Validation/Property.hs

+prop_applicativeIdentity :: Property
+prop_applicativeIdentity = property $ do
+    vx <- forAll $ asValidation genSmallText
+    (pure id <*> vx) === vx


Suggestion: Use <$>

Suggested change

(pure id <*> vx) === vx

(id <$> vx) === vx

hint-man · 2019-10-23T10:03:27Z

test/Test/Relude/Extra/Validation/Property.hs

+    vf <- forAllWith (const "f") $ asValidation genFunction
+    vg <- forAllWith (const "g") $ asValidation genFunction
+    vx <- forAll $ asValidation genSmallInt
+    (pure (.) <*> vf <*> vg <*> vx) === (vf <*> (vg <*> vx))


Suggestion: Use <$>

Suggested change

(pure (.) <*> vf <*> vg <*> vx) === (vf <*> (vg <*> vx))

(((.) <$> vf) <*> vg <*> vx) === (vf <*> (vg <*> vx))

test/Test/Relude/Extra/Validation/Property.hs

hint-man · 2019-10-23T10:03:28Z

test/Test/Relude/Extra/Validation/Property.hs

+prop_applicativeInterchange = property $ do
+    vf <- forAllWith (const "f") $ asValidation genFunction
+    x <- forAll genSmallInt
+    (vf <*> pure x) === (pure ($ x) <*> vf)


Suggestion: Use <$>

Suggested change

(vf <*> pure x) === (pure ($ x) <*> vf)

(vf <*> pure x) === (($ x) <$> vf)

hint-man · 2019-10-23T10:03:28Z

test/Test/Relude/Extra/Validation/Property.hs

+prop_alternativeRightIdentity :: Property
+prop_alternativeRightIdentity = property $ do
+    x <- forAll $ asValidation genSmallText
+    (x <|> empty) === x


Warning: Alternative law, right identity

Suggested change

(x <|> empty) === x

(x) === x

hint-man · 2019-10-23T10:03:28Z

test/Test/Relude/Extra/Validation/Property.hs

+prop_alternativeLeftIdentity :: Property
+prop_alternativeLeftIdentity = property $ do
+    x <- forAll $ asValidation genSmallText
+    (empty <|> x) === x


Warning: Alternative law, left identity

Suggested change

(empty <|> x) === x

(x) === x

astynax · 2019-10-23T14:18:25Z

@chshersh will I need to suppress such warnings from the bot? In this particular case code contains lines like empty <|> by purpose :)

chshersh · 2019-10-23T15:33:23Z

@astynax Yeah, I know that this code is on purpose. Ideally, there should be a good library that already implements all required test-suites for laws for the standard typeclasses. Unfortunately, I'm not aware of such library for hedgehog so it's unavoidable that we will have all these warnings...

vrom911 · 2019-10-24T06:22:01Z

We can use HLint ignore pragmas to not yell on these particular cases 🙂

chshersh

@astynax Thanks for your work! Looks good and very useful 👍 I have a few comments, but they are mostly stylistic.

chshersh · 2019-10-24T06:40:18Z

relude.cabal


  other-modules:       Test.Relude.Property
-
+                     , Test.Relude.Extra.Validation.Property


Commas are redundant in module definitions

Suggested change

, Test.Relude.Extra.Validation.Property

Test.Relude.Extra.Validation.Property

chshersh · 2019-10-24T06:41:14Z

test/Test/Relude/Extra/Validation/Property.hs

+Copyright:  (c) 2016 Stephen Diehl
+            (c) 2016-2018 Serokell
+            (c) 2018-2019 Kowainik


I believe there's no need to specify everyone here, since it's a brand new module. So this can be just:

Copyright: (c) 2019 Kowainik

chshersh · 2019-10-24T06:41:59Z

test/Test/Relude/Extra/Validation/Property.hs

+validationTestList :: [Group]
+validationTestList =


Let's rename List suffix to Laws or Group to have better semantics

chshersh · 2019-10-24T06:43:01Z

test/Test/Relude/Extra/Validation/Property.hs

+    a `op` (b `op` c) === (a `op` b) `op` c
+
+----------------------------------------------------------------------------
+-- Semogroup instance properties


Minor typo 🙂

Suggested change

-- Semogroup instance properties

-- Semigroup instance properties

chshersh · 2019-10-24T06:47:21Z

test/Test/Relude/Extra/Validation/Property.hs

+prop_applicativeHomomorphism = property $ do
+    f <- forAllWith (const "f") genFunction
+    x <- forAll genSmallInt
+    (pure f <*> (pure x :: Validation [Text] Int)) === pure (f x)


I think this can be written a bit cleaner with type applications

Suggested change

(pure f <*> (pure x :: Validation [Text] Int)) === pure (f x)

(pure f <*> pure x) === pure @(Validation [Text]) (f x)

chshersh

Thanks, looks great!

vrom911

Thanks for the contribution!

I will silence the HLint warnings in a separate PR.

Add property tests for the Validation type

4f0f147

hint-man bot reviewed Oct 23, 2019

View reviewed changes

generalize assotiativity check

e149d0d

astynax mentioned this pull request Oct 23, 2019

Implement property-based tests for Validation #176

Closed

chshersh requested review from chshersh and vrom911 October 24, 2019 06:39

chshersh assigned astynax Oct 24, 2019

chshersh added the tests label Oct 24, 2019

chshersh reviewed Oct 24, 2019

View reviewed changes

fix notes

5b663ab

chshersh approved these changes Oct 24, 2019

View reviewed changes

vrom911 approved these changes Oct 25, 2019

View reviewed changes

vrom911 merged commit 9e7fbfa into kowainik:master Oct 25, 2019

astynax deleted the validaion-props branch October 25, 2019 06:29

chshersh added the Hacktoberfest https://hacktoberfest.digitalocean.com/ label Nov 5, 2019

	(pure (.) <> vf <> vg <> vx) === (vf <> (vg <*> vx))
	(((.) <$> vf) <> vg <> vx) === (vf <> (vg <> vx))

	(vf <> pure x) === (pure ($ x) <> vf)
	(vf <*> pure x) === (($ x) <$> vf)


		other-modules: Test.Relude.Property

		, Test.Relude.Extra.Validation.Property

	, Test.Relude.Extra.Validation.Property
	Test.Relude.Extra.Validation.Property

	-- Semogroup instance properties
	-- Semigroup instance properties

	(pure f <*> (pure x :: Validation [Text] Int)) === pure (f x)
	(pure f <*> pure x) === pure @(Validation [Text]) (f x)

		validationTestList :: [Group]
		validationTestList =

Uh oh!

Conversation

astynax commented Oct 23, 2019

Checklist:

HLint

General

Uh oh!

hint-man bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hint-man bot Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

hint-man bot Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

hint-man bot Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

hint-man bot Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hint-man bot Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

hint-man bot Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

hint-man bot Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

astynax commented Oct 23, 2019

Uh oh!

chshersh commented Oct 23, 2019

Uh oh!

vrom911 commented Oct 24, 2019

Uh oh!

chshersh left a comment

Choose a reason for hiding this comment

Uh oh!

chshersh Oct 24, 2019

Choose a reason for hiding this comment

Uh oh!

chshersh Oct 24, 2019

Choose a reason for hiding this comment

Uh oh!

chshersh Oct 24, 2019

Choose a reason for hiding this comment

Uh oh!

chshersh Oct 24, 2019

Choose a reason for hiding this comment

Uh oh!

chshersh Oct 24, 2019

Choose a reason for hiding this comment

Uh oh!

chshersh left a comment

Choose a reason for hiding this comment

Uh oh!

vrom911 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants