chore(core): fix simple engine test

[NathanFlurry]

No description provided.

[claude]

Claude finished @NathanFlurry's task —— View job

---

Code Review Complete ✅

Reviewed PR #1225: "chore(core): fix simple engine test"

Todo List:

• Read and analyze changed files
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Evaluate test coverage impact
• Provide constructive feedback

Review Summary

This PR makes simple but important configuration updates to align default port numbers and clean up test configuration.

✅ Positive Changes

1. Port Standardization: Updated default port from 7080 to 6420 in both config and test files, creating consistency across the codebase.

2. Code Simplification: Removed unnecessary addresses configuration and getPort() usage in tests, making the test setup cleaner and more straightforward.

3. Resource Allocation: Increased totalSlots from 100 to 1000 in tests, which may provide better test reliability under load.

📝 Code Quality Assessment

Good Practices Observed:

• Changes follow the repository's coding standards
• Consistent formatting and style
• Proper use of environment variable fallbacks
• Clean removal of unused imports and logic

Areas for Improvement:

• Missing PR Description: The PR has no description, making it harder to understand the reasoning behind these changes
• Magic Numbers: The port 6420 and slot count 1000 could benefit from being defined as named constants for better maintainability

🔍 Technical Analysis

Config Changes (packages/rivetkit/src/drivers/engine/config.ts:11):

- () => getEnvUniversal("RIVET_ENGINE") ?? "http://localhost:7080",
+ () => getEnvUniversal("RIVET_ENGINE") ?? "http://localhost:6420",

This change aligns the default port with what appears to be a project-wide standard.

Test Changes (packages/rivetkit/tests/driver-engine.test.ts:21,40-48):

• Simplified endpoint configuration
• Removed unused port allocation logic
• Removed complex addresses configuration object
• Increased test slot allocation

🚀 Performance Considerations

• Positive: Removing the getPort() call eliminates an async operation during test setup
• Positive: Higher slot allocation (1000 vs 100) may improve test stability under concurrent load

🔒 Security Assessment

• No security concerns identified: Changes are configuration-only and don't introduce any security vulnerabilities
• Environment variable usage remains properly implemented

🧪 Test Coverage Impact

• Maintains coverage: The test functionality remains intact while simplifying configuration
• Improved reliability: Simplified setup should make tests more predictable and less prone to port conflicts

📋 Recommendations

1. Add PR Description: Consider adding a brief description explaining the port standardization and configuration cleanup
2. Consider Constants: For future maintenance, consider defining default ports as named constants
3. Documentation: If this port change affects deployment or local development setup, ensure documentation is updated accordingly

Overall Assessment: ✅ APPROVED

This is a clean, well-executed chore that improves consistency and simplifies test configuration. The changes are minimal, focused, and follow good practices.

---

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• fix(core): fix websockets #1229
• fix(inspector): make inspector work again #1228
• chore(core): remove manager driver #1227
• fix(core): fix issue deserializing WebSocket message with BARE #1226
• chore(core): fix simple engine test #1225 👈 (View in Graphite)
• chore(core): simplify engine keys #1224
• chore(core): remove engine runner addresses config #1223
• chore(core): add welcome message #1222
• chore(fs): remove unneeded warning for onRunnerStart #1221
• chore(core): add readme #1220
• chore(core): add logger to engine runner #1219
• chore(core): update engine runner #1218
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• merge queue - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[pkg-pr-new]

More templates

• example-ai-agent
• example-better-auth-external-db
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• counter
• example-crdt
• example-database
• example-sqlite
• example-elysia
• example-express
• example-game
• example-hono
• example-hono-react
• example-next-js
• example-rate
• example-raw-fetch-handler
• example-raw-websocket-handler-proxy
• example-react
• starter
• example-stream
• example-sync
• example-tenant
• example-trpc

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/actor@1225

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/cloudflare-workers@1225

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/core@1225

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/db@1225

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/framework-base@1225

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/next-js@1225

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/react@1225

rivetkit

pnpm add https://pkg.pr.new/rivet-gg/rivetkit@1225

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/sql-loader@1225

commit: 2542084