search

FuelLabs / fuels-ts

Fuel Network Typescript SDK
https://docs.fuel.network/docs/fuels-ts/
Apache License 2.0
44.39k stars 1.32k forks source link

Browser tests are inconsistently failing due to dynamic `istanbul` import #2612

Closed maschad closed 6 days ago

maschad commented 1 week ago

There are a few PRs currently failing on CI due to the failure to dynamically fetch istanbul for code coverage. This doesn't happen consistently though.

Here are a few affected PRs:

arboleya commented 1 week ago

Bumping priority to p0 as it seems to affect everything else. cc @nedsalk

maschad commented 1 week ago

I see that some of the CI jobs are now passing upon re-running the jobs, so the PRs in my description may become outdated.

petertonysmith94 commented 1 week ago

The message may be misleading, as the logs the following errors are present:

Module "child_process" has been externalized for browser compatibility. Cannot access "child_process.spawn" in client code. See https://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.
Module "child_process" has been externalized for browser compatibility. Cannot access "child_process.exec" in client code. See https://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.

This appears to have been introduced in the following PR (based upon the CI failures):

I can imagine that there is failure path that is causing child_process to be called in the browser env, not 💯 sure where yet.

cc @nedsalk

danielbate commented 1 week ago

Indeed child_process will not work in browser. When bundling, every time vite identifies that dep it'll give that warning. However, we should be dynamically importing it everywhere it is actually needed in tests. I'd expect it to throw on that line otherwise, like it did in the past before we had a dynamic import.

We could try the v8 coverage provider.