Refactor from jest to vitest #2483 #3128
Conversation
WalkthroughThis pull request introduces several configuration and testing-related changes to the Talawa-admin project. The modifications include creating new environment configuration example files ( Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/AddPeopleToTag/AddPeopleToTag.spec.tsx (1)
184-184: Typographical nit: "searchs" → "searches".The test descriptions contain a minor misspelling. Consider renaming them to maintain clarity:
- it('searchs for tags where the firstName matches...', async () => { + it('searches for tags where the firstName matches...', async () => { - it('searchs for tags where the lastName matches...', async () => { + it('searches for tags where the lastName matches...', async () => {Also applies to: 217-217
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.env_BACKUP_322.example(1 hunks).env_LOCAL_322.example(1 hunks).env_REMOTE_322.example(1 hunks)eslint.config.js(1 hunks)package.json(1 hunks)src/components/AddPeopleToTag/AddPeopleToTag.spec.tsx(11 hunks)
✅ Files skipped from review due to trivial changes (3)
- eslint.config.js
- .env_LOCAL_322.example
- .env_REMOTE_322.example
🧰 Additional context used
📓 Learnings (1)
src/components/AddPeopleToTag/AddPeopleToTag.spec.tsx (1)
Learnt from: bitbard3
PR: PalisadoesFoundation/talawa-admin#2588
File: src/components/ChangeLanguageDropdown/ChangeLanguageDropdown.spec.tsx:145-155
Timestamp: 2024-12-02T04:20:11.745Z
Learning: In PRs focused solely on refactoring test cases from Jest to Vitest, avoid suggesting optimizations or changes outside the migration scope.
🔇 Additional comments (5)
src/components/AddPeopleToTag/AddPeopleToTag.spec.tsx (4)
2-2: Switched from Jest to Vitest.The import of
{ vi, expect, describe, it }from 'vitest' is correctly placed. Good job keeping the test framework usage consistent throughout the spec file.
38-41: Usage of vi.mock is consistent.Mocking
react-toastifywithvi.mockaligns well with Vitest's approach. This change looks correct.
118-125: Mockingreact-router-domwith Vitest.The approach of partially importing and then mocking
useParamsis a standard Vitest pattern. This transition away fromjest.mockis applied correctly.
127-128: Clearing mocks before each test run.Calling
vi.clearAllMocks()inbeforeEachensures a clean state for every test. Excellent practice.package.json (1)
142-148: Dependencies upgraded successfully.Upgrading ESLint and related plugins appears consistent with the transition to the new test setup. No issues detected.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3128 +/- ##
=====================================================
+ Coverage 33.14% 89.52% +56.38%
=====================================================
Files 299 322 +23
Lines 7429 8455 +1026
Branches 1624 1844 +220
=====================================================
+ Hits 2462 7569 +5107
+ Misses 4762 655 -4107
- Partials 205 231 +26 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.env_BACKUP_322.example (3)
11-11: Add example value for REACT_APP_TALAWA_URL.To help developers understand the expected format, provide an example value.
-REACT_APP_TALAWA_URL= +REACT_APP_TALAWA_URL=http://localhost:4000/graphql
17-17: Enhance security-related documentation.For security-sensitive configurations (RECAPTCHA), consider:
- Adding a comment warning users not to commit actual keys
- Providing a dummy/example key format
-REACT_APP_USE_RECAPTCHA= +REACT_APP_USE_RECAPTCHA= # yes/no - DO NOT COMMIT ACTUAL CONFIGURATION # If you are using Google Recaptcha, i.e., REACT_APP_USE_RECAPTCHA=yes, read the following steps # Get the google recaptcha site key from google recaptcha admin or https://www.google.com/recaptcha/admin/create # from here for reCAPTCHA v2 and "I'm not a robot" Checkbox, and paste the key here. # Note: In domains, fill localhost -REACT_APP_RECAPTCHA_SITE_KEY= +REACT_APP_RECAPTCHA_SITE_KEY= # Format: 6LcXXXXXXXXXXXXXXXXXXXXXXXXXXXXX - DO NOT COMMIT ACTUAL KEYAlso applies to: 24-24
29-31: Standardize the ALLOW_LOGS configuration.The current documentation for ALLOW_LOGS could be improved:
- Inconsistent casing ("YES" vs typical boolean values)
- Typos and formatting in the comment
-# If you want to logs Compiletime and Runtime error , warning and info write YES or if u want to -# keep the console clean leave it blank -ALLOW_LOGS= +# Enable logging of compile-time and runtime errors, warnings, and info +# Set to 'true' to enable logging, leave blank to keep the console clean +ALLOW_LOGS= # true/false
| # 👋 Welcome, we're glad you're setting up an installation of Talawa-admin. Copy this | ||
| # file to .env or set the variables in your local environment manually. | ||
|
|
||
|
|
||
| # Custom port number for the talawa-admin development server to run on. Default is 4321. | ||
|
|
||
| PORT=4321 | ||
|
|
||
| # Run Talawa-api locally in your system, and put its url into the same. | ||
|
|
||
| REACT_APP_TALAWA_URL= | ||
|
|
||
| # Do you want to setup and use "I'm not a robot" Checkbox (Google Recaptcha)? | ||
| # If no, leave blank, else write yes | ||
| # Example: REACT_APP_USE_RECAPTCHA=yes | ||
|
|
||
| REACT_APP_USE_RECAPTCHA= | ||
|
|
||
| # If you are using Google Recaptcha, i.e., REACT_APP_USE_RECAPTCHA=yes, read the following steps | ||
| # Get the google recaptcha site key from google recaptcha admin or https://www.google.com/recaptcha/admin/create | ||
| # from here for reCAPTCHA v2 and "I'm not a robot" Checkbox, and paste the key here. | ||
| # Note: In domains, fill localhost | ||
|
|
||
| REACT_APP_RECAPTCHA_SITE_KEY= | ||
|
|
||
| # has to be inserted in the env file to use plugins and other websocket based features. | ||
| REACT_APP_BACKEND_WEBSOCKET_URL=ws://localhost:4000/graphql | ||
|
|
||
| # If you want to logs Compiletime and Runtime error , warning and info write YES or if u want to | ||
| # keep the console clean leave it blank | ||
| ALLOW_LOGS= |
There was a problem hiding this comment.
Rename the file to follow standard conventions.
The filename .env_BACKUP_322.example appears to be a merge conflict artifact (containing "BACKUP" and "322"). This should be renamed to follow the standard .env.example convention for environment template files.
Apply this change:
- Rename the file from
.env_BACKUP_322.exampleto.env.example - Remove any duplicate
.env_LOCAL_322.exampleand.env_REMOTE_322.examplefiles if they contain the same configuration
| # has to be inserted in the env file to use plugins and other websocket based features. | ||
| REACT_APP_BACKEND_WEBSOCKET_URL=ws://localhost:4000/graphql |
There was a problem hiding this comment.
💡 Codebase verification
WebSocket URL format inconsistency found in environment files
There is an inconsistency in the trailing slash of the WebSocket URLs across the environment files:
- With trailing slash
/:.env.example:ws://localhost:4000/graphql/.env_LOCAL_322.example:ws://localhost:4000/graphql/
- Without trailing slash:
.env_BACKUP_322.example:ws://localhost:4000/graphql.env_REMOTE_322.example:ws://localhost:4000/graphql
The format should be standardized across all environment files. Based on the INSTALLATION.md examples, the format with the trailing slash (ws://localhost:4000/graphql/) appears to be the intended format.
🔗 Analysis chain
Verify WebSocket URL format consistency.
The WebSocket URL format should be consistent across all environment files. Let's verify this:
Let me gather more information to check the actual .env files.
Let me try again with the hidden flag.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for inconsistencies in WebSocket URL format across env files
echo "Checking WebSocket URL format in env files..."
rg -l "REACT_APP_BACKEND_WEBSOCKET_URL" | while read -r file; do
echo "=== $file ==="
rg "REACT_APP_BACKEND_WEBSOCKET_URL" "$file"
done
Length of output: 2122
Script:
#!/bin/bash
# Check all env files and examples for WebSocket URL format
echo "Checking all .env* files..."
fd -t f "^\.env" | while read -r file; do
echo "=== $file ==="
grep "REACT_APP_BACKEND_WEBSOCKET_URL" "$file" || true
done
Length of output: 306
Script:
#!/bin/bash
# Check all env files and examples for WebSocket URL format
echo "Checking all .env* files..."
fd -H -t f "^\.env" | while read -r file; do
echo "=== $file ==="
grep "REACT_APP_BACKEND_WEBSOCKET_URL" "$file" || true
done
Length of output: 1063
|
Refactor : src/components/AddPeopleToTag/AddPeopleToTag.test.tsx from Jest to Vitest #2483
What kind of change does this PR introduce?
Refactor
Issue Number: #2483
Fixes #2483
Did you add tests for your changes?
Yes
Snapshots/Videos:
Summary
Migrated test for ChangeLanguageDropdown.tsx from jest to vitest.
Does this PR introduce a breaking change?
No
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Configuration
Development Dependencies
Testing