Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(vitest): replace jest with vitest #1404

Merged

Conversation

ThatOneBro
Copy link
Contributor

@ThatOneBro ThatOneBro commented Sep 4, 2023

Author should do the followings, if applicable

  • Add tests
  • Run tests
  • yarn denoify to generate files for Deno

What this PR does

Replaces jest with vitest and cleans up dependencies.

I did notice one failing test in lagon suite, but I think it's unrelated to this PR. EDIT: Tests not failing on CI, so seems ok.

Closes #1390.

TODO

EDIT: One last test failing on Fastly Compute@Edge...

  • Get last test running on Fastly

@ThatOneBro
Copy link
Contributor Author

Currently the fastly suite is failing on the test that is supposed to throw crypto.subtle.importKey in undefined but is instead throwing crypto is not defined... I found I got this same error locally when I was testing in node v18, but on v19 the test passes...

Node v18:
Screen Shot 2023-09-03 at 10 39 37 PM

Node v19:
Screen Shot 2023-09-03 at 10 39 51 PM

@yusukebe
Copy link
Member

yusukebe commented Sep 4, 2023

@ThatOneBro

Great work! I've tested this in my local machine, it's fast!

I'll take a look about the Fastly issue. Thanks!

@yusukebe
Copy link
Member

yusukebe commented Sep 7, 2023

Hi @ThatOneBro,

I think reproducing the runtime environment is important. As of now, we've successfully emulated the Fastly environment with jest-preset-fastly-js-compute. Considering this, how about we remain the Jest test for Fastly? Of course, we replace other tests with Vitest. This might be a better solution, while we're currently juggling two testing engines: Jest and Vitest.

What do you think about this?

@ThatOneBro
Copy link
Contributor Author

Hey @yusukebe, that makes sense to me. I'll make that change tonight so we can merge this. Keeping jest for Fastly until there is a vitest alternative can work

@yusukebe
Copy link
Member

yusukebe commented Sep 7, 2023

@ThatOneBro

Thanks!

@ThatOneBro
Copy link
Contributor Author

No problem, @yusukebe! I added jest back for the fastly tests and looks like everything works now! Let me know if there is anything I missed, otherwise feel free to merge!

@yusukebe
Copy link
Member

yusukebe commented Sep 8, 2023

Hi @ThatOneBro !

Perfect! Let's go with it. Thanks Derrick!

@yusukebe yusukebe merged commit 5b07146 into honojs:main Sep 8, 2023
10 checks passed
yusukebe added a commit to CyberFlameGO/hono that referenced this pull request Sep 16, 2023
* test(vitest): replace `jest` with `vitest` for core tests

* test(fastly): `jest` -> `vitest`, correct env for fastly

* test(jest): remove `jest-environment-miniflare` as dep

* test(lagon): configure `lagon` tests to run on `vitest`

* test(lambda): `jest` -> `vitest`

* test(lambda-edge): `jest` -> `vitest`

* test(node): `jest` -> `vitest`

* test(wrangler): `jest` -> `vitest`

* chore(deps): remove `jest` and `ts-jest` from deps

* test(vitest): add `yarn coverage` for checking coverage

* test(fastly): fix check for `globalThis.crypto`

* test(handler): fix stray `.only`

* test(lagon): change env file back to original path

* test(fastly): go back to `jest` until `vitest` has support for fastly env

* test(fastly): remove hack for the `crypto` global from the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Vitest
2 participants