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

Make 'PropertyT' an instance of 'MonadResource' #268

Merged

Conversation

geigerzaehler
Copy link
Contributor

Make PropertyT an instance of MonadResource simplifies writing tests that use the functions with a MonadResource constraint. See the changed example for instance.

Thomas Scholtes and others added 3 commits April 11, 2019 18:15
Make `PropertyT` an instance of `MonadResource` simplifies writing tests
that use the functions with a `MonadResource` constraint. See the
changed example for instance.
@jacobstanley
Copy link
Member

jacobstanley commented Apr 15, 2019

This is awesome, I didn't have this from the beggining because, I think I thought ResourceT wouldn't work on the inside, i.e. hoist and I wasn't clever enough to get ResourceT to work on the outside of GenT.

See the recent PR #255

I updated the PR with a fix for 7.10.

@jacobstanley jacobstanley merged commit 1f40804 into hedgehogqa:master Apr 15, 2019
@jacobstanley
Copy link
Member

It's funny the only reason the TestT / PropertyT separation even exists is for ResourceT support, maybe I should merge PropertyT and TestT like it used to be...

@geigerzaehler
Copy link
Contributor Author

Thanks for merging! This is going to make my life a lot easier.

@geigerzaehler geigerzaehler deleted the monad-resource-for-property-t branch April 15, 2019 08:02
@MichaelXavier
Copy link

Given that recent versions of ResourceT require MonadUnliftIO, which forbids instances in stateful monads, is this even possible to use anymore? I've got a test where I'd like to interleave some assertions with a ResourceT so I don't get too far in the test if something is wrong and I'm not seeing a way how. I might have to fall back to removing ResourceT entirely and using bracket.

@HuwCampbell
Copy link
Member

HuwCampbell commented Jul 30, 2019

So I believe that the example is now quite dangerous.

prop_unix_sort :: Property
property . hoist runResourceT $ do
    (_, dir) <- Temp.createTempDirectory Nothing "prop_dir"
    doStuff

If I understand correctly, this will create a directory for every shrink and not clean them up until all shrinking is complete. As shrinking can run hundreds or thousands of times, we will have that many temporary directories on the system; and if it was instead file handles, it could crash the system.

EDIT: Apologies, I see you've addressed this in #285

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.

4 participants