Shouldn't test return a promise?

[tsnobip]

Right now test is modeled like this:

@send external test: (t, testTitle, zoraTest) => unit = "test"

Shouldn't it be modeled like this instead:

@send external test: (t, testTitle, zoraTest) => Js.Promise.t<unit> = "test"

Otherwise you can write multiple tests in a Zora block, finish with a done() and get an error:

tried to collect an assertion after it has run to its completion.
You might have forgotten to wait for an asynchronous task to complete

What do you think?

[dusty-phillips]

Technically yes, in the sense that Zora test() returns a promise so it should be modelled the same way. I left it off because I didn't want to have to add a bunch of ->Promise.ignore calls cluttering up the Rescript syntax and all the Zora documentation silently ignores the return of test().

That said, I haven't seen your problem before. Are you doing the same thing as parallel.test.res? Does that file work for you?

I'd add it if it seems necessary, but the only reason I can see would be that you collect the promises inside your file and call Promise.all to wait for them all. But I think (I haven't actually spent much time in the Zora codebase) that the Zora test harness is doing this on your behalf so you shouldn't have to. 🤔

[brettcannon]

@tsnobip Did Dusty's suggestion work for you?

Are you doing the same thing as parallel.test.res? Does that file work for you?

[tsnobip]

@tsnobip Did Dusty's suggestion work for you?

Are you doing the same thing as parallel.test.res? Does that file work for you?

to be honest I don't remember anymore, I left the company where I encountered this issue since then and switched to vitest for my new projects 😬

[dusty-phillips]

The best solution here is to migrate everything to async/await syntax. I think that would clarify the docs.

[brettcannon]

The best solution here is to migrate everything to async/await syntax.

@dusty-phillips The only explicit promise I can see is:

https://github.com/dusty-phillips/rescript-zora/blob/main/src%2FZora.res#L2

So are you saying to move that line and the tests?

[dusty-phillips]

Ah, it appears I already migrated it to async/await. 😅 I’m pretty sure this is a legacy issue from when I was depending on an external promise library. I went to a lot of trouble to make tests parallel by default, but I think most of that trouble was abstracted away with async/await syntax. Will close for now unless it comes up again.

[tsnobip]

I'm also pretty sure it now works out of the box.