Fix: Make setup.ts module execution conditional #6313
Conversation
Signed-off-by: Yuvraj Kolkar <[email protected]>
|
To generate Unit Tests for this PR, please click here. |
Our Pull Request Approval ProcessThis PR will be reviewed according to our: Your PR may be automatically closed if:
Thanks for contributing! |
WalkthroughStops automatic execution on import; exposes explicit entry points Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as setup.ts (main)
participant Inquirer as Inquirer
participant Validator as Recaptcha Validator
participant FS as File System
User->>CLI: invoke main()
CLI->>Inquirer: prompt recaptcha & logging questions
Inquirer-->>CLI: answers
CLI->>Validator: validate recaptcha key (if provided)
Validator-->>CLI: validation result / throws error
CLI->>FS: read .env (sync)
FS-->>CLI: .env content
CLI->>FS: backup .env
CLI->>FS: write updated .env
CLI-->>User: success or error (logs and process.exit)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/setup/setup.ts (2)
52-62: Inconsistent error handling betweenaskAndSetLogErrorsandaskAndSetRecaptcha.
askAndSetRecaptcha(lines 13-49) has a try/catch that wraps errors with a descriptive message, butaskAndSetLogErrorshas no error handling and lets errors propagate raw. While errors will eventually be caught inmain(), this inconsistency may confuse maintainers.Consider adding consistent error handling:
♻️ Proposed fix for consistency
// Ask and set up logging errors in the console export const askAndSetLogErrors = async (): Promise<void> => { + try { const { shouldLogErrors } = await inquirer.prompt({ type: 'confirm', name: 'shouldLogErrors', message: 'Would you like to log Compiletime and Runtime errors in the console?', default: true, }); updateEnvFile('ALLOW_LOGS', shouldLogErrors ? 'YES' : 'NO'); + } catch (error) { + console.error('Error setting up log errors:', error); + throw new Error(`Failed to set up log errors: ${(error as Error).message}`); + } };
12-14: Missing TSDoc comments on exported functions.Per coding guidelines, all exported functions must have TSDoc documentation. The exported functions
askAndSetRecaptcha,askAndSetLogErrors, andmainlack TSDoc comments. Based on learnings, use TSDoc format without curly braces around types.📝 Proposed TSDoc additions
+/** + * Prompts the user to configure reCAPTCHA settings and updates the environment file. + * @throws Error if the reCAPTCHA setup fails + * @returns Promise<void> + */ export const askAndSetRecaptcha = async (): Promise<void> => {+/** + * Prompts the user to configure console error logging and updates the environment file. + * @returns Promise<void> + */ export const askAndSetLogErrors = async (): Promise<void> => {+/** + * Main entry point for the Talawa Admin setup process. + * Orchestrates environment configuration, Docker options, API URL, reCAPTCHA, and logging. + * @throws Error if any setup step fails (exits with code 1) + * @returns Promise<void> + */ export async function main(): Promise<void> {Also applies to: 51-52, 64-65
🤖 Fix all issues with AI agents
In @src/setup/setup.ts:
- Around line 100-103: The ES module execution guard calls main() without
handling rejections; change the invocation inside the import.meta.url check to
handle async errors by awaiting or attaching a catch handler to main() (e.g.,
Promise.resolve(main()).catch(err => { console.error('Uncaught error in main:',
err); process.exit(1); })), ensuring any thrown or rejected promise from main()
is logged and the process exits with a non-zero code; reference the existing
guard around import.meta.url and the main() function.
- Around line 100-103: The ES module execution guard comparing import.meta.url
to `file://${process.argv[1]}` fails on Windows; convert the module URL to a
native filesystem path using Node's fileURLToPath and compare that to
process.argv[1] before calling main(). Import or require fileURLToPath from
'url' (and optionally normalize with path.resolve if needed), replace the
current equality check with fileURLToPath(import.meta.url) === process.argv[1],
and keep the call to main() inside that guard.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/setup/setup.spec.tssrc/setup/setup.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/setup/setup.tssrc/setup/setup.spec.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/setup/setup.tssrc/setup/setup.spec.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/setup/setup.spec.ts
🧠 Learnings (5)
📚 Learning: 2026-01-06T21:27:50.880Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6222
File: src/utils/userUpdateUtils.ts:22-22
Timestamp: 2026-01-06T21:27:50.880Z
Learning: In the Talawa Admin codebase, the i18n extraction tool can misclassify TypeScript generic syntax (e.g., (obj: T): Partial) as translatable content. Suppress these false positives by adding a // i18n-ignore-next-line comment directly above the line with the type declaration or generic function signature. Apply this only to TS code where misclassification is confirmed; avoid blanket suppression.
Applied to files:
src/setup/setup.tssrc/setup/setup.spec.ts
📚 Learning: 2026-01-07T14:21:19.711Z
Learnt from: cotishq
Repo: PalisadoesFoundation/talawa-admin PR: 6198
File: src/setup/setup.ts:123-124
Timestamp: 2026-01-07T14:21:19.711Z
Learning: Use TSDoc (eslint-plugin-tsdoc) instead of JSDoc. Do not wrap type names in curly braces in throws and returns. Write as "throws Error" and "returns Promise<void>" (no braces). Curly-brace usage causes tsdoc-malformed-inline-tag and tsdoc-escape-right-brace lint errors. Apply this rule to all TypeScript source files (e.g., src/**/*.ts) to ensure consistency.
Applied to files:
src/setup/setup.tssrc/setup/setup.spec.ts
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/setup/setup.spec.ts
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/setup/setup.spec.ts
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/setup/setup.spec.ts
🧬 Code graph analysis (2)
src/setup/setup.ts (2)
src/setup/validateRecaptcha/validateRecaptcha.ts (1)
validateRecaptcha(1-4)src/setup/checkEnvFile/checkEnvFile.ts (1)
modifyEnvFile(19-27)
src/setup/setup.spec.ts (6)
src/setup/checkEnvFile/checkEnvFile.ts (2)
checkEnvFile(7-17)modifyEnvFile(19-27)src/setup/askForDocker/askForDocker.ts (1)
askAndUpdateTalawaApiUrl(34-100)src/setup/updateEnvFile/updateEnvFile.ts (1)
updateEnvFile(18-61)src/setup/validateRecaptcha/validateRecaptcha.ts (1)
validateRecaptcha(1-4)src/setup/backupEnvFile/backupEnvFile.ts (1)
backupEnvFile(6-48)src/setup/setup.ts (3)
main(65-98)askAndSetRecaptcha(13-49)askAndSetLogErrors(52-62)
🔇 Additional comments (9)
src/setup/setup.ts (2)
32-37: LGTM - Validation function refactored correctly.The block-bodied arrow function properly returns
truefor valid keys or the error message string for invalid ones, satisfying inquirer's validation contract.
45-48: LGTM - Simplified error handling.The error wrapping is cleaner and provides a consistent, informative message.
src/setup/setup.spec.ts (7)
15-22: LGTM - Test-specific type definition.The
PromptQuestioninterface is appropriately scoped to the test file for typing mock prompt calls. This is acceptable for test utilities and doesn't violate the coding guideline about interface placement (which applies to component props).
79-161: LGTM - Comprehensive main function tests.The tests cover:
- Success path with default options
- Docker YES/NO branching logic
- Error logging opt-in/out
- Error handling with process.exit verification
- Early exit when checkEnvFile returns false
- Welcome and success message display
Good coverage of the main flow and its branches.
301-471: LGTM - Thorough askAndSetRecaptcha tests.Excellent coverage including:
- Opt-in with valid key
- Opt-out path
- Error propagation with helpful messages
- Validation function behavior (returns true/error string)
- Second prompt error handling
- updateEnvFile error handling
The validation function extraction and testing (lines 388-434) is particularly well done.
474-528: LGTM - askAndSetLogErrors tests cover the function well.Tests cover opt-in, opt-out, error propagation, updateEnvFile errors, and prompt configuration validation. The tests correctly expect raw error propagation given the current implementation without try/catch.
530-551: LGTM - Module execution tests validate PR objective.The "should not have side effects on import" test (lines 537-551) directly validates the PR's core objective of preventing automatic execution during testing. This is a key regression test for the ES module guard.
554-597: LGTM - Edge case coverage is thorough.Good defensive testing:
- Empty env config (USE_DOCKER undefined)
- Malformed env file content
- Case-sensitive Docker flag verification (
yesvsYES)These tests document expected behavior for edge cases.
71-77: Redundant mock clearing calls.
vi.resetAllMocks()already clears mock state (call history, instances, etc.) in addition to resetting implementations. Callingvi.clearAllMocks()beforehand is redundant.♻️ Simplify cleanup
afterEach(() => { - vi.clearAllMocks(); vi.resetAllMocks(); processExitSpy.mockRestore(); consoleErrorSpy.mockRestore(); consoleLogSpy.mockRestore(); });⛔ Skipped due to learnings
Learnt from: AdityaRathi-10 Repo: PalisadoesFoundation/talawa-admin PR: 0 File: :0-0 Timestamp: 2025-12-19T06:50:24.198Z Learning: In talawa-admin tests, prefer global afterEach(() => vi.clearAllMocks()) and use per-test spy.mockRestore() to clean up specific vi.spyOn(...) instances. Avoid vi.restoreAllMocks() in afterEach because it breaks hoisted/module mocks; restoring individual spies is safe and recommended.
Signed-off-by: Yuvraj Kolkar <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/setup/setup.ts (3)
66-99:main()is exported but hard-exits the process (makes programmatic use and tests brittle).
Ifmain()is imported/called (tests or tooling), any failure terminates the process. Prefer:main()throws; the entrypoint guard decidesprocess.exit(1).Proposed refactor (throw from main; exit only in entrypoint)
export async function main(): Promise<void> { try { if (!checkEnvFile()) { return; } console.log('Welcome to the Talawa Admin setup! 🚀'); await backupEnvFile(); modifyEnvFile(); await askAndSetDockerOption(); const envConfig = dotenv.parse(fs.readFileSync('.env', 'utf8')); const useDocker = envConfig.USE_DOCKER === 'YES'; if (useDocker) { await askAndUpdateTalawaApiUrl(useDocker); } else { await askAndUpdatePort(); await askAndUpdateTalawaApiUrl(useDocker); } await askAndSetRecaptcha(); await askAndSetLogErrors(); console.log( '\nCongratulations! Talawa Admin has been successfully set up! 🥂🎉', ); } catch (error) { console.error('\n❌ Setup failed:', error); console.log('\nPlease try again or contact support if the issue persists.'); - process.exit(1); + throw error; } }Also applies to: 101-107
52-63: Public export lacks error-context wrapping; prompt text typo (“Compiletime”).
If the intent is to make helpers uniformly testable and debuggable, mirror theaskAndSetRecaptchapattern and fix the copy.Proposed fix (consistent error context + copy)
export const askAndSetLogErrors = async (): Promise<void> => { - const { shouldLogErrors } = await inquirer.prompt({ - type: 'confirm', - name: 'shouldLogErrors', - message: - 'Would you like to log Compiletime and Runtime errors in the console?', - default: true, - }); - - updateEnvFile('ALLOW_LOGS', shouldLogErrors ? 'YES' : 'NO'); + try { + const { shouldLogErrors } = await inquirer.prompt({ + type: 'confirm', + name: 'shouldLogErrors', + message: + 'Would you like to log compile-time and runtime errors in the console?', + default: true, + }); + + updateEnvFile('ALLOW_LOGS', shouldLogErrors ? 'YES' : 'NO'); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error(`Failed to configure ALLOW_LOGS: ${message}`); + } };
13-66: Missing TSDoc on exported functions (askAndSetRecaptcha,askAndSetLogErrors,main).
Repo linting may require TSDoc; also ensure TSDoc style matches repo rules (no{}around types). Based on learnings.
🤖 Fix all issues with AI agents
In @src/setup/setup.ts:
- Around line 33-39: The validation code uses direct `(error as Error).message`
and may double-log or produce undefined for non-Error throws; update the
validate handlers that call validateRecaptcha (the inline validate: (input:
string) => ... functions) to normalize error extraction by checking if the
caught value is an Error and using its message, otherwise converting the thrown
value to a string (e.g., String(err) or JSON.stringify when safe), and avoid
logging the same failure twice—apply this robust message extraction and single
logging pattern to both validate handlers referencing validateRecaptcha (the
block shown and the similar block at lines 46-49).
- Line 3: The entrypoint guard using fileURLToPath(import.meta.url) vs
process.argv[1] is brittle; remove the guard and convert this file into a
script-only module by making main() non-exported (remove any export on main) and
change any direct process.exit(1) calls inside main() to return numeric error
codes (or throw) so callers aren't forced to exit the process; ensure the CLI
invocation still calls main() and exits based on the returned code (e.g., call
main().then(code => process.exit(code)). Use identifiers main() and process.exit
in your changes to locate usages.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/setup/setup.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/setup/setup.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/setup/setup.ts
🧠 Learnings (2)
📚 Learning: 2026-01-06T21:27:50.880Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6222
File: src/utils/userUpdateUtils.ts:22-22
Timestamp: 2026-01-06T21:27:50.880Z
Learning: In the Talawa Admin codebase, the i18n extraction tool can misclassify TypeScript generic syntax (e.g., (obj: T): Partial) as translatable content. Suppress these false positives by adding a // i18n-ignore-next-line comment directly above the line with the type declaration or generic function signature. Apply this only to TS code where misclassification is confirmed; avoid blanket suppression.
Applied to files:
src/setup/setup.ts
📚 Learning: 2026-01-07T14:21:19.711Z
Learnt from: cotishq
Repo: PalisadoesFoundation/talawa-admin PR: 6198
File: src/setup/setup.ts:123-124
Timestamp: 2026-01-07T14:21:19.711Z
Learning: Use TSDoc (eslint-plugin-tsdoc) instead of JSDoc. Do not wrap type names in curly braces in throws and returns. Write as "throws Error" and "returns Promise<void>" (no braces). Curly-brace usage causes tsdoc-malformed-inline-tag and tsdoc-escape-right-brace lint errors. Apply this rule to all TypeScript source files (e.g., src/**/*.ts) to ensure consistency.
Applied to files:
src/setup/setup.ts
🧬 Code graph analysis (1)
src/setup/setup.ts (2)
src/setup/validateRecaptcha/validateRecaptcha.ts (1)
validateRecaptcha(1-4)src/setup/checkEnvFile/checkEnvFile.ts (1)
modifyEnvFile(19-27)
🔇 Additional comments (1)
src/setup/setup.ts (1)
74-86: No TOCTOU vulnerability exists at this location; original concern is invalid.
updateEnvFile()returnsvoidand usesfs.writeFileSync()(lines 32, 57 of updateEnvFile.ts), which are synchronous. AllupdateEnvFile()calls withinaskAndSetDockerOption()(lines 18, 21, 22, 31) complete synchronously and finish before theawait askAndSetDockerOption()resolves on line 77. Thefs.readFileSync()on line 78 reads the file after all file writes have already persisted, soenvConfig.USE_DOCKERwill have the correct, updated value.
Signed-off-by: Yuvraj Kolkar <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/setup/setup.ts (3)
27-67: Add TSDoc comments for the exported function.Per coding guidelines and learnings, all exported functions must have TSDoc documentation. Based on learnings, use TSDoc format without curly braces around type names.
📝 Proposed TSDoc addition
+/** + * Prompts the user to configure reCAPTCHA settings and updates the environment file. + * @throws Error when reCAPTCHA setup fails + * @returns Promise<void> + */ export const askAndSetRecaptcha = async (): Promise<void> => {
69-80: Inconsistent error handling and missing TSDoc.
Inconsistent error handling:
askAndSetRecaptchawraps errors with contextual messages, butaskAndSetLogErrorslets errors propagate raw. Consider adding consistent error handling for parity.Missing TSDoc: Add documentation for this exported function.
📝 Proposed changes for consistency and documentation
-// Ask and set up logging errors in the console +/** + * Prompts the user to configure console logging for compile-time and runtime errors. + * @throws Error when logging setup fails + * @returns Promise<void> + */ export const askAndSetLogErrors = async (): Promise<void> => { - const { shouldLogErrors } = await inquirer.prompt({ - type: 'confirm', - name: 'shouldLogErrors', - message: - 'Would you like to log Compiletime and Runtime errors in the console?', - default: true, - }); - - updateEnvFile('ALLOW_LOGS', shouldLogErrors ? 'YES' : 'NO'); + try { + const { shouldLogErrors } = await inquirer.prompt({ + type: 'confirm', + name: 'shouldLogErrors', + message: + 'Would you like to log Compiletime and Runtime errors in the console?', + default: true, + }); + + updateEnvFile('ALLOW_LOGS', shouldLogErrors ? 'YES' : 'NO'); + } catch (error) { + console.error('Error setting up log errors:', error); + throw new Error(`Failed to set up log errors: ${getErrorMessage(error)}`); + } };
82-116: Add required TSDoc for exported main function.The
main()function is exported and must have JSDoc documentation per coding guidelines (section 4). Currently it has only a comment line.Update the comment to proper JSDoc format:
📝 Required JSDoc addition
-// Main function to run the setup process +/** + * Main entry point for the Talawa Admin setup process. + * Runs the interactive setup flow including Docker, API URL, reCAPTCHA, and logging configuration. + * @throws Error when setup fails (also calls process.exit(1)) + * @returns Promise<void> + */ export async function main(): Promise<void> {The implementation is sound (correct error handling, no auto-execution code after line 116).
🤖 Fix all issues with AI agents
In @src/setup/setup.spec.ts:
- Around line 15-22: Replace the local PromptQuestion interface with the
official Inquirer types: import type { Question } from 'inquirer' and use
Question (or Questions) where PromptQuestion is referenced; remove the custom
PromptQuestion declaration and update any variable annotations to
Question<string> (or Questions<string>) so the test uses the library-provided
types for consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/setup/setup.spec.tssrc/setup/setup.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/setup/setup.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/setup/setup.spec.tssrc/setup/setup.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/setup/setup.spec.tssrc/setup/setup.ts
🧠 Learnings (5)
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/setup/setup.spec.ts
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/setup/setup.spec.ts
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/setup/setup.spec.ts
📚 Learning: 2026-01-06T21:27:50.880Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6222
File: src/utils/userUpdateUtils.ts:22-22
Timestamp: 2026-01-06T21:27:50.880Z
Learning: In the Talawa Admin codebase, the i18n extraction tool can misclassify TypeScript generic syntax (e.g., (obj: T): Partial) as translatable content. Suppress these false positives by adding a // i18n-ignore-next-line comment directly above the line with the type declaration or generic function signature. Apply this only to TS code where misclassification is confirmed; avoid blanket suppression.
Applied to files:
src/setup/setup.spec.tssrc/setup/setup.ts
📚 Learning: 2026-01-07T14:21:19.711Z
Learnt from: cotishq
Repo: PalisadoesFoundation/talawa-admin PR: 6198
File: src/setup/setup.ts:123-124
Timestamp: 2026-01-07T14:21:19.711Z
Learning: Use TSDoc (eslint-plugin-tsdoc) instead of JSDoc. Do not wrap type names in curly braces in throws and returns. Write as "throws Error" and "returns Promise<void>" (no braces). Curly-brace usage causes tsdoc-malformed-inline-tag and tsdoc-escape-right-brace lint errors. Apply this rule to all TypeScript source files (e.g., src/**/*.ts) to ensure consistency.
Applied to files:
src/setup/setup.spec.tssrc/setup/setup.ts
🧬 Code graph analysis (2)
src/setup/setup.spec.ts (6)
src/setup/checkEnvFile/checkEnvFile.ts (2)
checkEnvFile(7-17)modifyEnvFile(19-27)src/setup/askForDocker/askForDocker.ts (1)
askAndUpdateTalawaApiUrl(34-100)src/setup/updateEnvFile/updateEnvFile.ts (1)
updateEnvFile(18-61)src/setup/validateRecaptcha/validateRecaptcha.ts (1)
validateRecaptcha(1-4)src/setup/backupEnvFile/backupEnvFile.ts (1)
backupEnvFile(6-48)src/setup/setup.ts (3)
main(83-116)askAndSetRecaptcha(28-67)askAndSetLogErrors(70-80)
src/setup/setup.ts (2)
src/setup/validateRecaptcha/validateRecaptcha.ts (1)
validateRecaptcha(1-4)src/setup/updateEnvFile/updateEnvFile.ts (1)
updateEnvFile(18-61)
🔇 Additional comments (10)
src/setup/setup.ts (1)
12-25: LGTM! Robust error message extraction helper.The
getErrorMessagehelper properly handles all error types:Errorinstances, strings, JSON-serializable objects, and falls back toString()for circular references or non-serializable values.src/setup/setup.spec.ts (9)
1-14: LGTM! Proper imports and type usage.All necessary imports are present, including the
MockInstancetype from Vitest for proper spy typing.
37-77: LGTM! Proper test isolation and cleanup.The test setup correctly:
- Uses
vi.clearAllMocks()inbeforeEach(per learnings)- Uses both
vi.clearAllMocks()andvi.resetAllMocks()inafterEach- Properly restores spies with
mockRestore()- Mocks
process.exitto throw for assertion purposes
79-101: LGTM! Comprehensive happy-path test for main().The test properly verifies the complete setup flow including all expected function calls and environment variable updates.
140-146: LGTM! Critical test for early exit behavior.This test verifies the PR objective that
main()returns early whencheckEnvFile()returns false, without triggering subsequent setup steps.
301-340: LGTM! Thorough reCAPTCHA function tests.Tests cover both opt-in (with valid key) and opt-out scenarios, verifying correct environment variable updates.
546-570: Acceptable use ofanyfor circular reference test.The
anytype is used to create a circular reference object for testing thegetErrorMessagefallback path. This is acceptable in test code where creating such structures requires bypassing TypeScript's type system.
593-604: Acceptable use ofanyfor circular reference test (duplicate pattern).Same pattern as the previous circular reference test - acceptable for testing edge cases.
637-691: LGTM! Comprehensive askAndSetLogErrors tests.Tests properly cover:
- User opting in and out
- Error propagation from
inquirer.prompt- Error propagation from
updateEnvFile- Prompt configuration verification
693-714: LGTM! Critical test verifying no side effects on import.This test directly validates the PR objective of making module execution conditional. It confirms that importing the module does not trigger any setup functions.
src/setup/setup.spec.ts
Outdated
| // Define proper types for inquirer prompts | ||
| interface PromptQuestion { | ||
| type: string; | ||
| name: string; | ||
| message: string; | ||
| default?: boolean | string | number; | ||
| validate?: (input: string) => boolean | string; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# Check inquirer version in package.json
grep -A 2 '"inquirer"' package.jsonRepository: PalisadoesFoundation/talawa-admin
Length of output: 154
🏁 Script executed:
# Read the setup.spec.ts file to see the context
cat -n src/setup/setup.spec.ts | head -50Repository: PalisadoesFoundation/talawa-admin
Length of output: 2429
🌐 Web query:
inquirer v13 TypeScript types PromptQuestion prompt interface
💡 Result:
Short answer — in Inquirer v13 the prompt/question types are provided by the package type declarations (split across @inquirer/* packages). Use the Questions / Question types (and the per-prompt question subtypes) exported in the type files. For example the old DefinitelyTyped/@types/inquirer definitions expose:
- Questions and Question (QuestionCommon + per-type options). [1]
- Many projects use a union alias like PromptQuestion = CheckboxQuestion | ListQuestion | … | InputQuestion (a union of the per-prompt Question subtypes). [2]
If you need the exact .d.ts sources to inspect or import types:
- @inquirer/prompts package exports its types in dist/*.d.ts (see package.json pointing to the types file). [3]
- The classic @types/inquirer index.d.ts shows Questions, Question and all per-prompt question interfaces you can reuse. Use import { Questions, Question, Answers } from 'inquirer' (or inspect the .d.ts) when typing your prompts. [1]
References:
[1] @types/inquirer index.d.ts (Questions / Question / per-prompt interfaces).
[2] Example union alias usage in community typings (PromptQuestion = inquirer.Question | inquirer.CheckboxQuestion | …).
[3] @inquirer/prompts package.json (points to dist .d.ts files).
Inquirer v13 does export TypeScript types; consider using them for consistency.
Inquirer v13 exports Question<T>, Questions<T>, and per-prompt question subtypes that can be imported directly (e.g., import type { Question } from 'inquirer'). However, the local PromptQuestion interface is appropriate for this test file since it provides a simplified, focused type tailored to the specific use case.
🤖 Prompt for AI Agents
In @src/setup/setup.spec.ts around lines 15 - 22, Replace the local
PromptQuestion interface with the official Inquirer types: import type {
Question } from 'inquirer' and use Question (or Questions) where PromptQuestion
is referenced; remove the custom PromptQuestion declaration and update any
variable annotations to Question<string> (or Questions<string>) so the test uses
the library-provided types for consistency.
Signed-off-by: Yuvraj Kolkar <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/setup/setup.ts (2)
27-68: Add TSDoc documentation for the exported function.Per coding guidelines §4, all exported functions require TSDoc comments. Based on learnings, avoid curly braces in
@throwsand@returns.📝 Suggested TSDoc
-// Ask and set up reCAPTCHA +/** + * Prompts the user to configure reCAPTCHA settings and updates the .env file. + * @throws Error if the prompt fails or reCAPTCHA setup encounters an error + * @returns Promise<void> + */ export const askAndSetRecaptcha = async (): Promise<void> => {
83-117: Add TSDoc and conditional execution guard for CLI entry point.
- Add proper JSDoc to the exported
main()function (currently has only a comment, not JSDoc format):-// Main function to run the setup process +/** + * Main entry point for the Talawa Admin setup process. + * Orchestrates environment checks, backups, and configuration prompts. + * @returns Promise<void> + */ export async function main(): Promise<void> {
- Add conditional execution guard at the end of
setup.tsto prevent automatic execution during imports:if (require.main === module) { main().catch((error) => { console.error('Fatal error:', error); process.exit(1); }); }This ensures the setup only runs when the script is executed directly (via
pnpm setup), not when imported as a module elsewhere.
🤖 Fix all issues with AI agents
In @src/setup/setup.spec.ts:
- Around line 1-116: The file currently contains implementation code instead of
tests; replace its contents with proper Vitest tests that import the module
under test (import { askAndSetRecaptcha, askAndSetLogErrors, main } from
'./setup'), import Vitest utilities (describe, it/test, expect, vi, beforeEach,
afterEach), mock external deps like inquirer and updateEnvFile
(vi.mock('inquirer'); vi.mock('./updateEnvFile/updateEnvFile'); and any others
used by main such as checkEnvFile/backupEnvFile/askAndSetDockerOption), and
implement test suites using describe blocks for askAndSetRecaptcha,
askAndSetLogErrors and main that assert behavior for success, error and edge
cases by stubbing inquirer.prompt results and throwing errors; ensure test
isolation by calling vi.clearAllMocks() in afterEach; remove the implementation
functions getErrorMessage/askAndSetRecaptcha/askAndSetLogErrors/main from this
file (they belong in setup.ts) so the spec only contains tests and mocks.
In @src/setup/setup.ts:
- Around line 70-81: askAndSetLogErrors is missing a local try/catch and TSDoc,
causing inconsistent error handling with askAndSetRecaptcha; add a TSDoc block
describing the function and wrap the prompt + updateEnvFile call in a try/catch
that logs a clear message (use the same logger used elsewhere, e.g.,
processLogger.error, or console.error if unavailable) including the caught
error, then rethrow the error to preserve propagation; ensure you reference
updateEnvFile in the comment and mirror the error-handling pattern used in
askAndSetRecaptcha for consistency.
- Around line 12-25: The helper getErrorMessage is currently internal; make it a
named export so tests and other modules can import it directly by changing its
declaration to a named export (e.g., export const getErrorMessage = (error:
unknown): string => { ... }) and ensure any existing imports in tests or other
files import { getErrorMessage } from this module; no behavior change needed
otherwise.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/setup/setup.spec.tssrc/setup/setup.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/setup/setup.tssrc/setup/setup.spec.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/setup/setup.tssrc/setup/setup.spec.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/setup/setup.spec.ts
🧠 Learnings (5)
📚 Learning: 2026-01-06T21:27:50.880Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6222
File: src/utils/userUpdateUtils.ts:22-22
Timestamp: 2026-01-06T21:27:50.880Z
Learning: In the Talawa Admin codebase, the i18n extraction tool can misclassify TypeScript generic syntax (e.g., (obj: T): Partial) as translatable content. Suppress these false positives by adding a // i18n-ignore-next-line comment directly above the line with the type declaration or generic function signature. Apply this only to TS code where misclassification is confirmed; avoid blanket suppression.
Applied to files:
src/setup/setup.tssrc/setup/setup.spec.ts
📚 Learning: 2026-01-07T14:21:19.711Z
Learnt from: cotishq
Repo: PalisadoesFoundation/talawa-admin PR: 6198
File: src/setup/setup.ts:123-124
Timestamp: 2026-01-07T14:21:19.711Z
Learning: Use TSDoc (eslint-plugin-tsdoc) instead of JSDoc. Do not wrap type names in curly braces in throws and returns. Write as "throws Error" and "returns Promise<void>" (no braces). Curly-brace usage causes tsdoc-malformed-inline-tag and tsdoc-escape-right-brace lint errors. Apply this rule to all TypeScript source files (e.g., src/**/*.ts) to ensure consistency.
Applied to files:
src/setup/setup.tssrc/setup/setup.spec.ts
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/setup/setup.spec.ts
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/setup/setup.spec.ts
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/setup/setup.spec.ts
🧬 Code graph analysis (2)
src/setup/setup.ts (3)
src/setup/setup.spec.ts (2)
askAndSetRecaptcha(28-67)askAndSetLogErrors(70-80)src/setup/updateEnvFile/updateEnvFile.ts (1)
updateEnvFile(18-61)src/setup/validateRecaptcha/validateRecaptcha.ts (1)
validateRecaptcha(1-4)
src/setup/setup.spec.ts (6)
src/setup/setup.ts (3)
askAndSetRecaptcha(28-68)askAndSetLogErrors(71-81)main(84-117)src/setup/updateEnvFile/updateEnvFile.ts (1)
updateEnvFile(18-61)src/setup/validateRecaptcha/validateRecaptcha.ts (1)
validateRecaptcha(1-4)src/setup/checkEnvFile/checkEnvFile.ts (2)
checkEnvFile(7-17)modifyEnvFile(19-27)src/setup/backupEnvFile/backupEnvFile.ts (1)
backupEnvFile(6-48)src/setup/askForDocker/askForDocker.ts (1)
askAndUpdateTalawaApiUrl(34-100)
|
Please fix the merge conflicts |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/setup/setup.ts (1)
113-113: Stale TSDoc: CTRL+C handling was removed.The TSDoc mentions "Can be cancelled with CTRL+C (exits with code 130)" but the AI summary indicates SIGINT handling was removed from this refactor. This documentation is now misleading and should be removed.
📝 Proposed fix
* If any step fails, exits with error code 1. - * Can be cancelled with CTRL+C (exits with code 130).
🤖 Fix all issues with AI agents
In @src/setup/setup.ts:
- Line 79: The call updateEnvFile(ENV_KEYS.RECAPTCHA_SITE_KEY, '') will throw
because ENV_KEYS is not defined; fix by either importing the ENV_KEYS constant
from its module or replacing ENV_KEYS.RECAPTCHA_SITE_KEY with the literal key
string used elsewhere (e.g., 'RECAPTCHA_SITE_KEY') so updateEnvFile is passed a
valid key; update the import list at the top of this file if you choose the
import approach and ensure consistency with other uses in this module.
- Around line 87-98: The exported function askAndSetLogErrors is missing TSDoc
and lacks the try/catch error handling used by askAndSetRecaptcha; add a TSDoc
block describing the function, params (none) and Promise<void> return, and wrap
the existing logic in try/catch that catches any error, augments it with context
(e.g., "askAndSetLogErrors: failed to set ALLOW_LOGS") and re-throws the error
so callers see consistent propagation; ensure updateEnvFile and inquirer.prompt
remain inside the try block and preserve the Promise<void> signature.
- Line 162: The file exports main() but never invokes it; add a conditional
execution guard so main() runs only when the module is executed directly by
wrapping a call to main() in if (require.main === module) { main(); } to satisfy
the PR objective and ensure pnpm setup (tsx src/setup/setup.ts) triggers the
interactive setup.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/setup/setup.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/setup/setup.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/setup/setup.ts
🧠 Learnings (2)
📚 Learning: 2026-01-06T21:27:50.880Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6222
File: src/utils/userUpdateUtils.ts:22-22
Timestamp: 2026-01-06T21:27:50.880Z
Learning: In the Talawa Admin codebase, the i18n extraction tool can misclassify TypeScript generic syntax (e.g., (obj: T): Partial) as translatable content. Suppress these false positives by adding a // i18n-ignore-next-line comment directly above the line with the type declaration or generic function signature. Apply this only to TS code where misclassification is confirmed; avoid blanket suppression.
Applied to files:
src/setup/setup.ts
📚 Learning: 2026-01-07T14:21:19.711Z
Learnt from: cotishq
Repo: PalisadoesFoundation/talawa-admin PR: 6198
File: src/setup/setup.ts:123-124
Timestamp: 2026-01-07T14:21:19.711Z
Learning: Use TSDoc (eslint-plugin-tsdoc) instead of JSDoc. Do not wrap type names in curly braces in throws and returns. Write as "throws Error" and "returns Promise<void>" (no braces). Curly-brace usage causes tsdoc-malformed-inline-tag and tsdoc-escape-right-brace lint errors. Apply this rule to all TypeScript source files (e.g., src/**/*.ts) to ensure consistency.
Applied to files:
src/setup/setup.ts
🧬 Code graph analysis (1)
src/setup/setup.ts (3)
src/setup/updateEnvFile/updateEnvFile.ts (1)
updateEnvFile(18-61)src/setup/validateRecaptcha/validateRecaptcha.ts (1)
validateRecaptcha(1-4)src/setup/setup.spec.ts (1)
askAndSetLogErrors(70-80)
🔇 Additional comments (1)
src/setup/setup.ts (1)
12-25: LGTM - Defensive error message extraction.The helper correctly handles multiple error shapes (Error instances, strings, objects) with a fallback for circular references. Good defensive coding.
src/setup/setup.ts
Outdated
| updateEnvFile(ENV_KEYS.RECAPTCHA_SITE_KEY, recaptchaSiteKeyInput); | ||
| updateEnvFile('REACT_APP_RECAPTCHA_SITE_KEY', siteKeyAnswers.recaptchaSiteKeyInput); | ||
| } else { | ||
| updateEnvFile(ENV_KEYS.RECAPTCHA_SITE_KEY, ''); |
There was a problem hiding this comment.
ENV_KEYS is undefined - this will cause a ReferenceError at runtime.
ENV_KEYS is not imported or defined in this file. Lines 56 and 77 correctly use string literals, but line 79 references an undefined constant. This will crash when the user chooses not to use reCAPTCHA.
🐛 Proposed fix
} else {
- updateEnvFile(ENV_KEYS.RECAPTCHA_SITE_KEY, '');
+ updateEnvFile('REACT_APP_RECAPTCHA_SITE_KEY', '');
}🤖 Prompt for AI Agents
In @src/setup/setup.ts at line 79, The call
updateEnvFile(ENV_KEYS.RECAPTCHA_SITE_KEY, '') will throw because ENV_KEYS is
not defined; fix by either importing the ENV_KEYS constant from its module or
replacing ENV_KEYS.RECAPTCHA_SITE_KEY with the literal key string used elsewhere
(e.g., 'RECAPTCHA_SITE_KEY') so updateEnvFile is passed a valid key; update the
import list at the top of this file if you choose the import approach and ensure
consistency with other uses in this module.
| } | ||
|
|
||
| main(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Reading src/setup/setup.ts (end of file) ==="
tail -30 src/setup/setup.ts
echo ""
echo "=== Full file length ==="
wc -l src/setup/setup.ts
echo ""
echo "=== Searching for imports of setup module and main calls ==="
rg -n "from.*setup/setup|from.*setup\.ts|import.*setup" --type ts --type tsx -g '!*.spec.ts' -g '!*.test.ts' | head -20
echo ""
echo "=== Searching for calls to main() or setup.main() ==="
rg -n "setup\.main\(\)|\.main\(\)" --type ts --type tsx -g '!*.spec.ts' -g '!*.test.ts' | head -20
echo ""
echo "=== Checking package.json scripts ==="
grep -A 30 '"scripts"' package.json | grep -i "setup"
echo ""
echo "=== Checking bin or entry points in package.json ==="
grep -E '"bin"|"main"|"entry"' package.jsonRepository: PalisadoesFoundation/talawa-admin
Length of output: 1332
🏁 Script executed:
#!/bin/bash
echo "=== Reading FULL src/setup/setup.ts ==="
cat -n src/setup/setup.ts | tail -40
echo ""
echo "=== Searching for any invocation of main at module level ==="
rg "main\(" src/setup/setup.ts -n
echo ""
echo "=== Checking if require.main pattern exists ==="
rg "require\.main" src/setup/setup.ts -nRepository: PalisadoesFoundation/talawa-admin
Length of output: 1664
🏁 Script executed:
#!/bin/bash
echo "=== Checking file beginning - imports and module type ==="
head -30 src/setup/setup.ts
echo ""
echo "=== Checking for any conditional at file end ==="
tail -5 src/setup/setup.tsRepository: PalisadoesFoundation/talawa-admin
Length of output: 1315
🏁 Script executed:
#!/bin/bash
echo "=== Complete file to verify structure ==="
cat -n src/setup/setup.ts
echo ""
echo "=== Check if setup module is imported anywhere in entry points ==="
fd --type f '\.(ts|tsx|js|mjs)$' src --exec grep -l "from.*setup/setup\|require.*setup" {} \;
echo ""
echo "=== Check index.ts or other possible entry points ==="
cat src/index.ts 2>/dev/null || echo "No src/index.ts"Repository: PalisadoesFoundation/talawa-admin
Length of output: 6563
🏁 Script executed:
#!/bin/bash
echo "=== Check windows.ts for setup import ==="
rg "setup" src/install/os/windows.ts -n -B 2 -A 2
echo ""
echo "=== Check if main is called from install files ==="
rg "main\(\)" src/install/os/windows.ts src/install/os/macos.ts src/install/os/linux.ts -n -B 1 -A 1Repository: PalisadoesFoundation/talawa-admin
Length of output: 475
🏁 Script executed:
#!/bin/bash
echo "=== Search for actual import of setup/setup module ==="
rg "from.*setup/setup|import.*setup.*setup" src --type ts --type tsx
echo ""
echo "=== Search in install directory structure ==="
ls -la src/install/
echo ""
echo "=== Search for any file that imports or calls main from setup ==="
rg "setup/setup|setup\.main" src --type ts --type tsx -B 2 -A 2
echo ""
echo "=== Check if setup.ts or setup.js is referenced as entry point anywhere ==="
grep -r "setup\.ts\|setup\.js" src/ package.json 2>/dev/null | grep -v node_modules | head -20Repository: PalisadoesFoundation/talawa-admin
Length of output: 1014
🏁 Script executed:
#!/bin/bash
echo "=== Check if there's a separate invocation pattern or wrapper ==="
find src -name "*.ts" -type f -exec grep -l "if.*require\.main\|if.*import\.meta" {} \;
echo ""
echo "=== Final verification - what happens when tsx runs setup.ts ==="
echo "When 'tsx src/setup/setup.ts' executes:"
echo "- Module loads, all exports defined"
echo "- main() function exported but NOT called"
echo "- Script ends with no output or action"
echo ""
echo "Checking if any recent commit added the guard:"
git log --oneline -20 -- src/setup/setup.ts 2>/dev/null || echo "Git not available"Repository: PalisadoesFoundation/talawa-admin
Length of output: 482
Missing conditional execution guard - PR objective not implemented.
The file ends at line 162 with the main() function exported but never invoked. The PR requires guarding main() with if (require.main === module) to conditionally execute the setup when run directly. Without this guard:
pnpm setup(which runstsx src/setup/setup.ts) loads the module but never callsmain()- The setup script is non-functional—users receive no prompts and no configuration occurs
- The PR's primary objective stated in the title "Fix: Make setup.ts module execution conditional" is not fulfilled
Add the conditional invocation at the file end:
Proposed fix
export async function main(): Promise<void> {
try {
if (!checkEnvFile()) {
return;
}
// ... existing implementation
} catch (error) {
console.error('\n❌ Setup failed:', error);
console.log('\nPlease try again or contact support if the issue persists.');
process.exit(1);
}
}
+
+// Only run main when executed directly, not when imported
+if (require.main === module) {
+ main();
+}🤖 Prompt for AI Agents
In @src/setup/setup.ts at line 162, The file exports main() but never invokes
it; add a conditional execution guard so main() runs only when the module is
executed directly by wrapping a call to main() in if (require.main === module) {
main(); } to satisfy the PR objective and ensure pnpm setup (tsx
src/setup/setup.ts) triggers the interactive setup.
|
Please:
|
|
Hi @palisadoes , I’m working on the feedback and addressing the issues raised by CodeRabbit. I’ll update you once the fixes are completed. |
Signed-off-by: Yuvraj Kolkar <[email protected]>
yuvraj-kolkar17
force-pushed
the
fix-setup-conditional-execution
branch
from
81bf307 to
2a25913
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/setup/setup.ts (2)
33-78: Add TSDoc comment foraskAndSetRecaptcha.Good improvements with explicit typing for inquirer prompts and enhanced validation error handling via
getErrorMessage. Missing required TSDoc documentation.📝 Suggested TSDoc
-// Ask and set up reCAPTCHA -export const askAndSetRecaptcha = async (): Promise<void> => { +/** + * Prompts user to configure reCAPTCHA settings and updates the .env file. + * @throws Error if reCAPTCHA setup fails + */ +export const askAndSetRecaptcha = async (): Promise<void> => {Based on learnings, use TSDoc format—write "throws Error" without braces.
100-134: Missing conditional execution guard — primary PR objective not fulfilled.The PR objective explicitly requires: "Ensure src/setup/setup.ts does not execute on import by invoking main() only when run directly (guard with
if (require.main === module))."The file currently only defines and exports
main()but never invokes it conditionally. Without this guard, when the script is run directly vianode setup.tsorts-node setup.ts,main()will not execute.Additionally, add TSDoc for the
mainfunction.🐛 Proposed fix to add conditional execution guard
+/** + * Main entry point for the Talawa Admin setup process. + * @throws Error and exits with code 1 if setup fails + */ export async function main(): Promise<void> { try { // ... existing code } catch (error) { console.error('\n❌ Setup failed:', error); console.log('\nPlease try again or contact support if the issue persists.'); process.exit(1); } } + +// Only run main when script is executed directly, not when imported +if (require.main === module) { + main(); +}
🤖 Fix all issues with AI agents
In @src/setup/setup.spec.ts:
- Around line 35-62: Remove the unnecessary "as any" cast on the fs.readFileSync
mock: change vi.mocked(fs.readFileSync).mockReturnValue('USE_DOCKER=NO' as any)
to a properly typed return such as
vi.mocked(fs.readFileSync).mockReturnValue('USE_DOCKER=NO') (or, if TypeScript
complains, use 'USE_DOCKER=NO' as unknown as string) so the mock matches the
expected string return type of readFileSync without using any.
In @src/setup/setup.ts:
- Around line 80-98: The function askAndSetLogErrors is missing a required TSDoc
comment; add a TSDoc block immediately above the exported function describing
its purpose (prompts user to enable console logging for compile/runtime errors),
the returned Promise<void>, and parameters/behavior (none, it updates ALLOW_LOGS
via updateEnvFile based on the inquirer prompt), and mention possible thrown
errors (wraps errors and uses getErrorMessage); reference the function name
askAndSetLogErrors and related symbols updateEnvFile, inquirer.prompt, and
getErrorMessage in the TSDoc to aid readers.
- Around line 12-31: Add a TSDoc block for the exported function getErrorMessage
describing its purpose, parameters, and return value: document that it accepts
an unknown error and returns a string representation, list the parameter "error"
with its type described in plain text, and describe the returned string values
for common cases (Error, string, undefined, null, and fallback JSON/string
conversion); place the TSDoc immediately above the getErrorMessage declaration
following project TSDoc style (no curly-braced types).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/setup/setup.spec.tssrc/setup/setup.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/setup/setup.tssrc/setup/setup.spec.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/setup/setup.tssrc/setup/setup.spec.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/setup/setup.spec.ts
🧠 Learnings (5)
📚 Learning: 2026-01-06T21:27:50.880Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6222
File: src/utils/userUpdateUtils.ts:22-22
Timestamp: 2026-01-06T21:27:50.880Z
Learning: In the Talawa Admin codebase, the i18n extraction tool can misclassify TypeScript generic syntax (e.g., (obj: T): Partial) as translatable content. Suppress these false positives by adding a // i18n-ignore-next-line comment directly above the line with the type declaration or generic function signature. Apply this only to TS code where misclassification is confirmed; avoid blanket suppression.
Applied to files:
src/setup/setup.tssrc/setup/setup.spec.ts
📚 Learning: 2026-01-07T14:21:19.711Z
Learnt from: cotishq
Repo: PalisadoesFoundation/talawa-admin PR: 6198
File: src/setup/setup.ts:123-124
Timestamp: 2026-01-07T14:21:19.711Z
Learning: Use TSDoc (eslint-plugin-tsdoc) instead of JSDoc. Do not wrap type names in curly braces in throws and returns. Write as "throws Error" and "returns Promise<void>" (no braces). Curly-brace usage causes tsdoc-malformed-inline-tag and tsdoc-escape-right-brace lint errors. Apply this rule to all TypeScript source files (e.g., src/**/*.ts) to ensure consistency.
Applied to files:
src/setup/setup.tssrc/setup/setup.spec.ts
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/setup/setup.spec.ts
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/setup/setup.spec.ts
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/setup/setup.spec.ts
🧬 Code graph analysis (1)
src/setup/setup.ts (2)
src/setup/validateRecaptcha/validateRecaptcha.ts (1)
validateRecaptcha(1-4)src/setup/updateEnvFile/updateEnvFile.ts (1)
updateEnvFile(18-61)
🔇 Additional comments (11)
src/setup/setup.spec.ts (11)
1-14: Imports and mocks setup looks good.The test file correctly imports all necessary dependencies and mocks external modules. The use of
MockInstancetype from Vitest is appropriate.
64-70: Proper cleanup in afterEach block.Good practice to call both
vi.clearAllMocks()andvi.resetAllMocks(), followed by restoring spies. This ensures test isolation.Based on learnings, the explicit
vi.clearAllMocks()in afterEach satisfies ESLint/Vitest requirements.
72-166: Comprehensive main function test coverage.Tests cover:
- Success with default options
- Docker mode variations
- Error logging opt-in
- Error handling with process.exit(1)
- Early return when checkEnvFile returns false
- Welcome/success messages
- Error support messages
167-291: Good coverage of main function error paths.Tests exercise error handling for:
- backupEnvFile failures
- modifyEnvFile failures
- askAndUpdatePort failures
- askAndUpdateTalawaApiUrl failures
- askAndSetRecaptcha failures in main flow
- askAndSetLogErrors failures in main flow
Full Docker=NO and Docker=YES flows are also tested with all options enabled.
294-438: Thorough askAndSetRecaptcha test coverage.Tests cover:
- User opt-in with valid key
- User opt-out
- Error propagation with helpful messages
- Validation function behavior (valid/invalid keys)
- Second prompt errors
- updateEnvFile errors
460-549: Excellent edge case coverage for validation function.Tests exercise:
- Validation throwing Error instances
- Non-Error thrown objects (strings)
- Plain objects with properties
- Circular reference objects
This thoroughly tests the
getErrorMessageintegration within the validation function.
551-614: Good coverage of various error types in catch block.Tests verify that
askAndSetRecaptchacorrectly handles:
- String errors
- Object errors
- Circular references
- Numbers
- null
- undefined
This ensures
getErrorMessageis properly integrated.
616-680: Complete askAndSetLogErrors test coverage.Tests cover:
- User opt-in and opt-out scenarios
- Error handling during prompt
- updateEnvFile error handling
- Correct prompt configuration
682-721: Comprehensive getErrorMessage unit tests.Tests verify handling of:
- Error instances
- Strings
- Objects (JSON serialization)
- Numbers
- null and undefined
- Circular references (String fallback)
- Arrays
749-792: Good edge case and integration scenario coverage.Tests handle:
- Empty env config
- Malformed env file content
- Case-sensitive Docker flag (lowercase "yes" treated as false)
This validates robustness of the configuration parsing.
723-747: Add test for conditional module execution once guard is implemented.The test correctly verifies that importing the module does not trigger side effects. When you add the
if (require.main === module) { main(); }guard tosetup.tsto enable direct script execution, add a corresponding test that validates the guard is in place and thatmain()is invoked when the script runs directly.
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
|
|
||
| // default flow: env file check passes | ||
| vi.mocked(checkEnvFile).mockReturnValue(true); | ||
|
|
||
| // default fs content says NO docker | ||
| vi.mocked(fs.promises.readFile).mockResolvedValue('USE_DOCKER=NO'); | ||
| vi.mocked(fs.readFileSync).mockReturnValue('USE_DOCKER=NO' as any); | ||
| vi.mocked(dotenv.parse).mockReturnValue({ USE_DOCKER: 'NO' }); | ||
|
|
||
| // mock external functions to resolve normally | ||
| vi.mocked(askAndSetDockerOption).mockResolvedValue(undefined); | ||
| vi.mocked(askAndUpdatePort).mockResolvedValue(undefined); | ||
| vi.mocked(askAndUpdateTalawaApiUrl).mockResolvedValue(undefined); | ||
| vi.mocked(modifyEnvFile).mockImplementation(() => undefined); | ||
| vi.mocked(updateEnvFile).mockImplementation(() => undefined); | ||
| vi.mocked(validateRecaptcha).mockReturnValue(true); | ||
| vi.mocked(backupEnvFile).mockResolvedValue(undefined); | ||
|
|
||
| // default process.exit spy (we won't throw unless a test needs it) | ||
| vi.spyOn(process, 'exit').mockImplementation(() => { | ||
| // keep as noop for normal tests | ||
| return undefined as never; | ||
| }); | ||
| processExitSpy = vi.spyOn(process, 'exit').mockImplementation((( | ||
| code?: number, | ||
| ) => { | ||
| throw new Error(`process.exit called with code ${code}`); | ||
| }) as never); | ||
|
|
||
| consoleErrorSpy = vi | ||
| .spyOn(console, 'error') | ||
| .mockImplementation(() => undefined); | ||
|
|
||
| vi.spyOn(console, 'error').mockImplementation(() => undefined); | ||
| vi.spyOn(console, 'log').mockImplementation(() => undefined); | ||
| consoleLogSpy = vi | ||
| .spyOn(console, 'log') | ||
| .mockImplementation(() => undefined); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Test setup properly initializes mocks and spies.
The beforeEach block correctly:
- Clears all mocks
- Sets up default mock implementations
- Spies on
process.exit,console.error, andconsole.log
Minor improvement: The as any cast on line 39 could be avoided.
♻️ Optional: Improve type safety for fs.readFileSync mock
- vi.mocked(fs.readFileSync).mockReturnValue('USE_DOCKER=NO' as any);
+ vi.mocked(fs.readFileSync).mockReturnValue(Buffer.from('USE_DOCKER=NO'));Alternatively, if the code expects a string, use a more explicit assertion or adjust the mock signature.
🤖 Prompt for AI Agents
In @src/setup/setup.spec.ts around lines 35 - 62, Remove the unnecessary "as
any" cast on the fs.readFileSync mock: change
vi.mocked(fs.readFileSync).mockReturnValue('USE_DOCKER=NO' as any) to a properly
typed return such as vi.mocked(fs.readFileSync).mockReturnValue('USE_DOCKER=NO')
(or, if TypeScript complains, use 'USE_DOCKER=NO' as unknown as string) so the
mock matches the expected string return type of readFileSync without using any.
| // Helper to extract error message safely | ||
| export const getErrorMessage = (error: unknown): string => { | ||
| if (error instanceof Error) { | ||
| return error.message; | ||
| } | ||
| if (typeof error === 'string') { | ||
| return error; | ||
| } | ||
| if (error === undefined) { | ||
| return 'undefined'; | ||
| } | ||
| if (error === null) { | ||
| return 'null'; | ||
| } | ||
| try { | ||
| return JSON.stringify(error); | ||
| } catch { | ||
| return String(error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add TSDoc comment for exported utility function.
Per coding guidelines, all exported functions must have TSDoc comments. The getErrorMessage function is well-implemented but missing documentation.
📝 Suggested TSDoc
-// Helper to extract error message safely
-export const getErrorMessage = (error: unknown): string => {
+/**
+ * Extracts a human-readable message from an unknown error type.
+ * @param error - The error value to extract a message from
+ * @returns A string representation of the error
+ */
+export const getErrorMessage = (error: unknown): string => {Based on learnings, use TSDoc format without curly braces around types.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Helper to extract error message safely | |
| export const getErrorMessage = (error: unknown): string => { | |
| if (error instanceof Error) { | |
| return error.message; | |
| } | |
| if (typeof error === 'string') { | |
| return error; | |
| } | |
| if (error === undefined) { | |
| return 'undefined'; | |
| } | |
| if (error === null) { | |
| return 'null'; | |
| } | |
| try { | |
| return JSON.stringify(error); | |
| } catch { | |
| return String(error); | |
| } | |
| }; | |
| /** | |
| * Extracts a human-readable message from an unknown error type. | |
| * @param error - The error value to extract a message from | |
| * @returns A string representation of the error | |
| */ | |
| export const getErrorMessage = (error: unknown): string => { | |
| if (error instanceof Error) { | |
| return error.message; | |
| } | |
| if (typeof error === 'string') { | |
| return error; | |
| } | |
| if (error === undefined) { | |
| return 'undefined'; | |
| } | |
| if (error === null) { | |
| return 'null'; | |
| } | |
| try { | |
| return JSON.stringify(error); | |
| } catch { | |
| return String(error); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In @src/setup/setup.ts around lines 12 - 31, Add a TSDoc block for the exported
function getErrorMessage describing its purpose, parameters, and return value:
document that it accepts an unknown error and returns a string representation,
list the parameter "error" with its type described in plain text, and describe
the returned string values for common cases (Error, string, undefined, null, and
fallback JSON/string conversion); place the TSDoc immediately above the
getErrorMessage declaration following project TSDoc style (no curly-braced
types).
| // Ask and set up logging errors in the console | ||
| const askAndSetLogErrors = async (): Promise<void> => { | ||
| const { shouldLogErrors } = await inquirer.prompt({ | ||
| type: 'confirm', | ||
| name: 'shouldLogErrors', | ||
| message: | ||
| 'Would you like to log Compiletime and Runtime errors in the console?', | ||
| default: true, | ||
| }); | ||
|
|
||
| updateEnvFile('ALLOW_LOGS', shouldLogErrors ? 'YES' : 'NO'); | ||
| export const askAndSetLogErrors = async (): Promise<void> => { | ||
| try { | ||
| const { shouldLogErrors } = await inquirer.prompt<{ | ||
| shouldLogErrors: boolean; | ||
| }>({ | ||
| type: 'confirm', | ||
| name: 'shouldLogErrors', | ||
| message: | ||
| 'Would you like to log Compiletime and Runtime errors in the console?', | ||
| default: true, | ||
| }); | ||
|
|
||
| updateEnvFile('ALLOW_LOGS', shouldLogErrors ? 'YES' : 'NO'); | ||
| } catch (error) { | ||
| console.error('Error setting up logging:', error); | ||
| throw new Error(`Failed to set up logging: ${getErrorMessage(error)}`); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add TSDoc comment for askAndSetLogErrors.
The function is now correctly exported as required by the PR objectives. Error handling with getErrorMessage is consistent. Missing required TSDoc documentation.
📝 Suggested TSDoc
-// Ask and set up logging errors in the console
-export const askAndSetLogErrors = async (): Promise<void> => {
+/**
+ * Prompts user to configure console error logging and updates the .env file.
+ * @throws Error if logging setup fails
+ */
+export const askAndSetLogErrors = async (): Promise<void> => {🤖 Prompt for AI Agents
In @src/setup/setup.ts around lines 80 - 98, The function askAndSetLogErrors is
missing a required TSDoc comment; add a TSDoc block immediately above the
exported function describing its purpose (prompts user to enable console logging
for compile/runtime errors), the returned Promise<void>, and parameters/behavior
(none, it updates ALLOW_LOGS via updateEnvFile based on the inquirer prompt),
and mention possible thrown errors (wraps errors and uses getErrorMessage);
reference the function name askAndSetLogErrors and related symbols
updateEnvFile, inquirer.prompt, and getErrorMessage in the TSDoc to aid readers.
2 participants
What kind of change does this PR introduce?
Setup file execution issue
Continue #6237
Issue Number:
Fixes #6184
Snapshots/Videos:
If relevant, did you update the documentation?
NA
Summary
This PR implements conditional execution for the setup.ts module to prevent automatic execution during testing.
Does this PR introduce a breaking change?
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.