-
Notifications
You must be signed in to change notification settings - Fork 106
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
Adding NodeJS tests for WASM module #262
Conversation
bindings/wasm/test/test.js
Outdated
const genus = manifold.genus(); | ||
return { ...prop, genus }; | ||
} catch (error) { | ||
console.log(error.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering do we need to catch the error. I think it is also fine to just let the error propagate and catch by the unit test framework?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
FYI @stewartoallen, this adds a JS test framework you're welcome to add to. It's also a breaking change for our WASM, as it's now a module, so you have to update how it's imported. I think when we get your PR sorted out we'll probably do a v2.0 npm release. |
* modularize wasm and add mocha tests * add npm tests to CI * fix mv command * revert to build directory * added rest of example tests * fixed three.js test * update web examples for module
In order to get this working in Node I needed to modularize our WASM bundle, which I think is a good practice regardless. I tried going all the way to an ES6 module, but couldn't make it work. UPDATE: turns out I wasn't crazy, ES6 modules from Emscipten don't yet work in NodeJS. So we'll just stick with CommonJS for now until they fix that (I don't feel like dealing with workarounds, since this works just fine).