Lwt.dont_wait: a more local, more explicit Lwt.async #820
Conversation
test/core/test_lwt.ml
Outdated
| !saw = Some Exception) | ||
| end; | ||
| ] | ||
| let suites = suites @ [async_tests ; dont_wait_tests] |
There was a problem hiding this comment.
I recommend not accumulating two suites on one line, i.e. leave old line 1804 alone. The idea is to do the concatenation "locally" to each test suite, so that the concatenations don't get lost, and we don't accidentally stop running some tests when code is edited.
test/core/test_lwt.ml
Outdated
| later (fun () -> !f_ran = true) | ||
| end; | ||
|
|
||
| test ~sequential:true "f raises" begin fun () -> |
There was a problem hiding this comment.
This test doesn't need to be sequential. That's probably a leftover from using async tests as a template, where the test corresponding to this one mutates the global async_exception_hook ref. This doesn't happen in dont_wait tests.
There was a problem hiding this comment.
The same goes for the other tests in this suite with ~sequential:true.
|
Change this to "Resolves #819" or "Fixes #819" to automatically close #819 when this PR is merged. Just listing the issue number only links to the issue. Furthermore, "Fixes #819" or similar changes the message GitHub prints in #819 for the back-link, making it clearer that this PR did close that issue to readers of #819. |
|
LGTM modulo test suite nits. |
raphael-proust
force-pushed
the
dont-wait
branch
from
227c0f9
to
ea450d1
Compare
|
Rebased on master to check the CI. I'll squash-and-merge if it all goes well. |
Fixes #819
This is an alternative to
async/ignore_resultwith a more explicit exception handler.