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

mtl-style class for YesodExample #1813

Open
chris-martin opened this issue Aug 3, 2023 · 6 comments
Open

mtl-style class for YesodExample #1813

chris-martin opened this issue Aug 3, 2023 · 6 comments

Comments

@chris-martin
Copy link

Yesod testing assumes that you're going to write your tests in the YesodExample context. If you're working in a different monad (e.g. YesodExample augmented with a ReaderT or StateT layer that helps manage test fixture data), you're constantly having to lift YesodExample actions. I'd like to propose adding the following polymorphic lifting function:

class (MonadIO m, Yesod site) => MonadYesodExample site m | m -> site where
  liftYesodExample :: YesodExample site a -> m a

Many of the utilities in the Yesod.Test module could then be generalized to a MonadYesodExample constraint.

I've already done this internally. If there is interest from maintainers, I'll start work on a PR to upstream it.

@parsonsmatt
Copy link
Contributor

One downside to polymorphic test helpers is that it can make inference worse - you may have to specify type annotations or aliases to get hspec to pick the right instance.

This seems like a useful library - yesod-test-lifted - any thoughts on packaging that, linking to it from the yesod-test readme, and see what adoption/real-world use looks like before making a change to upstream?

@chris-martin
Copy link
Author

chris-martin commented Aug 3, 2023

Fair concern. Our own test suite required very little extra type annotation, but I suppose this change would break the examples in the yesod-test readme. I have some other things I'd like to contribute to yesod-test as well, and I'd really prefer not to try to keep a separate package in sync. What about doing the work directly in yesod-test but in a separate Yesod.Test.Lifted module so that the change is non-breaking?

@parsonsmatt
Copy link
Contributor

That works for me!

I'm curious what else you're intending to do - I have a rewrite/new library planned with yesod-hspec #1763 and it may be nice to join forces there.

@chris-martin
Copy link
Author

Not anything too exciting, I think. We've just been collecting an assortment of convenient assertions that might be nice to share.

@chris-martin
Copy link
Author

chris-martin commented Aug 3, 2023

Rather than expanding yesod-test, I wonder if instead I could persuade you to make some adjustments to yesod-hspec, because you've already got a polymorphic API there that looks very close to what I'm after. (And now that I see you've already made your own variant of the Yesod.Test module, the thought of making yet another one of my own is making me uneasy.)

Your utilities in yesod-hspec use a MonadState (YesodExampleData site) constraint, which is not quite general enough for me due to the MonadState class's functional dependency. Would it be reasonable to introduce a new class (akin to my MonadYesodExample) instead?

@parsonsmatt
Copy link
Contributor

Yeah, I'd be happy to introduce that. The generalization from YesodExample to MonadState _ m, MonadIO m would be nicer as a MonadYesodExample, though I do think it'd be nice to use MonadIO where feasible (ie assertEq).

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

No branches or pull requests

2 participants