feat: make seed-performance-testing script configurable from CLI#29006
feat: make seed-performance-testing script configurable from CLI#29006SinghaAnirban005 wants to merge 2 commits intocalcom:mainfrom
Conversation
|
Welcome to Cal.diy, @SinghaAnirban005! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
📝 WalkthroughWalkthroughThe 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/seed-performance-testing.ts`:
- Around line 212-223: main() is invoked without error handling or a clean
shutdown, so rejections become unhandled and the Prisma DB connection opened by
functions like createUserAndEventType (via seed-utils) can keep the process
hanging; wrap the main logic in a try/catch/finally (or add a top-level promise
chain) that catches errors from parseArgs(),
createManyDifferentUsersWithDifferentEventTypesAndBookings, and
createAUserWithManyBookings, logs the error and exits with a non-zero code on
failure, and in the finally block call the Prisma disconnect function provided
by your seed utilities (e.g., await prisma.$disconnect() or the seed-utils
disconnection helper) to ensure connections are closed and in-flight writes are
flushed before exiting.
- Around line 29-36: The parsed CLI integers bookingsPerEventType, startFrom and
tillUser (from get()) must be validated because parseInt can produce NaN; update
the block that defines bookingsPerEventType, startFrom and tillUser to check
each parsed value (e.g., Number.isInteger(...) && value >= 0 or
!Number.isNaN(...)) and if any is invalid, print a clear console.error including
the flag name and invalid value and call process.exit(1); keep the existing mode
validation but add this numeric-arg guard around the variables referenced later
(bookingsPerEventType, startFrom, tillUser) so loops and _numBookings never
receive NaN.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a8af9b9e-15bf-4bfa-b378-721dbd7f7614
📒 Files selected for processing (1)
scripts/seed-performance-testing.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/seed-performance-testing.ts (2)
19-32: Optional: deriveModefrom a single source of truth to keep the type and runtime check in sync.The
as Modecast on line 28 is immediately validated by the literal comparison on line 29, so it's safe today. However, the union type on line 19 and the runtime check on line 29 can drift independently if a new mode is added. Minor refactor:♻️ Proposed refactor
-type Mode = "many-bookings" | "many-users"; +const MODES = ["many-bookings", "many-users"] as const; +type Mode = (typeof MODES)[number]; @@ - const mode = (get("--mode") ?? "many-bookings") as Mode; - if (mode !== "many-bookings" && mode !== "many-users") { - console.error(`Invalid --mode "${mode}". Must be many-bookings or many-users.`); + const modeRaw = get("--mode") ?? "many-bookings"; + if (!MODES.includes(modeRaw as Mode)) { + console.error(`Invalid --mode "${modeRaw}". Must be one of: ${MODES.join(", ")}.`); process.exit(1); } + const mode = modeRaw as Mode;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/seed-performance-testing.ts` around lines 19 - 32, Replace the separate manual union type and runtime check with a single source-of-truth constant array and derive Mode from it: create a const (e.g., MODES) with the allowed mode strings (as const), change the type alias Mode to type Mode = typeof MODES[number], use MODES[0] as the default instead of the hardcoded "many-bookings", and validate by checking MODES.includes(mode) inside parseArgs; update parseArgs to remove the unsafe as Mode cast and rely on the derived type and includes-based runtime check to keep compile-time and runtime lists in sync.
4-7: Add a comment explaining CommonJS dependency of the dotenv-before-imports pattern.The
dotenv.config()call at the top of the file relies on CommonJS sequential import execution, which is guaranteed here becausets-nodeis configured with"module": "CommonJS"in packages/tsconfig/base.json. However, if this script is ever migrated to ESM (e.g., by changing the ts-node configuration), the import hoisting would silently break env-dependent imports likeseed-utils. A brief inline comment clarifying this dependency would prevent regressions during future refactors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/seed-performance-testing.ts` around lines 4 - 7, Add a brief inline comment above the dotenv.config({ path: ... }) call explaining that this placement relies on CommonJS sequential import execution (ts-node configured with "module": "CommonJS") so environment variables are available to subsequent imports (e.g., seed-utils), and warn that migrating to ESM or changing ts-node module settings will hoist imports and break env-dependent modules unless dotenv is initialized earlier or an alternative pattern is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/seed-performance-testing.ts`:
- Around line 19-32: Replace the separate manual union type and runtime check
with a single source-of-truth constant array and derive Mode from it: create a
const (e.g., MODES) with the allowed mode strings (as const), change the type
alias Mode to type Mode = typeof MODES[number], use MODES[0] as the default
instead of the hardcoded "many-bookings", and validate by checking
MODES.includes(mode) inside parseArgs; update parseArgs to remove the unsafe as
Mode cast and rely on the derived type and includes-based runtime check to keep
compile-time and runtime lists in sync.
- Around line 4-7: Add a brief inline comment above the dotenv.config({ path:
... }) call explaining that this placement relies on CommonJS sequential import
execution (ts-node configured with "module": "CommonJS") so environment
variables are available to subsequent imports (e.g., seed-utils), and warn that
migrating to ESM or changing ts-node module settings will hoist imports and
break env-dependent modules unless dotenv is initialized earlier or an
alternative pattern is used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e831401-cef6-4101-a5bd-8c1511a7dc4b
📒 Files selected for processing (1)
scripts/seed-performance-testing.ts
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
1 participant
What does this PR do?
Resolves the
TODO: Make it more structured and configurable from CLIinseed-performance-testing.ts.Video Demo (if applicable):
Screencast.from.2026-04-26.19-30-14.webm
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Run
docker-compose up -d databasecd into /scripts
Three modes are available
--bookings <n>— number of bookings per event type (default:100)--start-from <n>— start user index for many-users mode (default:0), enables resuming an interrupted run--till-user <n>— end user index for many-users mode (default:20)--mode many-bookings | many-users— replaces the previously commented-outcreateManyDifferentUserscallExamples