Unit test suite not running because of visual tests

[limzykenneth]

Most appropriate sub-area of p5.js?

• [ ] Accessibility
• [ ] Color
• [ ] Core/Environment/Rendering
• [ ] Data
• [ ] DOM
• [ ] Events
• [ ] Image
• [ ] IO
• [ ] Math
• [ ] Typography
• [ ] Utilities
• [ ] WebGL
• [ ] Build process
• [X] Unit testing
• [ ] Internationalization
• [ ] Friendly errors
• [ ] Other (specify if possible)

p5.js version

current

Web browser and version

n/a

Operating system

n/a

Steps to reproduce this

Steps:

1. Run unit tests with npm test.
2. Unit tests for typography is not run and its relevant entry cannot be found in the logs

I was working on a fix to #6997 and noticed that in the unit test suite for textWrap() it is expecting the function to throw on incorrect argument type and if it does not throw (say when switch to using FES which does not throw), the unit test should be failing but for some reason it did not.

After some digging I found that this behavior starts in commit https://github.com/processing/p5.js/commit/7fb55a44e4e52631e8bef7d38de5972c959e2d7c, specifically by adding the test/unit/visual/cases/typography.js file. Removing the file makes the unit test work as it should again.

@davepagurek I'm not sure if it is due to the unit test clashing with the visual test somehow and whether this is actually happening to the WebGL tests as well. Is it possible to further decouple the unit test and visual test, perhaps to the point where they are run by two different tasks if need be? I'll be fixing #6997 first along with its unit test, although it is not run without fixing this. I can try to look a bit deeper if you are not quite available to look at this yet.

[davepagurek]

Agh I think I accidentally committed this, just running the one file: https://github.com/processing/p5.js/blob/7fb55a44e4e52631e8bef7d38de5972c959e2d7c/test/unit/visual/cases/typography.js#L20

[davepagurek]

Does it run if you remove that?

[limzykenneth]

@davepagurek Yes that worked, now there's just a previously failed test that wasn't caught

1) Graphics
       p5.Graphics.resizeCanvas
         Rendering
           p5.prototype.resizeCanvas
             p5.Image
               p5.Image.prototype.mask
                 it should mask images from createGraphics:
     ReferenceError: createGraphics is not defined
      at Context.<anonymous> (unit/image/p5.Image.js:128:18)

[davepagurek]

Looks like it's from the PR I just merged, there's a createGraphics that should be a myp5.createGraphics. I'm just running tests again to make sure it's nothing more than that

[davepagurek]

ok looks like it has another failure, I'll figure out what's up and make a PR

[davepagurek]

I put up this PR fixing that test case: https://github.com/processing/p5.js/pull/7010

I also looked into making CI fail when you accidentally keep .only, but I couldn't actually get mocha to make it fail. Do you know if vitest has something like that? It would be great to at least get that for 2.0 if not now.

[limzykenneth]

For Vitest it works kinda opposite in that it has allowOnly which by default is false in CI environment but still allow it in local development. Another thing for mocha that can potentially help catch is to use ESLint rule.

[davepagurek]

that default behaviour sounds like exactly what we want! that's good to know, thanks