golevelup / nestjs

A collection of badass modules and utilities to help you level up your NestJS applications 🚀
MIT License
2.16k stars 244 forks source link

feat(ts-vitest): added a new package to support vitest for testing utils #702

Closed jderochervlk closed 4 months ago

jderochervlk commented 4 months ago

Copied over the ts-jest files and adapted them to work with Vitest, so now you can use these testing utils with Vitest.

fix #613

underfisk commented 4 months ago

@jderochervlk It looks like for CI 16 (for example) vitest is expecting a higher node engine version. We should accept some lower verion/range to keep support for those until we fully deprecate support for node 16

jderochervlk commented 4 months ago

@jderochervlk It looks like for CI 16 (for example) vitest is expecting a higher node engine version. We should accept some lower verion/range to keep support for those until we fully deprecate support for node 16

Let me take a look.

jderochervlk commented 4 months ago

It looks like this has to have a minimum node version of 18 unless we roll back to the vitest pre-release, which is probably not a good idea @underfisk

https://github.com/vitest-dev/vitest/releases/tag/v1.0.0

It also looks like 18 and 19 hit an issue with SyntaxError: Unexpected token 'export, which is coming from all tests being run with Jest.

underfisk commented 4 months ago

https://github.com/vitest-dev/vitest/releases/tag/v1.0.0

I know.. I really want to get rid of node 16 which i have a PR here but it would require @WonderPanda merge's power.

jderochervlk commented 4 months ago

Fixed the test:ci command to have Jest exclude ts-vitest and also add vitest to run with only the new package.

jderochervlk commented 4 months ago

Ah, I messed up some deps. It should be good now to have 18/19 pass.

underfisk commented 4 months ago

Ah, I messed up some deps. It should be good now to have 18/19 pass.

Still failing, for Node 19 it doesn't seem to support for some reason

jderochervlk commented 4 months ago

Ah, I messed up some deps. It should be good now to have 18/19 pass.

Still failing, for Node 19 it doesn't seem to support for some reason

Vitest seems to suport only 18 and 20. It should be fine to figure out how to get the CI to pass since anyone who would install this package would only be on a version of node that Vitest supports. We might be able to tweak it to only run the "vitest" command when on a supported version and run Jest on all versions.

underfisk commented 4 months ago

Ah, I messed up some deps. It should be good now to have 18/19 pass.

Still failing, for Node 19 it doesn't seem to support for some reason

Vitest seems to suport only 18 and 20. It should be fine to figure out how to get the CI to pass since anyone who would install this package would only be on a version of node that Vitest supports. We might be able to tweak it to only run the "vitest" command when on a supported version and run Jest on all versions.

I agree with you, if you want to make those adjustments just update the package json and instead of having the test script to run both jest and vitest, you expose a test:vitest command. That way we can integrate in CI for the supported node version(s)

jderochervlk commented 4 months ago

@underfisk Ok, I split out the vitest commands and added a CI step that only runs on Node 18 https://github.com/golevelup/nestjs/pull/702/commits/5b1b22fb92317cf77e50e9f3bd0a4b6309f9b41e

underfisk commented 4 months ago

@underfisk Ok, I split out the vitest commands and added a CI step that only runs on Node 18 5b1b22f

We may have to deal with the package installation tho, it seems that CI attempts to also install vitest in 16/19 and it just fails.

jderochervlk commented 4 months ago

@underfisk Ok, I split out the vitest commands and added a CI step that only runs on Node 18 5b1b22f

We may have to deal with the package installation tho, it seems that CI attempts to also install vitest in 16/19 and it just fails.

Oh that's fun!

jderochervlk commented 4 months ago

I moved vitest to optionalDependencies and tested that it works locally on Node 16 and 18.

underfisk commented 4 months ago

Details

now build fails 🤣

jderochervlk commented 4 months ago

I did the same thing to build as tests where it will only run on Node 18 now.

underfisk commented 4 months ago

Details

I think the argument given to CI is not valid, you can see the logs

jderochervlk commented 4 months ago

Details

I think the argument given to CI is not valid, you can see the logs

It should be fixed now 🤞

underfisk commented 4 months ago

@jderochervlk What a nice adventure!! Once again thank you for your contribution!!

jderochervlk commented 4 months ago

@jderochervlk What a nice adventure!! Once again thank you for your contribution!!

Open source is always fun!

underfisk commented 4 months ago

once @WonderPanda releases a new version and publishes the package, we can notify/close the other issue

WonderPanda commented 4 months ago

Hey @jderochervlk thanks for this contribution! I just pulled this down to see about publishing to NPM and it looks like the build command for @golevelup/ts-vitest has been ignored so there aren't going to be any assets to publish:

    "build": " lerna run build --ignore rabbitmq-integration --ignore @golevelup/ts-vitest",

Is this intentional or can it be removed now?

jderochervlk commented 4 months ago

It was ignored since it fails with node 16 and 19. There is a build:ts-vitest command to build it. On Feb 27, 2024, 10:44 AM -0500, Jesse Carter @.***>, wrote:

Hey @jderochervlk thanks for this contribution! I just pulled this down to see about publishing to NPM and it looks like the build command for @golevelup/ts-vitest has been ignored so there aren't going to be any assets to publish: "build": " lerna run build --ignore rabbitmq-integration --ignore @golevelup/ts-vitest", Is this intentional or can it be removed now? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

WonderPanda commented 4 months ago

It was ignored since it fails with node 16 and 19. There is a build:ts-vitest command to build it. … On Feb 27, 2024, 10:44 AM -0500, Jesse Carter @.>, wrote: Hey @jderochervlk thanks for this contribution! I just pulled this down to see about publishing to NPM and it looks like the build command for @golevelup/ts-vitest has been ignored so there aren't going to be any assets to publish: "build": " lerna run build --ignore rabbitmq-integration --ignore @golevelup/ts-vitest", Is this intentional or can it be removed now? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.>

Hmmm that's going to make publishing a little trickier. I use lerna:publish in the root package.json which expects to be able to build all the packages so that their assets are available to be versioned and pushed to NPM

jderochervlk commented 4 months ago

It should work ok. It's only ignored build, not in lerna:publish.

I just tried it locally and it was included in the build libraries.

lerna success run Ran npm script 'build' in 12 packages in 4.6s:
lerna success - rabbitmq-integration
lerna success - @golevelup/nestjs-common
lerna success - @golevelup/nestjs-discovery
lerna success - @golevelup/nestjs-graphql-request
lerna success - @golevelup/nestjs-hasura
lerna success - @golevelup/nestjs-modules
lerna success - @golevelup/nestjs-rabbitmq
lerna success - @golevelup/nestjs-stripe
lerna success - @golevelup/nestjs-webhooks
lerna success - @golevelup/ts-jest
lerna success - @golevelup/ts-sinon
lerna success - @golevelup/ts-vitest
jderochervlk commented 4 months ago

Is there anything else I can do to help with getting this released?

ej-shafran commented 4 months ago

Is there a plan to have this released?