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 tests #4

Closed
agilgur5 opened this issue Jun 13, 2019 · 3 comments
Closed

Add tests #4

agilgur5 opened this issue Jun 13, 2019 · 3 comments

Comments

@agilgur5
Copy link
Owner

agilgur5 commented Jun 13, 2019

Definitely needed to make some after #2 . I don't have a project to test the browserify integration in and apparently webworkify usage differs quite significantly from webpack's worker-loader.

This issue is more to document my attempts at getting tests to work and failing spectacularly 😕

There's quite some difficulty in getting tests to work as there's a bunch of requirements on browser environments here:

  • Running tests in Node gets errors on print not existing in wrappers to Ammo (not that Ammo even uses print).
  • Running tests with browser-env gets SyntaxError: Failed to execute 'postMessage' on 'Window': Invalid target origin '[object ArrayBuffer]' in a call to 'postMessage'. because PhysiJS passes an ArrayBuffer as the second arg to postMessage at least one time (to test if Transferable messages / TypedArrays are supported). This is the third argument in Window.postMessage but the second in Worker.postMessage. Perhaps jsdom didn't implement the latter and is only using the former? Very not sure. Tried jsdom-worker (with jsdom-global too) and that gave the same error 😕
  • Running tests with cypress doesn't quite fit the model as we're not doing UI tests/UI automation; just need to run tests in a real browser environment 😅 . I feel like I might have to write some tests in the old way in <script> tags or something to get them to work...
@agilgur5
Copy link
Owner Author

Ok, so jsdom actually doesn't support workers at all, so Worker.postMessage doesn't even exist. jsdom-worker would indeed be the right library to use.

This gave me a lead, because if Workers aren't even supported, how was it erroring on postMessage from physijs_worker.js? Ok, well turns out the browserify integration is even more complex, as not only does webworkify instantiate a Worker for you, it also requires that you export a function with no arguments as your Worker module because all code outside of the exported function will run in the main thread. So I have to create a stub for physijs_worker.js just for webworkify that just exports function () { require('./physijs_worker.js) }. Yea this is way more complex and more effort than I intended to support Browserify......

@agilgur5
Copy link
Owner Author

Ok finally got it to work with browser-env and @react-frontend-developer/jsdom-worker after even more bugs.

Now the only annoying issue remaining is setting up a test runner. The reason it's annoying is because the code needs to be transpiled with browserify (or webpack) to test that the integration is working. But at least when bundling with browserify, all exports are lost -- it's just meant to be run as a script. So I might need to add yet another stub to export as a window global or something and then import it as a global in the test 😕 😕 😕

Unless there's some other way around this...

This was referenced Jun 13, 2019
@agilgur5
Copy link
Owner Author

I ended up adding some small stubs ¯\_(ツ)_/¯ . Resolved by #5 . For future work for Webpack tests see #7

This was referenced Jun 14, 2019
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

No branches or pull requests

1 participant