Skip to content
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

add catch block to worker message handler, resolves #6 #7

Merged
merged 1 commit into from
Jul 22, 2020

Conversation

danielbank
Copy link
Contributor

This PR adds a catch block to the worker's message event handler. If the call() receives this message, it rejects its promise and downstream code can detect it.

@danielbank
Copy link
Contributor Author

danielbank commented Jul 22, 2020

I didn't add any unit tests but I didn't see any in place. If you're open to it, I might take a stab at that

@bminixhofer
Copy link
Owner

Hi, thanks for this PR! LGTM, I'll merge this later today.

Regarding Unit tests: That would definitely be useful!

There are integration tests using Cypress in quality/tests/test.js which compare the output of tractjs with that of tract.

I am not sure how to go about adding unit tests. Would you use Cypress too? Or some node script? (in that case, tractjs would have to be made compatible with Node.js first ;) )

@danielbank
Copy link
Contributor Author

I was thinking something like Mocha (https://mochajs.org/). Some other alternatives are Jest (https://jestjs.io/) or Jasmine(https://jasmine.github.io/) but Jest is more centered around UI and Jasmine is quite a bit older at this point. They run via Node but I don't think it is a problem to have them as a dev dependency. I'll try and take a look sometime this week

@bminixhofer
Copy link
Owner

Sounds good, the problem is that at the moment tractjs uses fetch internally so it doesn't work in Node out of the box. This used to be a bigger problem (when it was only Rust code) but now that there is a Typescript wrapper anyway it's actually really easy to fix.

@bminixhofer bminixhofer merged commit 0e6d78f into bminixhofer:master Jul 22, 2020
@danielbank danielbank deleted the fix/throw-errors branch July 25, 2020 06:35
@bminixhofer bminixhofer mentioned this pull request Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants