search

bytecodealliance / jco

JavaScript toolchain for working with WebAssembly Components
https://bytecodealliance.github.io/jco/
Apache License 2.0
618 stars 62 forks source link

deno: reenable Deno CI tests #414

Closed guybedford closed 6 months ago

guybedford commented 6 months ago

Thanks to the work by @mash-graz the latest Deno release now supports far more of the test suite.

This reenables the Deno CI test suite and also enables more of the tests - we are now passing 64/106 tests in Deno.

mash-graz commented 6 months ago

I was just waiting for this new release before I send you some jco resp. preview2-shim patches to eliminate a few more compatibility issues,

Many of the reported errors are not very important in practice. Most of them are just reporting, that deno still reports port number 0 instead of the automatically chosen one and also for wildcard interface 0.0.0.0 a similar error will be displayed. But both cases are nearly negligible.

But other compatibility issues have to be fixed. For example deno doesn't support setting the TTL value for UDP packets. It will throw an NotImplemented Error, but jco doesn't catch this case, therefore every UDP command will fail...

Some more complex workarounds, which I had to figure out, are not necessary anymore, because the deno maintainers spend a lot of time improving the node:workers_threads implementation. Most of the grave defects of this node compatibility module are now solved in deno.

There is still one showstopper remaining: The file access in different threads resp. worker contexts doesn't work. I already reported it here (#400). We could work around this issue by utilizing the synckit worker indirection also for all file system related operations, but this would have negative consequences for the performance. @bartlomieju thinks, that this incompatibility can be also solved on denos side after another complex change. (see: https://github.com/denoland/deno/issues/23012)

guybedford commented 6 months ago

Great to hear there may still be some low-hanging fruit. I've also disabled the Windows Deno tests for now as they had a few more errors as well.

It was fantastic to see the worker improvements - if anything this Jco compat does ensure Deno has very strong Node.js compat, so glad we can put pressure on that through these tests.

I'm open to your suggestions for any workarounds needed in Jco. Moving all filesystem descriptors to the worker could well be an option - alternatively we could even make that an implementation detail that is Deno-specific if it helps and this can't be fixed upstream.

bartlomieju commented 6 months ago

Hey @guybedford @mash-graz.

Thanks for the ping and thanks for surfacing this project to us. It's great seeing that we have 60% compatibility, but we'd like to get this number closer to 90% and 100% preferably. I'll schedule some time to figure out which tests are failing and we will keep our eye on https://github.com/denoland/deno/issues/23012 as it seems like a must to get to that number.

guybedford commented 6 months ago

@bartlomieju it was really great to see this progress! So glad you are interested in seeing these worked through. I can also aim to do some more issue isolation in due course as these cases progress.