ghiscoding / slickgrid-universal

Slickgrid-Universal is a monorepo which includes all Editors, Filters, Extensions, Services related to SlickGrid usage and is also Framework Agnostic
https://ghiscoding.github.io/slickgrid-universal/
Other
90 stars 29 forks source link

chore: migrate from Jest to Vitest for ESM support #1663

Closed ghiscoding closed 2 months ago

ghiscoding commented 2 months ago
stackblitz[bot] commented 2 months ago

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.7%. Comparing base (f6e4605) to head (ebcf4e8). Report is 21 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1663 +/- ## ======================================== - Coverage 99.7% 99.7% -0.0% ======================================== Files 199 187 -12 Lines 21895 31085 +9190 Branches 7319 9782 +2463 ======================================== + Hits 21827 30987 +9160 - Misses 68 98 +30 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ghiscoding commented 2 months ago

@zewa666 I'm curious, are you using JSDOM or happy-dom with Vitest? My current setup is a mix of both happy-dom and JSDOM, so my Vitest config currently has environment: 'happy-dom' and I also need jsdom-global. There currently 2 problems that I have

  1. if I change to environment: 'jsdom', it behaves weirdly, it sometimes execute the unit tests twice and sometimes doesn't complete
  2. if I try to remove all JSDOM related, just jsdom-globals now, it fails 31 of my test suites, I will probably have to rewrite a few unit tests since I assume happy-dom might be different compared to jsdom
    • I found @happy-dom/global-registrator which is maybe what I need to replace jsdom-globals. similar to this post, will try tonight after work

So it's a little weird, I have to go with a mix of both, I'll take another look at it tonight but so far the testing only works with the mix config... so are you using happy-dom and/or jsdom?

EDIT

as per this open issue but it looks like it's a known problem with Vite and happy-dom was suggested as an alternative. However I find happy-dom is still kind of incomplete (for example, DOM elements don't include aria labels and others, a couple more issues too)... so if the mix of both works, let's keep that and merge baby merge (1 step closer to ESM only)

zewa666 commented 2 months ago

I did not have that issue. besides are you sure jsdom is the problem since you already had it working with jest?

I personally would stay away from happydom due to even less coverage than jsdom, which shows itself especially with testing-library. but if it works for the slickgrid lib itself ... well why not ;)

ghiscoding commented 2 months ago

I wouldn't mind using strictly JSDOM but like I said above when I set it as the environment in Vitest, then some tests aren't passing but when I set it to happy-dom and still use jsdom-global then all is good... It's kinda weird but like you said, if it works then let's keep that setup and forget about it :)

https://github.com/ghiscoding/slickgrid-universal/blob/0bee20901b0d9745ccb1d4d9e398b672052c92ae/test/vitest-pretest.ts#L1-L2

zewa666 commented 2 months ago

would you mind creating a branch where only jsdom is acitvated? I'd really like to give it a closer look as it might also show some potential blockers for my own app

ghiscoding commented 2 months ago

it's just this line to change from happy-dom to jsdom

- environment: 'happy-dom',
+ environment: 'jsdom',

https://github.com/ghiscoding/slickgrid-universal/blob/0bee20901b0d9745ccb1d4d9e398b672052c92ae/test/vitest.config.mts#L3-L9

zewa666 commented 2 months ago

oh ok thought you mentioned adding additional stuff for happydom. I'll give it a try

zewa666 commented 2 months ago

@ghiscoding so I've tried switching to jsdom and all tests are passing nicely for me. I dunno, perhaps try to nuke the node_modules folder and a fresh install could be the difference?

ghiscoding commented 2 months ago

@zewa666 hmm that's strange because I tried on work laptop today and same results, I have 202 tests failing with this error

'removeEventListener' called on an object that is not a valid instance of EventTarget.

I didn't think of nuking the node_modules, so I just tried it and still the same result on my side. I think the problem is that I use Custom Events in the project and JSDOM doesn't work well with EventTarget properly with Custom Events because of this JDOM issue, I tried to mock it via this post from the same issue but that doesn't help. So I think that when I set happy-dom, its EventTarget mock is better and so when I mix them both then it passes.

image

it's also a lot quicker when it passes

85sec vs 171sec

image