sentry-javascript
sentry-javascript copied to clipboard
Migrate from jest to vitest and stop using jsdom based tests
After v8 gets merged in, let's look at migrating from jest to vitest. vitest is way faster, and doesn't fall into the same traps jest does in terms of esm compatibility. Example attempt with Vue SDK: https://github.com/getsentry/sentry-javascript/pull/11071
In addition, we should stop using jsdom based tests, and instead move tests that rely on the browser to use playwright instead. Simulating jsdom in an node environment always has it's faults, easier to not attempt to do that.
First we should align on a common vitest testing standard. Right now we use set globals via vitest/globals, but this is not recommended by vitest themselves. We should instead use direct imports like import { describe, test, expect } from 'vitest';.
https://github.com/getsentry/sentry-javascript/blob/780875fd9d13ef6be5f7f894b97c48777022f5e9/tsconfig.dev.json#L8
There's probably some other discussion that needs to be done here too. We can validate our decisions by changing our vitest usage in the Astro and SvelteKit SDKs.
### Update existing vitest usage
- [ ] https://github.com/getsentry/sentry-javascript/pull/13093
- [ ] https://github.com/getsentry/sentry-javascript/pull/13214
- [ ] Nuxt SDK
- [ ] https://github.com/getsentry/sentry-javascript/pull/13028
- [ ] SolidStart SDK
- [ ] https://github.com/getsentry/sentry-javascript/pull/13026
- [ ] https://github.com/getsentry/sentry-javascript/pull/12960
- [ ] https://github.com/getsentry/sentry-javascript/pull/13027
We can then tackle the following list in whatever order we want!
### Convert to using vitest
- [x] Angular https://github.com/getsentry/sentry-javascript/pull/11091
- [ ] https://github.com/getsentry/sentry-javascript/pull/13092
- [ ] https://github.com/getsentry/sentry-javascript/issues/13434
- [ ] AWS Serverless
- [ ] Browser Utils
- [ ] Feedback
- [ ] Gatsby
- [ ] Google Cloud Serverless
- [ ] Next.js
- [ ] Node
- [ ] OpenTelemetry
- [ ] Profiling Node
- [ ] React
- [ ] Remix
- [ ] https://github.com/getsentry/sentry-javascript/pull/12964
- [ ] https://github.com/getsentry/sentry-javascript/pull/11899
- [ ] https://github.com/getsentry/sentry-javascript/pull/12961
- [ ] Utils - first attempt: https://github.com/getsentry/sentry-javascript/pull/12959
- [ ] https://github.com/getsentry/sentry-javascript/pull/12958
- [ ] https://github.com/getsentry/sentry-javascript/pull/12955
- [x] Wasm
- [ ] Node Integration Tests
We also have some packages that use mocha. We should try to drop that dependency and make them use vitest
### Remove mocha
- [ ] https://github.com/getsentry/sentry-javascript/pull/11296
- [ ] https://github.com/getsentry/sentry-javascript/pull/11412
- [ ] https://github.com/getsentry/sentry-javascript/pull/11436
- [ ] https://github.com/getsentry/sentry-javascript/pull/11449
- [ ] https://github.com/getsentry/sentry-javascript/pull/11666
- [ ] https://github.com/getsentry/sentry-javascript/pull/11688
- [ ] https://github.com/getsentry/sentry-javascript/pull/11733
- [ ] https://github.com/getsentry/sentry-javascript/pull/11758
ref https://github.com/getsentry/sentry-javascript/issues/6040
vistest was already added to Angular here: https://github.com/getsentry/sentry-javascript/pull/11091
I will check it on the list 👍🏻
@AbhiPrasad Replay was migrated here: https://github.com/getsentry/sentry-javascript/pull/11899
edit though we are using globals and not importing everything from vitest
Looks like the task list needs updating because Node v14 doesn't support vitest?
I updated it - looks like we are stuck with jest longer than I thought.
Almost everything is now migrated to Vitest.
There are a couple of packages where I've created branches to migrate them but got stuck on some blockers:
@sentry/remix
Branch - https://github.com/getsentry/sentry-javascript/tree/timfish/test/vitest-remix
upload-sourcemaps.test.tstests the the files in./scripts/**which are all CJS and Vitest cannot not mock require so they can't be tested directly with Vitest
Maybe these files should be in TypeScript and transpiled to the output directory anyway?
@sentry/google-cloud-serverless
Branch - https://github.com/getsentry/sentry-javascript/tree/timfish/test/vitest-google
google-cloud-grpc.test.tstest fails withUNAUTHENTICATED: Request had invalid authentication credentials- Looks like dns isn't getting mocked because it hits Google and gets an auth error?
@sentry/nextjs
Branch - https://github.com/getsentry/sentry-javascript/tree/timfish/test/vitest-nextjs
constructWebpackConfig.test.tsfailures because mocks don't appear to be working and other strange behaviour
upload-sourcemaps.test.ts tests the the files in ./scripts/** which are all CJS and Vitest cannot not mock require so they can't be tested directly with Vitest
maybe you can try the hack I added for gatsby that patches Module.load to do CJS patching: https://github.com/getsentry/sentry-javascript/blob/ba4586f3251cb9a6b5746781d502c388035d962b/packages/gatsby/test/gatsby-node.test.ts#L3-L28
Looks like dns isn't getting mocked because it hits Google and gets an auth error?
These tests are so hard to debug 😬
I'd be open to try to just completely rewrite them.
constructWebpackConfig.test.ts failures because mocks don't appear to be working and other strange behaviour
let's open a draft PR for this one, we can ask Luca and Charly to help take a look given they have more experience with these tests.
I took another look at @sentry/google-cloud-serverless and I think we have two options
- test using the gcp simulators, and switch to using node integration tests + docker images
- mock out the PubSub object entirely:
I asked Claude to give me a mock and here's what it provided:
// Mock the entire PubSub module
vi.mock('@google-cloud/pubsub', () => {
return {
PubSub: vi.fn().mockImplementation(() => ({
topic: vi.fn().mockImplementation(topicName => ({
publish: vi.fn().mockImplementation(() => {
return Promise.resolve('1637084156623860');
}),
publishMessage: vi.fn().mockImplementation(() => {
return Promise.resolve('1637084156623860');
}),
name: `projects/project-id/topics/${topicName}`,
})),
close: vi.fn().mockResolvedValue(undefined),
})),
};
});
Our instrumentation hooks the google-gax module. If you mock out the PubSub object, it'll never hit any of the google-gax code and the instrumentation does not get tested.
I'm OOO in an hour so I've opened a PR with everything migrated apart from this test which is still failing:
- https://github.com/getsentry/sentry-javascript/pull/15567
We should write a engineering blog post about this https://sentry.engineering/