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

limiting liftIO for better testing #172

Open
joeyh opened this issue Apr 21, 2018 · 6 comments
Open

limiting liftIO for better testing #172

joeyh opened this issue Apr 21, 2018 · 6 comments

Comments

@joeyh
Copy link

joeyh commented Apr 21, 2018

Code in MomentIO can liftIO so there's no good way to mock out the IO parts for testing it.

In my reactive-banana-automation library, I found it useful to wrap MomentIO, producing a MomentAutomation monad that is not an instance of MonadIO. Thus the IO is limited to a few wrapper functions around fromAddHandler, reactimate, stepper, and changes. That allows me to run code like this, and know that it has no external dependencies and should give the same result each time (barring something unexpected in those IO actions).

> runner <- observeAutomation fridge mkSensors
> runner $ \sensors -> fridgeTemperature sensors =: 6
[FridgeRelay PowerOn]
> runner $ \sensors -> fridgeTemperature sensors =: 3
[]
> runner $ \sensors -> fridgeTemperature sensors =: 0.5
[FridgeRelay PowerOff]

I'm not sure how well-founded my hope is that this lets me reproducibly test a reactive-banana event network?

Anyway, I think it would be great if reactive-banana had a limited monad like my MomentAutomation to allow fully mocked testing of event networks like this.

@ocharles
Copy link
Collaborator

I think that the fact that things like reactimate choose MomentIO is the problem. We would much rather have

reactimate :: MonadFramework m => Event (IO ()) -> m ()

or something. MomentIO is then the canonical way to eliminate MonadFramework (it becomes an effect handler).

My suggestion is to change all MomentIO returning functions into functions polymorphic in their monad, subject to a MonadFramework constraint (much like how we have MonadMoment). I think this would address this concern.

@mitchellwrosen
Copy link
Collaborator

@ocharles Would you accept a patch that made this change? If so, I'm happy to help.

@ocharles
Copy link
Collaborator

I would, but I would like @HeinrichApfelmus to review before applying. Maybe it's worth having him confirm my API is sensible. @joeyh, would my proposed API work for you?

@joeyh
Copy link
Author

joeyh commented May 22, 2018 via email

@HeinrichApfelmus
Copy link
Owner

I think the trouble with replacing the concrete data type … MomentIO by a constraint MonadFramework m => … m is that — it may not actually be possible 😅. There are some functions like liftIOLater that I think the monad needs to know about. And then there is execute where the monad occurs in a contravariant position, and things become suddenly very difficult, because now the monad does need to know about IO.

If I understand the issue correctly, this is mainly about testing. When designing the library, I thought that testing should be done on pure combinators only, using Reactive.Banana.Combinators.interpret. I realize that in practice, this may not be the most ideal way to test. Could you describe your testing setup in more detail?

@joeyh
Copy link
Author

joeyh commented May 27, 2018 via email

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

No branches or pull requests

4 participants