-
Notifications
You must be signed in to change notification settings - Fork 657
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
Second working port to Rollup #525
base: develop
Are you sure you want to change the base?
Conversation
Tests should work in Node (I will double check). It just imports from the bundled commonjs file (in dist). And Node tests have to use For tests, I'm still looking into it. WebGL in browser is notoriously difficult to unit tests. It's even more difficult when there's hundreds of unit tests, like here. Plus, this is testing GPGPU math, not visual output to a canvas. Some options include:
I'm trying to evaluate which one would be best. P.S. is it too much to ask that v3 only exports the GPU class? I don't think many people use anything other than that. |
All tests need to run in both Node and Browser.
We're aware, hence the seemingly obscure means of unit testing.
Those (I've not checked the others) only test Chrome. For us, "The browser" is both desktop and mobile for Firefox, Chrome, Safari, and sometimes Edge.
Yes. There may be too much exposed in it as it is, but there still needs to be a baseline. |
If possible, is there any way we can not use building for node packages? It makes it so that we have double the src files. I mean, if it comes down to using |
You don't have to use double the src files. The QUnit browser tests already use the bundled version found in dist/ (all.html defines require() to simply redirect to an object based on the name). Both node and browser tests should work.
(just like in the official instructions). To run the browser tests: Both I realize that Puppeteer and Electron use Chrome, but Babylon.js, THREE, A-Frame, and every other major WebGL project I know of have no problem with that; It lets them catch enough errors that the exceptional cases when other browsers differ can be handled manually. Though I admit, this one depends more on mathematical correctness than visual output. |
I'll resolve this: #521 then I'll work on helping here, as it is the last high priority item. Sorry to keep this help up. I did find an issue with build that I've fixed locally, I'll release today that may have contributed to the build issue I was seeing. |
Thanks! I appreciate your interest. Did you run the tests for both node and browser? |
Not yet. |
Is there any interest in continuing this effort? Quite a few conflicts. |
I merged #499 too early, thinking that the node unit testing side was correctly implemented. Because of how drastic the change was, and that it no longer allowed testing in node, I reverted using git checkout HEAD~1 and a force push. I then reopened this in hopes that the changes will handle node eventually, and to keep a PR open and actionable.