search

pwlmaciejewski / imghash

Perceptual image hashing for Node.js
MIT License
167 stars 19 forks source link

Workaround for #21 #48

Closed bbhopesh closed 4 years ago

bbhopesh commented 4 years ago

Workaround for longjmp bug in cwasm-jpeg-turbo The bug was originally reported in #21

The workaround is to fallback on jpeg-js

bbhopesh commented 4 years ago

Hey @LinusU @pwlmaciejewski , can you please take a look at this?

bbhopesh commented 4 years ago

Hey @pwlmaciejewski I have added the test case and updated the PR. The test suite is passing with latest changes, and the test suite fails without these changes. So, the change is good.

However, output of the test suite is weird when it fails. It shows some other test as failing even though the newly added test is the one that is actually failing.

pwlmaciejewski commented 4 years ago

However, output of the test suite is weird when it fails. It shows some other test as failing even though the newly added test is the one that is actually failing.

Thanks, @bbhopesh! I left you comment regarding this ☝️

bbhopesh commented 4 years ago

Hi @pwlmaciejewski I have changed as suggested by you but the issue doesn't go away. I will try to see if I need to change other tests too. Can you please also take a look? Using resolves was not helping because internally async/await and resolves work the same way. The solution was to not use toThrow at all. Even without it test fails if hash throws an error. So, I think we should be good to merge now.