Set up testing

[lindapaiste]

I forked @ziyuan-linn's previous attempts and added a few things to get this kinda working.

• BodyPose tests pass ✔️
• HandPose tests fail 🚫 but I'm not going down this rabbit hole today.

More work still needs to be done here. The biggest underlying issue is getting browser code to run in Node.js and vice-versa. Our library is designed to be executed in the browser but these tests are run via Node.

• The polyfills in setupTests.js don't seem to apply globally, and need to be repeated in each test file.
  • The ImageData one was an issue in the old repo too which is why there is a polyfillImageData function in the testingUtils file.
  • I don't remember fetch being a pain. I'm looking to see where we used a polyfill and it was only in one specific test.
  • TFJS has its own built-in support for fetch when using the "node" platform instead of the "browser" platform, but I couldn't figure out how to get that to work. I could only get it to stick on node if I used jest-environment-node, but that created other problems.
  • There was one test in the old repo where I explicitly set the TFJS environment to fix an error and that did work.
• We get a bajillion console warnings from TensorFlow, including warnings telling us to use the @tensorflow/tfjs-node package which I tried to set up but I still get the warnings.

See also
ml5js/ml5-library#1345
ml5js/ml5-library#1306

[ziyuan-linn]

Thank you @lindapaiste for your help! I was trying to set up testing a while ago but couldn't get it to work.

[shiffman]

Yay! This is great, we haven't had any bandwidth to implement testing yet so this is an area that we could use a lot of help with! I don't have a lot of experience with writing tests so don't feel very qualified to review. I'm not sure if @gohai feels the same? Would it make sense to review and merge this with the BodyPose tests only or @lindapaiste should we wait until you have the HandPose tests working as well?

[lindapaiste]

I think that we should merge it (after resolving conflicts) so that we can continue adding more tests and hope that they will run.
TBH I'm pretty good at writing the actual tests but this setup stuff is 🤯. We can make further changes later.

[lindapaiste]

As an aside - It is slightly concerning that the old model had a 0.999 confidence score for detecting the nose and the new model is only 0.721.

[gohai]

I am wondering if the confidence score will change when running it with the tfjs runtime rather than mediapipe 🤔

[shiffman]

Yes, maybe it would make more sense to just validate that a floating point confidence score is outputted rather than a specific value? But we can address this later!

[shiffman]

Hi @lindapaiste! At our meeting at NYU today everyone expressed a lot of excitement and enthusiasm for adding these unit tests! We don't have a lot of experience writing and maintaining unit tests, but I'm happy to do my best to review these pull requests. Feel free to let me know when you feel when this branch is ready to be merged and I will check things over and merge! I think it's great to work on the tests in stages so even if we bring them in slowly, one at a time (just bodypose to start for example), that works for us!

[lindapaiste]

TBH I wonder if it makes sense to go back to Karma, which launches a browser window for running the tests. https://karma-runner.github.io/6.4/intro/how-it-works.html Or Mocha https://mochajs.org/#running-mocha-in-the-browser

Here's a discussion from 2018: ml5js/ml5-library#41 (comment)

Jest is good for pure JS logic but it's a struggle to get a full end-to-end detection/classification of a media file. IMO we want tests that actually run the model and make sure that we get the correct output. I'm not a fan of mocking as you're not testing the actual library.

I tried a few things with regards to testing a video and I pretty much gave up. The video "load" never happens so it hangs up on the await mediaReady. I got past that by faking the video load (altering the readyState) but then I got failures further down the chain when TFJS fromPixels tries to read the data.

Is support for running ml5.js in Node.js something that's on the agenda? If so then we should stick with Jest because a lot of the issues that come up in these tests are genuine issues that would come up when using the library.

Edit: After playing around with this a bit, I think that we want to stick to using Jest for writing the tests but modify the setup so that it runs in the browser. I will look into some of the strategies here: https://stackoverflow.com/questions/66084506/use-jest-with-web-test-runner.

I played around with using Web Test Runner to run tests on our examples folder. I created a test.html in an example and imported the sketch.js file. However this uses Chai as the assertion library and I am less familiar with that syntax (expect(bodyPix).to.be.undefined vs Jest expect(bodyPix).toBeUndefined()). I added Sinon for spying on what arguments were passed to a callback but again it's a syntax that I'm less familiar with. So I would like to figure out something that uses the Jest syntax.

[shiffman]

Is support for running ml5.js in Node.js something that's on the agenda? If so then we should stick with Jest because a lot of the issues that come up in these tests are genuine issues that would come up when using the library.

@lindapaiste Speaking for myself, this is something that I would like to see in the future, but it is not something I would prioritize in the near term. For me, having 1.0 out and released with updated models and refreshed documentation for use with p5.js client-side is the top priority. I think we could revisit the node.js question after that has happened.

Let me know when you want me to review for merging! Maybe it makes sense to merge with 1 or 2 tests only and then others can contribute additional tests or updates in separate branches. I don't have an opinion on jest or karma as I don't really have experience with either! If you are excited to start with a platform I would say go for it!

[lindapaiste]

@shiffman I think let's go ahead and merge this as-is right now. We can continue to make improvements in other branches/PRs.

[shiffman]

Excited to merge this! I'll start playing with the tests more and we can incrementally improve them and add more. I can look into how to get them running as CI checks on github too!

[gohai]

@lindapaiste Are those .babelrc and babel.config.json files in the top-level directory strictly necessary for the test infrastructure to work? (Always a bit deterring encountering large number of dot or configuration files as the first thing one looks at.)

Also wondering if setupTests.js might also want to go a folder tests or testing (or whatever is most common for those)?

[lindapaiste]

@lindapaiste Are those .babelrc and babel.config.json files in the top-level directory strictly necessary for the test infrastructure to work? (Always a bit deterring encountering large number of dot or configuration files as the first thing one looks at.)

We can move it into package.json.

Also wondering if setupTests.js might also want to go a folder tests or testing (or whatever is most common for those)?

Sure. It might also be cleaner/clearer if we move the test files into that folder, instead of putting them alongside the source code.

[gohai]

Thank you @lindapaiste