reason-native icon indicating copy to clipboard operation
reason-native copied to clipboard

TestFramework: Async support?

Open bryphe opened this issue 5 years ago • 6 comments

Has there been any thinking on how the test framework will handle async tests, like tests leveraging lwt?

In JavaScript, Mocha and Jest make this easy - if your test returns a promise, it's handled for you. We unfortunately can't replicate that sort of API in reason (as far as I know, at least).

But we could potentially have a testAsync method that allows for hooking up to asynchonous tests via a callback. Or, even a testLwt that takes an Lwt promise for a more opinionated solution.

Just wanted to start the discussion and see if there has already been thinking / ideas here.

bryphe avatar Dec 01 '18 20:12 bryphe

Thanks for starting the discussion. This would be great, I do not think we have talked about this yet.

Within the framework we do want to allow custom matchers to be defined, previously that required implementing something like https://github.com/facebookexperimental/reason-native/blob/master/src/test-runner/library/FnMatchers.re for whatever structure you would like, and then overriding describe at the top of the test file:

open TestFramework;

let describe = extendDescribe(describe, MyCustomMatchers.matchers);

describe("My custom data", ({test}) => {
  test("something", ({expect}) => {
    expect.ext.customData(blah).toBeLoggedIn();
  });
});

In the version we have open sourced I do not see this functionality (or at least it isn't documented yet). We will add it back, and hopefully we can even clean it up a bit so that you might not need to access the extensions through expect.ext. This should allow us (or anyone else) to write some good opinionated matchers for these frameworks and release them without having to force them into the core matchers.

kyldvs avatar Dec 01 '18 23:12 kyldvs

I have spent some time thinking about this. Right now there is some refactoring that needs to be done in order to better isolate tests/nested reporting/setup and teardown behavior/custom reporters before working on something like this makes sense. That said, I plan on taking on that work in the very near future (probably this coming week).

I think that the ultimate implementation should look something like the testAsync or testLwt suggestion that you offer and should be fairly straightforward to implement/experiment with once the work I mentioned above is done.

I don't think custom matchers will be sufficient for supporting async tests in the current world, however they might offer a short term workaround? Fundamentally right now a custom ."toResolveTo()" matcher would still have to be run synchronously.

The custom matcher API isn't exposed yet, however it should be fairly easy to build in. The only hesitancy with exposing it now I have is that highlighting file context is currently flakey with failure behavior. We may need to change test failure to use exceptions to better capture stack traces, which would fundamentally change the matcher API (e.g. instead of returning tuples, matchers would be calling "pass" and "fail" functions internally)

bandersongit avatar Dec 01 '18 23:12 bandersongit

Can I conclude correctly from this issue that it's not possible to use Rely with lwt at the moment?

tcoopman avatar Feb 27 '19 12:02 tcoopman

@tcoopman you are correct

That said, adding lwt support shouldn't be too difficult at this point (all my concerns from December have been addressed) and I think the only open questions related to implementation are:

  • testAsync vs testLwt per @bryphe
  • if testLwt, do we build directly into Rely, or do we have a separate Rely-lwt package (virtual modules under the hood)
    • I think I vote separate package in this case

bandersongit avatar Feb 27 '19 15:02 bandersongit

  • I think I vote separate package in this case

Makes sense. Baking it in seems be too opinionated - you could image Rely-async or Rely-repromise implementations perhaps too.

bryphe avatar Feb 27 '19 15:02 bryphe

For anyone looking to implement a quick and dirty solution, using Lwt_main.run to suspend the calling thread makes this test execute as intended:

open TestFramework;


let _ = describe("testing Lwt methods", ({ test, _ }) => {
  test("org should be orgname and mongoservice host should be http://localhost:3000", ({expect, _}) => {
  let lwt_res = {
    let%lwt res =
      Lib.Db.Postgress.add_org(
        "orgname",
        "[email protected]",
        Lib.AppState.pool,
      );
    switch (res) {
    | Ok((_, name, _)) => Lwt.return(Ok(name))
    | Error(_) as e => Lwt.return(e)
    };
  }
  let res = Lwt_main.run(lwt_res)
  expect.result(res).toBe(Ok("orgname"))

  expect.string(Lib.AppState.mongoservice_host).toEqual("http://localhost:3000")
  })
})`

dakotamurphyucf avatar Nov 12 '19 17:11 dakotamurphyucf