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

feat-#6: Setup jest rtl #84

Merged
merged 15 commits into from
Dec 20, 2023

Conversation

bajajcodes
Copy link
Collaborator

@bajajcodes bajajcodes commented Dec 15, 2023

Summary

This pull request addresses Issue #6 by adding integration tests to the frontend using Jest for a React TypeScript project created with Vite. The goal is to enhance the application's stability and security as it grows in size.

Description

This pull request introduces Jest as the testing framework for the React TypeScript project. The setup includes configuring Jest to work seamlessly with Vite, handling TypeScript, JSX syntax, and integrating testing libraries for React.

Steps Taken:

  1. Install Dependencies:

    npm i jest @types/jest ts-jest --save-dev
  2. Exclude Test Files from TypeScript Type Checking for Production:

    • Create tsconfig.prod.json to exclude test files.
    • Modify package.json to use tsconfig.prod.json during production build.
  3. Add Test Script to package.json:

    "scripts": {
      "test": "NODE_ENV=test jest"
    }
  4. Write Simple Test:

    • Create a basic test in src/App.test.tsx to check if 1 + 1 equals 2.
  5. Test the App Component:

    • Expand the test to render the App component.
  6. Configure JSX Support with Babel:

    • Install @babel/preset-env, @babel/preset-react, and @babel/preset-typescript.
    • Create a .babelrc file to configure Babel presets.
  7. Install Testing Libraries for React:

    npm i @testing-library/dom @testing-library/jest-dom @testing-library/react @testing-library/user-event --save-dev
  8. Configure Jest Environment:

    • Install ts-node and jest-environment-jsdom.
    • Create jest.config.ts to configure Jest settings.
    • Create mock files for CSS and other assets.
  9. Fix import.meta Error:

    • Install ts-jest-mock-import-meta.
    • Modify Jest config to mock import.meta of Vite.

Images

NA

Issue(s) Addressed

#6

Prerequisites

Additional Information

  • Snapshot testing has not been implemented in this PR.

Reviewers are encouraged to inspect the changes and provide feedback.

Copy link

vercel bot commented Dec 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
wanderlust ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2023 5:26pm
wanderlust-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2023 5:26pm

@krishnaacharyaa
Copy link
Owner

krishnaacharyaa commented Dec 16, 2023

Just some observations:

Closes #6

I assume it is bymistake, because we are aiming to just setup the jest in this PR, correct me if I am wrong.

About the folderstructure and filenames we might have to discuss about the tradeoffs,
You can actually refer to the backend unit test cases written, we are expecting that directory structure here if possible, open to sticking to standards,

Can you confirm if these filenames are by default designed to be that way? or is it custom file names? if custom we have to follow our guidelines.
image

@bajajcodes bajajcodes changed the title Setup jest rtl feat-#6: Setup jest rtl Dec 16, 2023
@bajajcodes
Copy link
Collaborator Author

bajajcodes commented Dec 16, 2023

  • Yes, Closes #6 was by mistake.

  • Why use tests? Well, it was probably chosen randomly.

Another benefit of starting the folder name with a non-letter or number is that it won't get mixed up with folders containing "production" code when you list the directories.

  • These filenames (fileMock.ts, styleMock.ts, setupTests.ts) are custom designed, and changed according to guidelines(file-mock.ts, style-mock.ts, setup-tests.ts).

@krishnaacharyaa
Copy link
Owner

@shmbajaj, Thank you for the quick update, just one more thing, i see we have lots of settings and many things in the json files, can we minimize it to what we actually want presently than getting everything at once, if everything is actually needed, we can have a call where you roughly say whats the importance of each of things.

Another benefit of starting the folder name with a non-letter or number is that it won't get mixed up with folders containing "production" code when you list the directories.

I understand, I am open to this, and I have seen at places being used this way, we'll discuss and come to decision, which will be best for our project and we can change in the backend aswell if needed.
Tests are not my expertise, so @chinmaykunkikar and @kuldeepjambhulkar can you offer your helping hand here.

@krishnaacharyaa
Copy link
Owner

Kindly follow commit format, refer #67 for better understanding, hope you don't get pestered, I understand for the new contributors it will not suddenly pop in mind because we are so involved in the code, but this was done so that we know in just a split of second what the commit is about as in a long run we know what to actually catch or look out for, hope you get it :)

p.s: I have also planned to keep some regex constraints before commiting, @chinmaykunkikar doesn't really agree the necessity of it though :| Let's see what we can do, I guess I'll have to convince him anyways xD

@chinmaykunkikar
Copy link
Collaborator

chinmaykunkikar commented Dec 16, 2023

@krishnaacharyaa

i see we have lots of settings and many things in the json files

Hmm.. I don't think we can do anything about it. That's just the part (and the pain) of using the current js ecosystem. May be a solution hack is to add those settings into the package.json. But then package.json will start getting bloated, if it's not already. I wish it gets better in the future.
Ah, yes, I'd suggest we find a way to merge all the tsconfig related files in the main tsconfig file itself. I haven't looked it up, yet. @shmbajaj can you please check if TypeScript lang has trick up its sleeve to automatically exclude dirs based on the node env?

whats the importance of each of things

Most of them are the part of setup process to ensure things are running smoothly. If tsconfig files are merged, that will only leave the extra .babelrc file, which is required by babel to help with the jsx transforms.

I have also planned to keep some regex constraints before commiting, @chinmaykunkikar doesn't really agree the necessity of it though

That's not completely true :P I agree with your intent, it just that it may discourage someone who thinks they're being constrained by git.
You can do it to check if the first word matches the commit action (refactor, feat, fix, derp (stupid mistakes that don't really count as a "fix"), wip and so on).

Also, haven't you checked this out yet?

Copy link
Collaborator

@chinmaykunkikar chinmaykunkikar left a comment

Choose a reason for hiding this comment

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

@shmbajaj this looks good to me to start with. We can always modify it according to our needs later.

Note

@krishnaacharyaa please check if anything on vercel needs to be changed for this to work. See if the production and the test environments is being recognized by vercel. We don't want the test specific logic to run on prod.

frontend/tsconfig.prod.json Show resolved Hide resolved
@krishnaacharyaa
Copy link
Owner

@krishnaacharyaa please check if anything on vercel needs to be changed for this to work. See if the production and the test environments is being recognized by vercel. We don't want the test specific logic to run on prod.

Yeah this one, I'll have to see, @shmbajaj can we do just this in another PR? or is it actually required right away? meaning we can sit together and properly differentiate whatever is needed for dev and prod environments including backend. If it is the main part of this. we'll sit and figure out. If that's fine by you @chinmaykunkikar (vercel is calling you) xD. Let's see what would be the required changes and the best practices, we'll separate it accordingly.

image

@bajajcodes
Copy link
Collaborator Author

@krishnaacharyaa @chinmaykunkikar

Let's connect over call and conclude this PR, and
then I will push changes required altogether.

@bajajcodes
Copy link
Collaborator Author

@krishnaacharyaa please check if anything on vercel needs to be changed for this to work. See if the production and the test environments is being recognized by vercel. We don't want the test specific logic to run on prod.

Yeah this one, I'll have to see, @shmbajaj can we do just this in another PR? or is it actually required right away? meaning we can sit together and properly differentiate whatever is needed for dev and prod environments including backend. If it is the main part of this. we'll sit and figure out. If that's fine by you @chinmaykunkikar (vercel is calling you) xD. Let's see what would be the required changes and the best practices, we'll separate it accordingly.

image

It is required, and we need to resolve the PR fast.

@kuldeepjambhulkar
Copy link

Hi @krishnaacharyaa

I can't really provide any suggestions on this PR, since I'm not equipped with enough knowledge of jest.
Feel free to drop me from mandatory reviewers from this one.

@bajajcodes
Copy link
Collaborator Author

bajajcodes commented Dec 20, 2023

Hi @krishnaacharyaa

I can't really provide any suggestions on this PR, since I'm not equipped with enough knowledge of jest. Feel free to drop me from mandatory reviewers from this one.

Hey @kuldeepjambhulkar, No issues you can watch this quick video on how to setup jest or even good read a 5 minute tutorial on topic in discussion.

YT Video: Mastering Jest Configuration for React TypeScript Projects with Vite: A Step-by-Step Guide

Blog: Mastering Jest Configuration for React TypeScript Projects with Vite: A Step-by-Step Guide

Copy link
Owner

@krishnaacharyaa krishnaacharyaa 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 for the PR, LGTM
Sorry for the delay in merging...
Test cases are always a little tough one xD

@krishnaacharyaa krishnaacharyaa removed the request for review from kuldeepjambhulkar December 20, 2023 17:30
@krishnaacharyaa krishnaacharyaa merged commit 771bd04 into krishnaacharyaa:main Dec 20, 2023
3 checks passed
@bajajcodes bajajcodes deleted the setup-jest-rtl branch December 20, 2023 18:42
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