tests: add tests to ensapi using ens-test-env#1873
Conversation
🦋 Changeset detectedLatest commit: 5e8221e The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 47 minutes and 32 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds integration tests for resolution API endpoints ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Greptile SummaryThis PR adds integration tests for the three ENSApi resolution endpoints ( Confidence Score: 5/5Safe to merge — changes are additive (new tests + shared constants) with a clean params refactor and no behavioral regressions. All findings are P2 (one unused export). The params.schema.ts refactor is strictly better than the old approach and the integration test coverage is solid. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Test as Integration Test
participant ENSApi as ENSApi (Hono)
participant Params as params.schema.ts
participant Handler as Resolution Handler
Test->>ENSApi: GET /api/resolve/records/:name?addresses=60
ENSApi->>Params: resolveRecordsQuery.parse(query)
Params->>Params: toSelection(fields, ctx)
alt Selection is empty
Params-->>ENSApi: ZodError "Selection cannot be empty."
ENSApi-->>Test: 400 Invalid Input
else Selection valid
Params-->>ENSApi: { selection, trace, accelerate }
ENSApi->>Handler: resolveRecords(name, selection, ...)
Handler-->>ENSApi: { records, accelerationRequested, ... }
ENSApi-->>Test: 200 { records: { addresses: { 60: "0x..." } }, ... }
end
Test->>ENSApi: GET /api/resolve/primary-name/:address/:chainId
ENSApi->>Params: parse(address, chainId)
Params-->>ENSApi: { address, chainId }
ENSApi->>Handler: resolvePrimaryName(address, chainId, ...)
Handler-->>ENSApi: { name: null, accelerationRequested, ... }
ENSApi-->>Test: 200 { name: null, ... }
Test->>ENSApi: GET /api/resolve/primary-names/:address?chainIds=1
ENSApi->>Params: parse(address, chainIds)
Params-->>ENSApi: { address, chainIds: [1] }
ENSApi->>Handler: resolvePrimaryNames(address, chainIds, ...)
Handler-->>ENSApi: { names: { "1": null }, ... }
ENSApi-->>Test: 200 { names: { "1": null }, ... }
Reviews (2): Last reviewed commit: "lint + openspec generate" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
sevenzing
changed the title
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@sevenzing Hey cool to see this. Reviewed and shared some suggestions appreciate your advice.
sevenzing
force-pushed
the
ll/chore-fixes
branch
2 times, most recently
from
50b9871 to
8bcfa0c
Compare
sevenzing
force-pushed
the
ll/rest-api-tests
branch
from
ac0822d to
1290e92
Compare
| const selection = params.selection.parse(selectionParams); | ||
| return { selection, trace, accelerate }; | ||
| }), | ||
| query: params.resolveRecordsQuery, |
There was a problem hiding this comment.
this solves problem with 500 in https://api.alpha.ensnode.io/api/resolve/records/vitalik.eth
root problem is that .parse throws error inside .transform - not zod style
sevenzing
force-pushed
the
ll/rest-api-tests
branch
from
1290e92 to
f331be7
Compare
![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
`@apps/ensapi/src/handlers/api/resolution/resolve-primary-names.integration.test.ts`:
- Around line 95-97: The fetch in the test (the call that builds
`${BASE_URL}/api/resolve/primary-names/${address}${query ? \`?\${query}\` :
""}`) can hang if the local service is down; wrap the request with an
AbortController, create a timeout (e.g., using setTimeout) that calls
controller.abort() after a short period (e.g., a few seconds), pass
controller.signal to fetch, and clear the timeout once fetch resolves to avoid
leaking timers; apply the same pattern wherever similar fetches are used in
tests like resolve-records.integration.test.ts and
resolve-primary-name.integration.test.ts.
In `@packages/ensnode-sdk/src/shared/devnet-accounts.ts`:
- Line 4: Typo in the module comment: replace the word "user" with "use" in the
top-of-file comment that reads "You can user `docker compose up devnet` to see
actual data in devnet" so it becomes "You can use `docker compose up devnet` to
see actual data in devnet"; update that comment in
packages/ensnode-sdk/src/shared/devnet-accounts.ts (the module header comment)
to fix the spelling.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 42ac76ab-6a5a-4042-afa0-19934647813a
📒 Files selected for processing (9)
.changeset/sparkly-mangos-tease.mdapps/ensapi/src/handlers/api/resolution/resolution-api.routes.tsapps/ensapi/src/handlers/api/resolution/resolve-primary-name.integration.test.tsapps/ensapi/src/handlers/api/resolution/resolve-primary-names.integration.test.tsapps/ensapi/src/handlers/api/resolution/resolve-records.integration.test.tsapps/ensapi/src/lib/handlers/params.schema.tsapps/ensapi/src/omnigraph-api/schema/account.integration.test.tspackages/ensnode-sdk/src/internal.tspackages/ensnode-sdk/src/shared/devnet-accounts.ts
sevenzing
requested review from
lightwalker-eth and
shrugs
and removed request for
tk-o
|
@greptile |
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@sevenzing Great to see this! Shared some small suggestions please take the lead to merge when ready 👍
| @@ -39,7 +36,7 @@ describe("Account.domains", () => { | |||
| `; | |||
|
|
|||
| it("returns domains owned by the default owner", async () => { | |||
There was a problem hiding this comment.
| it("returns domains owned by the default owner", async () => { | |
| it("returns domains owned by the devnet owner", async () => { |
Goal: Align with variable name update
| const selection = params.selection.parse(selectionParams); | ||
| return { selection, trace, accelerate }; | ||
| }), | ||
| query: params.resolveRecordsQuery, |
3 participants
Summary
resolve-primary-name,resolve-primary-names,resolve-records)docker-compose.yml: addbuildsection to build images locally, also addedRPC_URL_15658733because without it running indexer withens-test-envresults in fail with error likefailed to make request to localhost:8545Why
Pre-Review Checklist (Blocking)