feat(tests): add chromium browser mode for unit tests #10568

Smrtnyk · 2025-04-21T16:23:05Z

How it works

"test:vitest" still runs tests in jsdom
"test:vitest:coverage" still runs tests in jsdom

new scripts added

"test:vitest:chromium" runs same set of tests but in chromium using vitest browser environment

There is no magic in here
just some small adaptations needed for tests to run in both environments now
I will comment those inline

codesandbox · 2025-04-21T16:23:08Z

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

Smrtnyk · 2025-04-21T16:23:56Z

.gitignore

@@ -25,3 +25,4 @@ e2e/test-report/
 e2e/test-results/
 **/.cache/
 docs/
+__screenshots__


added this to gitignore because now if test fails there is a screenshot folder so you can see how page looked like at the time of failure
for example, if your test had canvas appended to the document, you can see how canvas looked like when it failed the test

Smrtnyk · 2025-04-21T16:25:00Z

src/Pattern/Pattern.spec.ts


 describe('Pattern', () => {
-  const IMG_SRC = 'greyfloral.png';
+  const IMG_SRC = isJSDOM() ? 'greyfloral.png' : GrayFloralImage;


in old qunit these links had isNode and they fetched images differently

now in browser mode we need to import image directly that vite resolves internaly, where in jsdom it is as it was before

this change was made for few more test files

github-actions · 2025-04-21T16:25:06Z

Build Stats

file / KB (diff)	bundled	minified
fabric	913.867 (0)	301.234 (0)

Smrtnyk · 2025-04-21T16:25:26Z

src/canvas/Canvas.spec.ts

@@ -130,15 +132,15 @@ describe('Canvas', () => {
      const canvas = new Canvas(undefined, {
        allowTouchScrolling: false,
      });
-      const touch = {
+      const touch = new Touch({


I had to add this polyfill for jsdom, because these plain objects passed to TouchEvent did not work in chromium

src/canvas/StaticCanvas.spec.ts

src/canvas/__tests__/__snapshots__/eventData.test.ts.snap

Smrtnyk · 2025-04-21T16:27:47Z

src/parser/parser.spec.ts

@@ -675,7 +675,8 @@ describe('fabric.Parser', () => {
  it('getCssRule', () => {
    expect(getCSSRules).toBeDefined();
    const rules: Record<PropertyKey, any> = {};
-    const doc = getFabricDocument();
+    // NOTE: We need to use a new fresh document here because vitest in browser mode already adds some stylesheets which pollutes the test
+    const doc = getFabricDocument().implementation.createHTMLDocument('');


in vitest browser mode there are already some stylesheets imported to testing frame by vitest, so I had to get clean doc implementation here
works fine in both jsdom and chromium

This test i think is wrong.
The parser and getCSSRules is used for SVGdocuments, i think here to test the function we assumed that one document is like anohter.
getFabricDocument return the document that holds the javascript from fabricjs.
We need a generic document, if we have a generic api that works in both cases like new Document() or new XLMDocument() (since is svg) we should prefer that.

I guess this is then something for a separate PR as a todo?

What i meant is, without having to create it from the fabric document itself.
Is there any global we can invoke that give us a new document, empty?

we can do the same thing with document.implementation.createHTMLDocument('');

yes i just cheked document.implementation is just a normal browser thing, i just never used it.

Smrtnyk · 2025-04-21T16:28:20Z

src/util/internals/getRandomInt.test.ts


 describe('getRandomInt', () => {
  beforeEach(() => {
-    global.Math.random = vi.fn(() => 0.1);
+    globalThis.Math.random = vi.fn(() => 0.1);


switched to globalThis for interop between jsdom and browser

test/snapshot-serializers/canvas-rendering-context.ts

Smrtnyk · 2025-04-21T16:29:20Z

vitest.env.d.ts

@@ -0,0 +1,14 @@
+declare module '*.gif' {


this is needed for imports of images to be recognized when we import them for browser mode tests

vitest.extend.ts

Smrtnyk · 2025-04-21T16:33:17Z

if you end up wanting this @asturur you will need to write new workflow that executes "test:vitest:chromium" script
but will need to run npx playwright install before to fetch the chromium

additionally, if you adopt this and decide to have it, I can work on adding firefox and webkit as well

vitest.config.ts

vitest.extend.ts

asturur · 2025-04-21T19:36:56Z

For sure this is the natural extension of what we had before.
And i think is good to have, or at least to try if it maintainable.

In the long run this may seems unnecessary because if unit tests are written by the book, you mock most o the things and test only your own code.

Is a bit different for visual tests in which you really want to test the output of the drawing functions.

As a maintainer doing this extra loops of tests keep me informed of different rounding methods between browser and node, difference in api that may happen between node-canvas and browser, in general if not extreme added maintainance i prefer to keep doing it.

I left a bunch of notes that do not preclude a merge, please see what you think about them

asturur · 2025-04-21T19:40:06Z

An additional improvement that can be done another time is that we export from fixtures functions that return the URL instead of returning the different url each time we need the image in a different file.
This will be more evident with visual tests.

Smrtnyk · 2025-04-21T19:42:26Z

For sure this is the natural extension of what we had before. And i think is good to have, or at least to try if it maintainable.

In the long run this may seems unnecessary because if unit tests are written by the book, you mock most o the things and test only your own code.

Is a bit different for visual tests in which you really want to test the output of the drawing functions.

As a maintainer doing this extra loops of tests keep me informed of different rounding methods between browser and node, difference in api that may happen between node-canvas and browser, in general if not extreme added maintainance i prefer to keep doing it.

I left a bunch of notes that do not preclude a merge, please see what you think about them

I agree with you with the concept of unit tests.

I noticed when I ran these same tests in webkit I already saw some numeric misalignement in few snapshots due to some precision, which showed difference between jsdom/chromium/webkit, not a lot but few tests did appear.
Also in firefox kerning was 13 in one snapshot where in jsdom, webkit and chromium is 12.

This does add maintenance burden a bit, but on other hand I see chromium more valuable source of testing reliability than jsdom. Chromium is the most used browser, and I assume most users of fabric are using it in browser and not in node env.

My point of this PR was done to demonstrate that vitest has excellent capabilities to run tests in browser as well and very fast and in an easy way.

If you decide to only stick with jsdom we can keep this out for now, and revive in the future if you decide to actually incorporate it, but maybe we could give a try for jsdom and chromium for now?

asturur · 2025-04-22T07:39:40Z

We do want to run tests in browsers. this is good

asturur · 2025-04-22T07:45:33Z

i ll try to add it to an action now

Smrtnyk · 2025-04-22T07:55:55Z

pls also let me know if I should work on webkit and firefox tests next?
they have some discrepancies so will need some minor adjustments as well to have it all stable across all browsers

Smrtnyk commented Apr 21, 2025

View reviewed changes

src/canvas/StaticCanvas.spec.ts Show resolved Hide resolved

Smrtnyk commented Apr 21, 2025

View reviewed changes

src/canvas/__tests__/__snapshots__/eventData.test.ts.snap Outdated Show resolved Hide resolved

Smrtnyk commented Apr 21, 2025

View reviewed changes

test/snapshot-serializers/canvas-rendering-context.ts Outdated Show resolved Hide resolved

Smrtnyk commented Apr 21, 2025

View reviewed changes

vitest.extend.ts Show resolved Hide resolved

Smrtnyk closed this Apr 21, 2025

Smrtnyk reopened this Apr 21, 2025

asturur reviewed Apr 21, 2025

View reviewed changes

vitest.config.ts Outdated Show resolved Hide resolved

asturur reviewed Apr 21, 2025

View reviewed changes

vitest.extend.ts Outdated Show resolved Hide resolved

Smrtnyk and others added 4 commits April 21, 2025 21:36

feat(tests): add chromium browser mode for unit tests

fe36db4

update CHANGELOG.md

4252645

adjust snapshot serializer for canvas ctx

51b74cd

pr feedback

46517f5

Smrtnyk force-pushed the feat(tests)-browser-mode branch from dffcfbd to 46517f5 Compare April 21, 2025 19:36

fix filter

56d64bd

use globalThis.document to get clean document

774e501

asturur approved these changes Apr 22, 2025

View reviewed changes

asturur merged commit 85b7551 into fabricjs:master Apr 22, 2025
15 checks passed

Smrtnyk deleted the feat(tests)-browser-mode branch April 22, 2025 07:55

Uh oh!

feat(tests): add chromium browser mode for unit tests #10568

feat(tests): add chromium browser mode for unit tests #10568

Uh oh!

Conversation

Smrtnyk commented Apr 21, 2025

Uh oh!

codesandbox bot commented Apr 21, 2025

Review or Edit in CodeSandbox

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Smrtnyk commented Apr 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asturur commented Apr 21, 2025

Uh oh!

asturur commented Apr 21, 2025

Uh oh!

Smrtnyk commented Apr 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asturur commented Apr 22, 2025

Uh oh!

asturur commented Apr 22, 2025

Uh oh!

Uh oh!

Smrtnyk commented Apr 22, 2025

Uh oh!

Uh oh!

Smrtnyk commented Apr 21, 2025 •

edited

Loading

Smrtnyk commented Apr 21, 2025 •

edited

Loading