pact-foundation / pact-js

JS version of Pact. Pact is a contract testing framework for HTTP APIs and non-HTTP asynchronous messaging systems.
https://pact.io
Other
1.58k stars 342 forks source link

Vitest - Pact Verifier fails with 3 or more interactions #1220

Open shishkin opened 3 weeks ago

shishkin commented 3 weeks ago

Software versions

Issue Checklist

Please confirm the following:

Expected behaviour

It should either work or explain clearly what user did wrong.

Actual behaviour

I'm running provider tests and as long as I get 3 or more interactions in the pact file, subsequent interactions start failing due to state change handlers. Weirdly, it fails when I duplicate/copy paste interactions in the pact file, so the same interaction that just succeeded will now fail.

Steps to reproduce

  1. Create an interaction and see it succeed.
  2. Duplicate it twice and see the last one fail.
  3. Move interactions in the file around and see a different one of the same interactions fail.

See test and pact JSON.

Relevant log files

See log.

YOU54F commented 3 weeks ago

can you update your packages so pact-js pulls in pact-js-core 14.3.7

i believe this is fixed and your logs show they are running 14.3.6

https://github.com/pact-foundation/pact-js/issues/1216#issuecomment-2137209732

shishkin commented 3 weeks ago

I still observe the same errors with pact-js-core 14.3.7.

YOU54F commented 3 weeks ago

The error is

libc++abi: terminating due to uncaught exception of type Napi::Error

I can resolve with this workaround, tested locally

https://github.com/vitest-dev/vitest/issues/2972#issuecomment-1948012464

mefellows commented 3 weeks ago

Glad you got to the bottom of this Yousaf!

We may need to look into how we can make the core threadsafe, if possible, to enable multi-threaded tests like this (see https://github.com/nodejs/node-addon-api/blob/main/doc/threadsafe.md). For now, that workaround is good to know.

YOU54F commented 3 weeks ago

no worries man, the reproducer was really helpful, so thanks to @shishkin

It might be worth getting a basic vitest example in place, and utilise this test to reproduce our issue, document the current workaround, and it will help us test out any solutions from the linked doc.

💯 enabling multi-threaded tests would be a nice bonus rather than having to forgoe them

shishkin commented 3 weeks ago

Thanks @YOU54F. Indeed, running vitest with --pool=forks resolves the issue. I think I can live with it for now, since provider verifier is only a single test.

mefellows commented 3 weeks ago

Thanks @shishkin. If you do feel inclined to go deep to see if we can make it multi-threaded, we'd be happy to give you some pointers (TL;DR - it'll be in here somewhere: https://github.com/pact-foundation/pact-js-core/blob/master/native/provider.cc).

shishkin commented 2 weeks ago

@mefellows sorry, I'm not competent in native development to dig in.