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

Multiple assertions in specified order #52

Closed
Yakimych opened this issue Apr 13, 2019 · 7 comments
Closed

Multiple assertions in specified order #52

Yakimych opened this issue Apr 13, 2019 · 7 comments

Comments

@Yakimych
Copy link

Hi. I am looking into porting some API-tests written in F# from our backend to OCaml/Bucklescript. I've read the documentation part about all tests having to return an assertion, which forces to have one and only one assertion, so I am wondering whether it's possible to achieve such an imperative testing scenario with bs-jest at all. So far, I have managed to make one API call and make one assertion on the result, but have no idea how I would go about chaining another API call using the ID returned by the first one and making another assertion.
Here is my simple attempt with one assertion:

testPromise "getResources should return 20 resources" (fun () ->
  getResources "url"
    |> Js.Promise.then_ (fun res ->
      expect (res |. Belt.Array.length) |> toBe 20 |> Js.Promise.resolve
    )
);

Here is a sample F#/xUnit test I am trying to convert:

[<Fact>]
let ``Sample test for adding and removing a resource`` () : unit =
    let createResponse = createRandomResource ()

    getAllResources()
    |> List.map (fun i -> i.Id)
    |> should contain createResponse.Id // First assertion

    deleteResource createResponse.Id  // Using the ID from the first response

    getAllResources ()
    |> List.map (fun i -> i.Id)
    |> should not' (contain createResponse.Id)  // Second assertion

Writing such a test with TypeScript and async/await is pretty much straightforward.

@glennsl
Copy link
Owner

glennsl commented Apr 13, 2019

The F# example doesn't look very asynchronous, unless there's a whole bunch of magic going on there. It also looks like bad test design since it tests two different properties in a single test. Creation and deletion seems like it should be separate tests, but perhaps this is just a contrived example.

In any case, you can just chain the promises. That's what they're for, after all. Something like this:

testPromise "deleteResource" (fun () ->
  getAllResources ()
    |> Js.Promise.then_ (fun before ->
      deleteResource resource.id
      |> Js.Promise.then_(getAllResources)
      |> Js.Promise.then_(fun after -> expect (before, after) |> toEqual ([resource], []) |> Js.Promise.resolve)
    )
);

@Yakimych
Copy link
Author

Yakimych commented Apr 13, 2019

The F# example doesn't look very asynchronous

You are right, those calls are synchronous. An asynchronous version would look something like this in an async computational expression:

[<Fact>]
let ``Sample test for adding and removing a resource`` () =
  async {
    let! createResponse = createRandomResource ()

    let! before = getAllResources()
    before
    |> List.map (fun i -> i.Id)
    |> should contain createResponse.Id // First assertion

    do! deleteResource createResponse.Id  // Using the ID from the first response

    let! after = getAllResources ()
    after
    |> List.map (fun i -> i.Id)
    |> should not' (contain createResponse.Id)  // Second assertion
  }

Creation and deletion seems like it should be separate tests

Not sure how that could work - those are API (Integration) tests - they run against a "real" environment with a real database and everything. Unless the test creates a resource, there is nothing it can delete in order to test whether deletion works, so the deletion test would have to create the resource in any case, even if it was a separate test.

Thanks for the example, I will give it a try! Chaining and nesting promises this way looks like a readability disaster though 🙃
I am relatively new to OCaml - do you happen to know whether there are ways to flatten this out, similarly to async/await or an async computational expression as in F# (or the do-notation in Haskell)?

@glennsl
Copy link
Owner

glennsl commented Apr 13, 2019

I'm not a big fan of integration tests that mutate shared state since failure can be caused by executing tests in different order, failing other tests, and because it requires a lot more setup and teardown which obfuscates the actual property you're trying to test. I'd rather use a mock database that's just thrown away and rebuilt for each test. Then you can just delete a known resource, and that's that.

The async computation expression will produce code that is essentially exactly like the nested promise chain I showed. You can make it a bit nicer by defining then_ as an operator, typically >>= (and called "bind"), but to flatten it out you need a syntax extension. Jaredly has a ppx that does something like this, and you might find some other approaches in this lengthy issue on the Reason repo. OCaml itself is also getting "monadic let operators", probably in the next release, but it's unlikely that BuckleScript will support this for several years. So the future is bright, but it's still a bit off in the future somewhere.

I'll close this issue now as I think it's been answered as best it can, but feel free to keep commenting if you have further questions or suggestions or whatnot :)

@glennsl glennsl closed this as completed Apr 13, 2019
@Yakimych
Copy link
Author

Yakimych commented Apr 13, 2019

I'm not a big fan of integration tests that mutate shared state

Neither am I, to be honest. In this case, however, it's not really a matter of choice - the backend/API/database package is simply a black box that we don't have control over, but rather build a frontend on top of. So a mock database and similar solutions are not really applicable.

I do need those tests on the frontend side, however, since it's rather common that backend changes of the API break stuff on the frontend that TypeScript is ill-equipped to handle due to the lack of runtime checks. OCaml JSON Decoders come in really handy (edit: just realized that bs-jest is your package too 😄), so that's the solution I am trying out now - might even do a writeup on the topic if it turns out a success :)

The async computation expression will produce code that is essentially exactly like the nested promise chain I showed

True, but the code would be "flat" - it's the "nestedness" I am concerned about. Maybe it's a matter of preference, but especially if there are more operations to perform in a sequence that depend on each other, and the number of levels of nesting grows, I find it increasingly more difficult to follow.

Thanks for the links, really useful stuff! Looks like there is a suggestion to backport this syntax even before bs supports OCaml 4.08, so hopefully it won't take years :) Meanwhile, I am actually leaning towards writing such tests in TypeScript. As much as I dislike it, it might just be a better fit, given the imperative/sequential nature of those tests.

Thanks again for your response!

@glennsl
Copy link
Owner

glennsl commented Apr 14, 2019

You need to teach those back-end guys about API-versioning and backwards-compatible API changes it seems. But yeah, I can understand the need in that case.

Maybe it's a matter of preference, but especially if there are more operations to perform in a sequence that depend on each other, and the number of levels of nesting grows, I find it increasingly more difficult to follow.

No, I completely agree. My point is just that it's not something that can be fixed by changing the API. The problem inherently requires language support. So for the moment a different language might be what you need.

@glennsl
Copy link
Owner

glennsl commented Apr 14, 2019

There exists a port of ppx_let for BuckleScript too it seems.

That would allow you to write

let%bind something = ...

similar to

let! something = ..

in F#

@Yakimych
Copy link
Author

You need to teach those back-end guys about API-versioning and backwards-compatible API changes it seems.

Hehe, they know :) Since API v1 hasn't been "officially" released yet, versioning and backwards compatibility would result in too much overhead and thus were consciously omitted for now. Who knows, maybe the need for such tests is not going to be as pronounced when backwards-compatibility is respected.

My point is just that it's not something that can be fixed by changing the API.

Yeah, the only thing I was thinking could help in this situation would be an assertion in the middle of the test. I supposed that could help with the nesting, since I wouldn't need to carry over the before variable to the last assertion. On the other hand, now that I think about it, I would still need the information returned by the first promise (Id) in order to make the deletion request, so I guess I was wrong the whole time 🙂

Will go try out ppx_let, thanks!

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