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

Testing of promises.ml seems incomplete #41

Open
ejgallego opened this issue Mar 22, 2024 · 5 comments
Open

Testing of promises.ml seems incomplete #41

ejgallego opened this issue Mar 22, 2024 · 5 comments

Comments

@ejgallego
Copy link

Dear effects examples maintainers,

thanks for the very nice examples on OCaml effects, they are very helpful!

@bhaktishh and I were looking into the promises.ml example, however we have noticed that the way the test are written, it seems that we never call the Wait effect, as all the promises are resolved immediately.

It seems to us that having a more complete test could be useful, what do you think?

We have a few ideas on how to solve this, let us know what do you think?

I guess the first to try would be to decouple promise creation from resolution, but we'd like to hear your opinion before trying to do a pull request.

@dhil
Copy link
Collaborator

dhil commented Mar 26, 2024

We will happily welcome pull requests to improve the state of affairs. It sounds reasonable to me to include a test that'd trigger the Wait effect. One thing to keep in mind is that we should keep the test cases succinct as the contents of this repository are intended to be educational rather than being of "production-grade quality".

@ejgallego
Copy link
Author

Indeed, it is very important the examples do remain simple and succinct!

We are not sure tho how to address this issue in the most direct way; with the current API, I see at least two possible options, hard to say which one is easiest.

How would you proceed to trigger Wait without having to change a lot of code?

@dhil
Copy link
Collaborator

dhil commented Mar 28, 2024

Looking at the code again, it may be enough to change the scheduling policy https://github.com/ocaml-multicore/effects-examples/blob/master/promises.ml#L129-L130 it seems fork eagerly schedules its argument, if we were to schedule to the continuation k to run before the argument, then I think the end result should be that the applicative operator <*> (https://github.com/ocaml-multicore/effects-examples/blob/master/promises.ml#L173) will trigger the Waiting-case in its implementation.

@dhil
Copy link
Collaborator

dhil commented Mar 28, 2024

Looking at the code again, it may be enough to change the scheduling policy

Coincidentally, this seems to be what I did in my toy implementation of async/await with multi-shot continuations, c.f. https://github.com/dhil/ocaml-multicont/blob/main/examples/async.ml#L103-L104

@ejgallego
Copy link
Author

Thanks a lot for the details @dhil ; we will try that approach.

[The other approach we were thinking is indeed to add a resolve method]

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