Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MonadAccum law tests #125

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

MonadAccum law tests #125

wants to merge 8 commits into from

Conversation

kozross
Copy link
Collaborator

@kozross kozross commented May 21, 2022

This relates to this comment here. Specifically, these tests use property-based testing to exercise all the stated laws of MonadAccum, with a large enough test count to be meaningful.

Copy link
Member

@andreasabel andreasabel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I help here?

Kudos for the tests!

I don't know MonadAccum, so I might not be the best person to review this.

@kozross
Copy link
Collaborator Author

kozross commented May 21, 2022

@andreasabel I mostly tagged you as a reviewer because you suggested/requested some tests. Was this what you had in mind?

I started with MonadAccum for two reasons: it's one of the (currently only two) type classes provided by mtl which even states any laws, and of the two, it is by far the easiest, both in terms of the laws themselves, and also to test.

@andreasabel
Copy link
Member

Well done!

I started with MonadAccum for two reasons: it's one of the (currently only two) type classes provided by mtl which even states any laws, and of the two, it is by far the easiest, both in terms of the laws themselves, and also to test.

Testing the laws is of course the gold standard!

Even simple unit tests that cover the functionality are useful, detecting basic problems like missing exports, unintended loops, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants