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

Migrate Testing from Jest to Vitest (#27) #31

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ThomasKaar
Copy link

@ThomasKaar ThomasKaar commented Jul 16, 2024

Migrate Testing from Jest to Vitest (#27)

♻️ Current situation & Problem

As described in #27, vitest is a more modern and faster testing framework.

⚙️ Release Notes

  • Added vitest
  • Removed jest

📚 Documentation

This Pull Request migrates Jest to Vitest according to the official vitest documentation and the Next.js testing guide.
The packages:

  • @types/jest
  • jest
  • jest-environment-jsdom
  • ts-jest

have been removed

✅ Testing

Before

$ npm run test
...
 PASS  app/page.test.tsx
  Home Component
    ✓ renders the Stanford Biodesign Digital Health Next.js Template heading (92 ms)
    ✓ renders the Stanford Biodesign Logo (6 ms)

-------------------------------|---------|----------|---------|---------|-------------------
File                           | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-------------------------------|---------|----------|---------|---------|-------------------
All files                      |     100 |       75 |     100 |     100 |                   
 app                           |     100 |       75 |     100 |     100 |                   
  page.tsx                     |     100 |       75 |     100 |     100 | 19                
 packages/example-package/dist |     100 |      100 |     100 |     100 |                   
  index.js                     |     100 |      100 |     100 |     100 |                   
-------------------------------|---------|----------|---------|---------|-------------------
Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        3.294 s
Ran all test suites.

After

$ npm run test
...
 ✓ app/page.test.tsx (2)
   ✓ Home Component (2)
     ✓ renders the Stanford Biodesign Digital Health Next.js Template heading
     ✓ renders the Stanford Biodesign Logo

 Test Files  1 passed (1)
      Tests  2 passed (2)
   Start at  21:47:52
   Duration  1.77s (transform 86ms, setup 0ms, collect 385ms, tests 103ms, environment 683ms, prepare 100ms)

 % Coverage report from v8
------------------------------|---------|----------|---------|---------|-------------------
File                          | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
------------------------------|---------|----------|---------|---------|-------------------
All files                     |   74.46 |    66.66 |   66.66 |   74.46 |                   
 app                          |      60 |       50 |      50 |      60 |                   
  layout.tsx                  |       0 |        0 |       0 |       0 | 1-28              
  page.tsx                    |     100 |      100 |     100 |     100 |                   
 packages/example-package/src |     100 |      100 |     100 |     100 |                   
  index.ts                    |     100 |      100 |     100 |     100 |                   
------------------------------|---------|----------|---------|---------|-------------------

Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@PSchmiedmayer
Copy link
Member

@ThomasKaar Thank you for the addition and taking a look at the issue!
I have requested a detailed review from @arkadiuszbachorski.

Looking at the PR at a high level, it seems like the coverage report you detail above is also including js files and reports a 0% test coverage for the TS files wich might not be correct.
It might make sense to take a look at some of the chances and mechanisms we made in https://github.com/StanfordBDHG/ENGAGE-HF-Web-Frontend which seems to properly report some TS test coverage despite some elements that we could improve there as well.

remove generated coverage files from eslint
add licensing to vite.config.mts
@ThomasKaar
Copy link
Author

Thanks for the brief review!

I reduced the scope of the coverage analysis similar to https://github.com/StanfordBDHG/ENGAGE-HF-Web-Frontend while also including js files with sourceMap enabled so the ts-file can be properly mapped to the js-file in the coverage analysis (I updated the test plan accordingly).

Moreover, I also added licensing information to the newly added vitest.config.mts and ran npm run lint:ci locally to ensure it is working properly

@PSchmiedmayer
Copy link
Member

Thank you @ThomasKaar; greatly appreciated!
Just started the builds and I can sync with @arkadiuszbachorski about the next steps 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants