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

chore: introduce vitests for tests #5269

Merged
merged 7 commits into from
Dec 6, 2022
Merged

chore: introduce vitests for tests #5269

merged 7 commits into from
Dec 6, 2022

Conversation

danez
Copy link
Contributor

@danez danez commented Nov 25, 2022

πŸŽ‰ Thanks for submitting a pull request! πŸŽ‰

Summary

This introduces vitests for test files with mjs extension.

For integration tests the CI uses sharding again to split up tests into multiple workers, right now with one test this is kind of useless, but will be important later. In PRs the flag --changed is used to only run tests that have changes.

vitest is clever enough to rerun all tests if package.json and shrinkwrap change. :)

The Jobs have also been renamed and are now called

  • Tests / E2E
  • Tests / Unit
  • Tests / Integration
  • Tests / Legacy Integration (which will be going away)

I was wondering what the all job is doing and found this: #4164 (comment)
So it was meant to be the only job required from this workflow, which isn't the case right now as all test jobs are required. I opted to remove the all job for that reason, but happy to restore.

Ref #5265


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code πŸ§‘β€πŸ’». This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire πŸ”₯ (e.g. incident related), you can skip this step.
  • Read the contribution guidelines πŸ“–. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) πŸ§ͺ
  • Update or add documentation (if features were changed or added) πŸ“
  • Make sure the status checks below are successful βœ…

A picture of a cute animal (not mandatory, but encouraged)

@danez danez self-assigned this Nov 25, 2022
@github-actions
Copy link

github-actions bot commented Nov 25, 2022

πŸ“Š Benchmark results

Package size: 247 MB

^  247 MB 
β”‚   β”Œβ”€β”€β”  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
β”‚   |β–’β–’|  
└───┴──┴──>
     T    
Legend
  • T (current commit): 247 MB

@danez danez changed the title chore: introduce vitests for ESM unit tests chore: introduce vitests for tests Nov 28, 2022
@danez danez marked this pull request as ready for review November 30, 2022 15:48
@danez danez requested a review from a team November 30, 2022 15:48
DIST_IMPORT_MAP_PATH,
INTERNAL_EDGE_FUNCTIONS_FOLDER,
EDGE_FUNCTIONS_FOLDER,
PUBLIC_URL_PATH,
SERVER_POLL_INTERNAL,
SERVER_POLL_TIMEOUT,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All unused

Copy link
Contributor

@jackiewmacharia jackiewmacharia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a few nitpick suggestions, all in all, this looks good.

e2e.config.mjs Outdated Show resolved Hide resolved
tests/integration/utils/got.cjs Outdated Show resolved Hide resolved
tests/unit/lib/functions/server.test.cjs Outdated Show resolved Hide resolved
@danez danez added the automerge Add to Kodiak auto merge queue label Dec 6, 2022
@kodiakhq kodiakhq bot merged commit 26c409d into main Dec 6, 2022
@kodiakhq kodiakhq bot deleted the vitest branch December 6, 2022 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Add to Kodiak auto merge queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants