✅ server: use injected account factory in tests - #621
Conversation
🦋 Changeset detectedLatest commit: fd9b7a8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughReplaces hardcoded factory addresses with dependency-injected Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @mainqueg, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the server-side test suite to enhance its realism and maintainability. It transitions from using a static Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
@cursor review |
PR SummaryAligns tests with injected deployment addresses to improve consistency and environment independence.
Written by Cursor Bugbot for commit fd9b7a8. This will update automatically on new commits. Configure here. |
There was a problem hiding this comment.
Code Review
This pull request is a nice improvement, refactoring tests to use an injected ExaAccountFactory address instead of a hardcoded zeroAddress. This enhances test maintainability and realism. My feedback focuses on ensuring consistent application of this new pattern across all updated test files. Specifically, I've suggested wrapping the injected value with parse(Address, ...) for validation in all instances, mirroring the approach in auth.test.ts, to improve the robustness of the tests.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @.changeset/loud-apples-double.md:
- Line 5: Remove the leading emoji from the changeset summary "✅ use injected
account factory in tests" and rewrite it in lowercase imperative present tense
(e.g., start with a verb and keep all words lowercase) so the summary follows
the project's changeset guidelines; update the line in
.changeset/loud-apples-double.md accordingly.
In @server/test/api/auth.test.ts:
- Line 28: This is an inconsistency: either remove the extra validation here or
add it everywhere; to keep tests consistent with panda.test.ts and
persona.test.ts, remove the parse(Address, ...) wrapper in auth.test.ts by
replacing the factory assignment that uses parse(Address,
inject("ExaAccountFactory")) with just inject("ExaAccountFactory"); if you
prefer to enforce type-safety project-wide instead, apply the same
parse(Address, inject(...)) pattern to the factory usages in panda.test.ts and
persona.test.ts (and any other tests that call inject for ExaAccountFactory) so
all tests are uniform.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.changeset/loud-apples-double.mdserver/test/api/auth.test.tsserver/test/api/card.test.tsserver/test/hooks/panda.test.tsserver/test/hooks/persona.test.tsserver/test/mocks/deployments.ts
🧰 Additional context used
📓 Path-based instructions (7)
server/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/server.mdc)
server/**/*.ts: Usec.varobject to pass strongly-typed data between Hono middleware and route handlers; do not usec.set
All request validation (headers, body, params) must be handled by@hono/valibot-validatormiddleware; do not perform manual validation inside route handlers
Use Hono's built-in error handling by throwingnew HTTPException()for expected errors; unhandled errors will be caught and logged automatically
Enforce Node.js best practices using ESLintplugin:n/recommendedconfiguration
Enforce Drizzle ORM best practices using ESLintplugin:drizzle/allconfiguration, including requiringwhereclauses forupdateanddeleteoperations
Use Drizzle ORM query builder for all database interactions; do not write raw SQL queries unless absolutely unavoidable
All authentication and authorization logic must be implemented in Hono middleware
Do not accessprocess.envdirectly in application code; load all configuration and secrets once at startup and pass them through dependency injection or context
Avoid long-running, synchronous operations; useasync/awaitcorrectly and be mindful of CPU-intensive tasks to prevent blocking the event loop
Files:
server/test/api/auth.test.tsserver/test/hooks/panda.test.tsserver/test/api/card.test.tsserver/test/mocks/deployments.tsserver/test/hooks/persona.test.ts
**/*.{js,ts,tsx,jsx,sol}
📄 CodeRabbit inference engine (AGENTS.md)
Follow linter/formatter (eslint, prettier, solhint) strictly with high strictness level. No
anytype.
Files:
server/test/api/auth.test.tsserver/test/hooks/panda.test.tsserver/test/api/card.test.tsserver/test/mocks/deployments.tsserver/test/hooks/persona.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Omit redundant type names in variable declarations - let the type system explain itself
**/*.{ts,tsx}: Use PascalCase for TypeScript types and interfaces
Use valibot for all runtime validation of API inputs, environment variables, and other data; define schemas once and reuse them
Infer TypeScript types from valibot schemas usingtype User = v.Input<typeof UserSchema>instead of manually defining interfaces
Files:
server/test/api/auth.test.tsserver/test/hooks/panda.test.tsserver/test/api/card.test.tsserver/test/mocks/deployments.tsserver/test/hooks/persona.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Omit contextual names - don't repeat class/module names in members
Omit meaningless words like 'data', 'state', 'manager', 'engine', 'value' from variable and function names unless they add disambiguation
**/*.{ts,tsx,js,jsx}: Prefer function declarations for all multi-line functions; use function expressions or arrow functions only for single-line implementations
Preferconstfor all variable declarations by default; only useletif the variable's value will be reassigned
Declare each variable on its own line with its ownconstorletkeyword, not multiple declarations on one line
Use camelCase for TypeScript variables and functions
Always useimport type { ... }for type imports
Use relative paths for all imports within the project; avoid tsconfig path aliases
Follow eslint-plugin-import order: react, external libraries, then relative paths
Use object and array destructuring to access and use properties
Use object method shorthand syntax when a function is a property of an object
Prefer optional chaining (?.), nullish coalescing (??), object and array spreading (...), andfor...ofloops over traditional syntax
Do not use abbreviations or cryptic names; write out full words likeerror,parameters,requestinstead oferr,params,req
UseNumber.parseInt()instead of the globalparseInt()function when parsing numbers
All classes called withnewmust use PascalCase
UseBuffer.from(),Buffer.alloc(), orBuffer.allocUnsafe()instead of the deprecatednew Buffer()
Use@ts-expect-errorinstead of@ts-ignore; follow it immediately with a single-line lowercase comment explaining why the error is expected, without separators like-or:
Do not include the type in a variable's name; let the static type system do its job (e.g., useconst user: Usernotconst userObject: User)
Do not repeat the name of a class or module within its members; omit contextual names (e.g., use `class User { getProfil...
Files:
server/test/api/auth.test.tsserver/test/hooks/panda.test.tsserver/test/api/card.test.tsserver/test/mocks/deployments.tsserver/test/hooks/persona.test.ts
server/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
server/**/*.{ts,tsx}: Server API: implement schema-first approach using OpenAPI via hono with validation via valibot middleware
Server database: drizzle schema is source of truth. Migrations required. No direct database access in handlers - usec.var.db
Files:
server/test/api/auth.test.tsserver/test/hooks/panda.test.tsserver/test/api/card.test.tsserver/test/mocks/deployments.tsserver/test/hooks/persona.test.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
For files with a single
defaultexport, name the file identically to the export; for files with multiple exports, use camelCase with a strong preference for a single word
Files:
server/test/api/auth.test.tsserver/test/hooks/panda.test.tsserver/test/api/card.test.tsserver/test/mocks/deployments.tsserver/test/hooks/persona.test.ts
**/.changeset/*.md
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Use a lowercase sentence in the imperative present tense for changeset summaries
Files:
.changeset/loud-apples-double.md
🧠 Learnings (8)
📚 Learning: 2025-12-30T15:03:28.449Z
Learnt from: CR
Repo: exactly/exa PR: 0
File: .cursor/rules/style.mdc:0-0
Timestamp: 2025-12-30T15:03:28.449Z
Learning: Applies to **/*.{ts,tsx} : Use valibot for all runtime validation of API inputs, environment variables, and other data; define schemas once and reuse them
Applied to files:
server/test/api/auth.test.ts
📚 Learning: 2025-12-31T00:23:55.034Z
Learnt from: cruzdanilo
Repo: exactly/exa PR: 610
File: .changeset/ready-experts-fly.md:1-2
Timestamp: 2025-12-31T00:23:55.034Z
Learning: In the exactly/exa repository, allow and require empty changeset files (containing only --- separators) when changes are not user-facing and do not warrant a version bump. This is needed because CI runs changeset status --since origin/main and requires a changeset file to exist. Ensure such empty changesets are used only for non-user-facing changes and document the rationale in the commit or changelog notes.
Applied to files:
.changeset/loud-apples-double.md
📚 Learning: 2025-12-31T00:20:16.737Z
Learnt from: cruzdanilo
Repo: exactly/exa PR: 610
File: server/test/database.ts:56-60
Timestamp: 2025-12-31T00:20:16.737Z
Learning: In server code: SIWE credentials store the Ethereum address as `publicKey` (20 bytes via `hexToBytes(address)`). The `decodePublicKey` utility has special handling for 20-byte inputs that bypasses COSE decoding, padding to 32 bytes for x-coordinate and using zero bytes for y-coordinate. Only passkey/WebAuthn credentials use COSE-encoded public keys.
Applied to files:
server/test/api/card.test.ts
📚 Learning: 2025-12-23T19:56:56.405Z
Learnt from: CR
Repo: exactly/exa PR: 0
File: .cursor/rules/dev.mdc:0-0
Timestamp: 2025-12-23T19:56:56.405Z
Learning: Server (exactly/server) uses Hono (framework), Node.js, Drizzle ORM (database), and PostgreSQL
Applied to files:
server/test/hooks/persona.test.ts
📚 Learning: 2025-12-23T19:57:35.503Z
Learnt from: CR
Repo: exactly/exa PR: 0
File: .cursor/rules/server.mdc:0-0
Timestamp: 2025-12-23T19:57:35.503Z
Learning: Applies to server/api/**/*.ts : The database client (`db`) should be passed via Hono's context (`c.var.db`), not imported directly into route handlers
Applied to files:
server/test/hooks/persona.test.ts
📚 Learning: 2025-12-23T19:58:16.574Z
Learnt from: CR
Repo: exactly/exa PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-23T19:58:16.574Z
Learning: Server stack: use hono, node.js, drizzle orm, postgres
Applied to files:
server/test/hooks/persona.test.ts
📚 Learning: 2025-12-23T19:58:16.574Z
Learnt from: CR
Repo: exactly/exa PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-23T19:58:16.574Z
Learning: Applies to server/**/*.{ts,tsx} : Server database: drizzle schema is source of truth. Migrations required. No direct database access in handlers - use `c.var.db`
Applied to files:
server/test/hooks/persona.test.ts
📚 Learning: 2025-12-23T19:56:56.405Z
Learnt from: CR
Repo: exactly/exa PR: 0
File: .cursor/rules/dev.mdc:0-0
Timestamp: 2025-12-23T19:56:56.405Z
Learning: Design all tests to run without `.env` files or external services
Applied to files:
server/test/hooks/persona.test.ts
🧬 Code graph analysis (1)
server/test/hooks/panda.test.ts (1)
server/database/schema.ts (1)
credentials(9-23)
🔇 Additional comments (10)
server/test/hooks/panda.test.ts (2)
62-64: LGTM!The change correctly replaces the hardcoded
zeroAddresswith a dynamically injected factory address, aligning test data with the dependency injection pattern.
1241-1243: LGTM!Consistent use of injected factory address in the concurrency test setup.
server/test/hooks/persona.test.ts (2)
7-7: LGTM!Correct addition of
injectimport from vitest to support dependency injection in test setup.
56-56: LGTM!Consistent replacement of hardcoded factory address with injected value.
server/test/api/auth.test.ts (2)
13-13: LGTM!Correct addition of
injectimport from vitest.
93-93: LGTM!Consistent use of the injected factory address in the mock, matching the pattern used in the actual test data setup.
server/test/mocks/deployments.ts (1)
8-8: LGTM!Correctly adds the factory address to the mocked chain exports, following the established pattern for other contract addresses. This change enables tests to access the injected factory address via
inject("ExaAccountFactory").server/test/api/card.test.ts (3)
11-11: LGTM! Unused import correctly removed.The removal of
zeroAddressfrom the viem imports is correct, as all previous usages have been replaced withinject("ExaAccountFactory").
27-50: LGTM! Factory injection correctly applied to test credentials.The credentials now consistently use
inject("ExaAccountFactory")for thefactoryfield instead of hardcoded addresses. This aligns with the existing pattern in the file (lines 25, 61) and supports dependency injection in tests.
168-174: LGTM! Factory injection correctly applied.The credential insertion correctly uses
inject("ExaAccountFactory")for thefactoryfield, maintaining consistency with the other credential insertions in this file.
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.