test(svelte): Use vitest instead of jest #10350
Conversation
JonathonRP
changed the title
There was a problem hiding this comment.
Hi @JonathonRP thanks for helping out!
The changes look reasonable to me. CI is not yet happy so I'd recommend running
yarn buildyarn testyarn lint
locally within the svelte package.
thanks, I wasn't sure how to run now I know, running and trying to resolve issues as I go |
|
@Lms24, running tests it looks like tests fail because assumptions about how the component name should be populated has changes. |
There was a problem hiding this comment.
Thanks for making the changes! I just saw that you're targeting my PR instead of develop. I'd like to keep these changes atomic though (I'd argue upgrading to vitest is a good change regardless of what we do with #10312). if you could change the target to develop we can merge in this change right now. This should also fix the svelte test fails for now.
JonathonRP
changed the base branch from
lms/ref-svelte-component-tracking-svelte-5
to
develop
JonathonRP
force-pushed
the
lms/ref-sentry-svelte-vitest
branch
from
d547177 to
aaee6fb
Compare
|
@Lms24 branch is changed to develop |
There was a problem hiding this comment.
Hmm looks like there's still an error with the vitest config: https://github.com/getsentry/sentry-javascript/actions/runs/7679861958/job/20960439408?pr=10350#step:5:869
Does running yarn test work locally for you?
Also, to fix the lint error, please rebase your PR to the latest develop changes. Unfortunately a formatting error made it to develop, causing the lint job to fail.
JonathonRP
force-pushed
the
lms/ref-sentry-svelte-vitest
branch
from
aaee6fb to
18545b6
Compare
|
@Lms24 I worked to get the tests to pass as much as I can, I'll need further help if more work is needed to pass more tests |
|
Ah I see, this might require more adjustments. I'll take a look when I have a minute. |
|
@Lms24 I see that 8.0.0 is getting pretty close, and hoping this gets addressed soon. |
|
We're still a couple of weeks away from a stable 8.0.0 release but yes it's getting closer. Had a brief look today and it seems like svelte testing library isn't correctly set up for vitest. The conponent lifecycle hooks aren't called as a result and hence the tests fail. |
Thank you, I'll take a look and see if I can solve it properly tomorrow |
JonathonRP
force-pushed
the
lms/ref-sentry-svelte-vitest
branch
from
c78a283 to
074953b
Compare
|
@Lms24 I took sometime and figured out how to get svelte lifecycle events firing again with vitest, it's an svelte4 and svelte5 issue with vitest. |
JonathonRP
force-pushed
the
lms/ref-sentry-svelte-vitest
branch
from
d21d594 to
88b39bf
Compare
There was a problem hiding this comment.
Awesome, thanks a lot for sticking with us @JonathonRP - Looks good to me now. Will merge soon!
Lms24
changed the title
|
@Lms24 just making sure this isn't forgotten. |
To not run in watch mode we need to specifically run `vitest run` instead of `vitest` (missed this when reviewing #10350). - `yarn test` will run the test suites once - `yarn test:watch` will continue to run the tests in watch mode Also unstaled `yarn.lock` which we also seem to have missed in #10350
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).