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

Add vitest for frontend testing #2810

Merged
merged 2 commits into from
Jun 5, 2024
Merged

Add vitest for frontend testing #2810

merged 2 commits into from
Jun 5, 2024

Conversation

deen13
Copy link
Contributor

@deen13 deen13 commented May 22, 2024

Short Description

This pull request configures vitest and adds a simple test as suggested in my review of deps/upgrade_webauthn_2_0.

Proposed Changes

  • Added a test task to package.json
  • Included required vitest dependency
  • Implemented a simple test for URL and base64 encoding
  • Configure vitest to ignore python dependencies
  • Configure import/resolver to resolve imports correctly
  • Add job to circleci workflow

Pull Request Review Guidelines

@deen13 deen13 requested a review from a team as a code owner May 22, 2024 09:11
Copy link

codeclimate bot commented May 22, 2024

Code Climate has analyzed commit c7a6d03 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 82.2% (0.0% change).

View more on Code Climate.

Copy link
Contributor

@JoeyStk JoeyStk left a comment

Choose a reason for hiding this comment

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

Thank you a lot 🚀 I think this is a good step forward.

  1. Small note: Once this PR is merged you can check the checkbox in our meta issue for tests.
  2. I personally like vitest more than jest for our usecase. I think Jest has a lot of features that we don't need as we are web only and therefore I rather prefer the speed of vitest.
  3. Lastly the reason why I don't approve yet. Do we need a dev tool for this? I'm kind of lost in the whole discussion about pre-commits and dev commands 😕 Once this question is answered I will approve this PR as it looks good to me so far :)

Special thanks for the dummy test. These things always help me to get started with a new feature :) In the mid-future we can remove it again, but for the beginning it's much appreciated (at least on my side) :)

@JoeyStk JoeyStk self-assigned this May 28, 2024
@deen13 deen13 requested a review from JoeyStk June 5, 2024 12:20
@deen13
Copy link
Contributor Author

deen13 commented Jun 5, 2024

@JoeyStk I've added a devtool and a pre-commit hook that calls it. ☺️

@deen13 deen13 requested review from timobrembeck, PeterNerlich, charludo, seluianova and theresantonie and removed request for timobrembeck June 5, 2024 12:36
Copy link
Contributor

@JoeyStk JoeyStk left a comment

Choose a reason for hiding this comment

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

I'm not an expert on this matter, but would like to move this forward.
To me this PR works. I didn't face issues when installing this and running the dev-tool. Thank you very much :)

@ulliholtgrave
Copy link
Member

Thank you very much 👍 This really is an improvement! Could you squash the first two commits into one? Then, this is good to go :)

deen13 added 2 commits June 5, 2024 16:41
includes a simple test for url and base64 encoding

Configure vitest to not watch files in .venv
Watching files in .venv is not required,
since we don't have any tests there and it can also
 lead to problems on platforms that only allow a
limited number of file watchers.

Co-authored-by: deen13 <[email protected]>

Configure import/resover in order to use import within vitest.config.ts

eslint/eslint#14667 (reply in thread)

Add vitest devtool
@deen13 deen13 force-pushed the feature/ts_tests branch from 8efbe67 to c7a6d03 Compare June 5, 2024 14:43
@deen13 deen13 merged commit 5c6602a into develop Jun 5, 2024
5 checks passed
@deen13 deen13 deleted the feature/ts_tests branch June 5, 2024 14:58
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.

4 participants