ci(): Migrate jest to vitest

[Smrtnyk]

proposed here #10417 (comment)

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

[Smrtnyk]

I feel like this ceremony with jest-snapshot package could be circumvented with https://vitest.dev/guide/snapshot#custom-serializer
but I didn't dig deep what exactly this is all about
i assume for more simple snapshots or something

I will leave it for some other time

[asturur]

I think is to add 'toMatchRoundedMatrix' maybe we can avoid to use that custom matcher or just have a function in that case that does what it has to do

[asturur]

My first impression is that the api changes are minimal ( just mock checks a little bit ).
I ll have a read of how vitest runs in a browser and see if is worth to have some of those running in a browser too.

[asturur]

those needs to stay in a separate PR, could also be the reason why your build is failing

[Smrtnyk]

those needs to stay in a separate PR, could also be the reason why your build is failing

build failed most probably because I removed @types/jsdom
https://github.com/fabricjs/fabric.js/pull/10420/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519L87
because I thought those were only used for tests, but they are actually needed for building the prod code
I will bring those types back
Regarding dropping the jsdom update here I am not sure about that, I reverted the old jsdom version and vitest tests didn't work
seems like it relies on newer jsdom
That being said, vitest and this PR rely on node at least being 18 and newer jsdom, so maybe this PR should land after your PR
#10417
And then we just rebase this PR when it is merged

After we land this (if we land this) I can help you migrate some of those QUnit tests to vitest
I am not that good with maths so can't really help you with fabric src code, but I have a bit of experience with testing and tooling so I can help you with that

[Smrtnyk]

My first impression is that the api changes are minimal ( just mock checks a little bit ). I ll have a read of how vitest runs in a browser and see if is worth to have some of those running in a browser too.

I am using browser mode heavily in my vue project, because I can test my components in real environment with real rendering and screenshots.
The only downside is that you can't use breakpoints and debugger from node, you have to start the ui and headed browser then in the ui put a breakpoint in the browser instance which is a bit cumbersome

[Smrtnyk]

@asturur could you please rerun the build? to see if it is fixed now

[asturur]

Sorry i have been out, just to be clear i m insterested in this PR

[asturur]

on my local, mac m1 arm64:

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 21 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  src/brushes/CircleBrush.test.ts > CircleBrush > can be initialized
 FAIL  src/canvas/SelectableCanvas.spec.ts > Selectable Canvas > _pointIsInObjectSelectionArea > points and selection area
 FAIL  src/controls/Control.spec.ts > Controls > method binding
 FAIL  src/controls/pathControl.spec.ts > pathControls > should fire events
 FAIL  src/controls/polyControl.spec.ts > polyControl > should fire events
 FAIL  src/controls/scale.test.ts > Scale > adjusts a 0 width rect for polyActionhandler without it returning Infinity/NaN side scale
 FAIL  src/shapes/ActiveSelection.spec.ts > ActiveSelection > `setActiveObject` should update the active selection ref on canvas if it changed
 FAIL  src/shapes/Group.spec.ts > Group > adding and removing an object
 FAIL  src/util/typeAssertions.spec.ts > typeAssertions > isTextObject > can detect FabricText
 FAIL  src/canvas/__tests__/SelectableCanvas.spec.ts > Canvas > invalidating `_objectsToRender` > initial state
 FAIL  src/shapes/Text/StyledText.spec.ts > setSelectionStyles > will set properties at the correct position
 FAIL  src/shapes/Text/TextSVGExportMixin.spec.ts > TextSvgExport > exports text background color correctly
Error: Test timed out in 5000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/21]⎯

 FAIL  src/canvas/StaticCanvas.spec.ts > StaticCanvas > toBlob
Error: Test timed out in 5000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/21]⎯

 FAIL  src/controls/changeWidth.test.ts > changeWidth > changeWidth changes the width
Error: Hook timed out in 10000ms.
If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[3/21]⎯

 FAIL  src/shapes/Textbox.spec.ts > Textbox > fromObject
 FAIL  src/shapes/IText/IText.test.ts > IText > cursor drawing width > group scaled by 1 and rotated by +0 , text scaled by 1 and rotated by +0, and canvas zoomed by 1
 FAIL  src/shapes/IText/ITextBehavior.test.ts > text imperative changes > removeChars
 FAIL  src/shapes/Object/InteractiveObject.spec.ts > InteractiveObject > setCoords for objects inside group with rotation > all corners are rotated as much as the object total angle
 FAIL  src/shapes/Text/Text.spec.ts > FabricText > toObject
Error: Test timed out in 5000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[4/21]⎯

 FAIL  src/canvas/__tests__/eventData.test.ts > Canvas event data > HTML event "mousedown" should fire a corresponding canvas event
Error: Hook timed out in 10000ms.
If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[5/21]⎯

 FAIL  src/shapes/IText/ITextKeyBehavior.test.ts > IText move cursor > selection changes > enterEditing does not use delayedCursor
Error: Hook timed out in 10000ms.
If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[6/21]⎯


 Test Files  21 failed | 30 passed (51)
      Tests  21 failed | 435 passed | 2 skipped (458)
   Start at  21:42:47
   Duration  53.51s (transform 962ms, setup 10.03s, collect 27.19s, tests 340.16s, environment 13.88s, prepare 2.08s)

[Smrtnyk]

Hm weird, works perfectly fine for me on linux

[asturur]

I think on the mac i have to move the pool from forks to thread.
Had to read the docs to figure out

[Smrtnyk]

I think on the mac i have to move the pool from forks to thread. Had to read the docs to figure out

Interesting
Is ci running on mac as well or only on linux?

I believe some tests need snapshot updates now

[asturur]

ok so for some reason vi.spyOn does not behave as 'jest.spyOn` and as a result i had to modify the test about eventData and the relative snapshot now makes more sense.
In that test a couple of fabric objects are serialized as 'drop target' or 'target' to simplify the snap, and this wasn't working anymore, hence the version issue.

[asturur]

I m really undecided it to use vitest globals or add the imports each test file.
It would reduce the diffs of the PR by a lot, but there isn't a way to customize the test spearator in the snapshots.

[asturur]

Also somehow we have a speed penality running vitest from 22s to 32s
I don't care for 10s extra, but i wonder where is the vitest speed for us 🤷

[Smrtnyk]

I m really undecided it to use vitest globals or add the imports each test file. It would reduce the diffs of the PR by a lot, but there isn't a way to customize the test spearator in the snapshots.

I usually avoid involving global types if I can because it can pull in some unwanted types in files that don't need it
Having imported only what I need is what I prefer, but that is just my personal preference

[Smrtnyk]

Also somehow we have a speed penality running vitest from 22s to 32s I don't care for 10s extra, but i wonder where is the vitest speed for us 🤷

I think that is some regression from vitest v2 to vitest v3
I noticed some other people complaining already
there were a few more complaints like these vitest-dev/vitest#7488
So I expect it to be fixed eventually, for us I think 10s is not the reason to downgrade to v2 vitest

[Smrtnyk]

ok so for some reason vi.spyOn does not behave as 'jest.spyOn` and as a result i had to modify the test about eventData and the relative snapshot now makes more sense. In that test a couple of fabric objects are serialized as 'drop target' or 'target' to simplify the snap, and this wasn't working anymore, hence the version issue.

regarding spy.on they are doing some changes to it
vitest-dev/vitest#7499
but lets see how that will affect us

[asturur]

I m really undecided it to use vitest globals or add the imports each test file. It would reduce the diffs of the PR by a lot, but there isn't a way to customize the test spearator in the snapshots.

I usually avoid involving global types if I can because it can pull in some unwanted types in files that don't need it Having imported only what I need is what I prefer, but that is just my personal preference

Yes i think globals are not a good idea, is ok to import vitest when you want to test.

I'll try to fix a couple of outstanding issues i want to put in, then i ll merge the jsdom/canvas3 pr and remove node 16. That will be a very simple 7.0 upgrade, so i don't have to rush building versioned docs on the website

[Smrtnyk]

there is 3.1.1 released with lots of fixes
maybe we should jump on that train as well

[Smrtnyk]

kinda unrelated but would you be open to add something like https://github.com/stackblitz-labs/pkg.pr.new
if you activate this on PR it would be very easy for community to try PR fixes/features out from pull requests without doing some acrobatics or waiting for new release to confirm the status of the PR

[asturur]

I think we are good.
What i can't see is how vitest interact with JSDOM and CANVAS and kind of bothers me.
Do you know how that works?

[Smrtnyk]

I think we are good. What i can't see is how vitest interact with JSDOM and CANVAS and kind of bothers me. Do you know how that works?

we declared jsdom as environment in vitest config
https://github.com/fabricjs/fabric.js/pull/10420/files#diff-2ee894bf23aa44ff4ce12a3da8af19ab20180474ebf2176dbe5f2eea3f96dc92R8
so it uses installed one
same for jsdom
it declares canvas as peer dep
https://github.com/jsdom/jsdom/blob/main/package.json#L48
so it uses the installed one as well
which we both have

[Smrtnyk]

btw, after this is done and merged, I will try to help with migration of some unit tests from qunit to vitest
maybe over the weekend when I have more free time

[asturur]

btw, after this is done and merged, I will try to help with migration of some unit tests from qunit to vitest maybe over the weekend when I have more free time

That would be great. I want to migrate the visuals test to playwright