Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Create separate package for property tests #190

Closed
aristidb opened this Issue · 9 comments

3 participants

@aristidb
Collaborator

Can be done by adding subdirectory with tests, and embedding them in the lens tests by means of hs-subdirectories or such.

For such a package to be valuable, the tests would also have to be more complete and give a certain amount of certainty that if something passes isTraversal, it really is a traversal.

@ekmett
Owner

The current plan is that for 3.8:

  • Make a sub-directory lens-properties with its own lens-properties.cabal.
  • Update those properties to use hspec or something nicer and have actual signatures.
  • Modify tests/properties.hs to reference lens-properties directly via hs-source-dirs.

Then the user can import Control.Lens.Properties from lens-properties if they want to get the QuickCheck or HSpec plumbing, and we don't have to incur the dependencies in the main package.

We'd then distribute lens-properties/* as part of the main lens.cabal distribution as well as a separate package that we update periodically.

@ekmett ekmett was assigned
@aristidb aristidb was assigned
@ekmett
Owner

Note: @dag converted tests/properties over to test-framework in #191.

@ocharles

I have started some work in this at https://github.com/ocharles/lens/tree/lens-properties. It seems to work; it still needs documentation for 'isIso' and 'isPrism', and someone needs to check this is actually the right solution :)

@ocharles

Also, cabal init gave me copyright in the license, but I'm unsure what to do with that. If Edward or someone else would rather have the copyright (after all, I just shuffled code around) I am fine with that.

@ekmett
Owner

@ocharles Would you feel up to updating this patch to work with 3.9 or 3.10?

@ocharles
@ocharles

I think a week has passed now. #377 opened, lets get this done! Please review and let me know what more needs to be done on the pull request.

@ekmett
Owner

Yay! Will do.

@ekmett
Owner

Merged.

@ekmett ekmett closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.