test(e2e): add indexing and gRPC query e2e tests#52
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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. 📝 WalkthroughWalkthroughCI workflow's e2e job renamed from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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: 1
🧹 Nitpick comments (2)
e2e/grpc_test.go (1)
81-161: Table-drive the gRPC endpoint checks to reduce duplication.The subtests are valid; converting them to table-driven cases would better match project test conventions and make future endpoint additions easier.
As per coding guidelines, "Use table-driven tests in Go test files".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/grpc_test.go` around lines 81 - 161, Convert the repeated subtests into a single table-driven test: define a slice of test cases (with fields name, call type or a function closure, request payload, and a verify function) and iterate with t.Run for each case; replace the separate invocations of blobClient.GetByCommitment, blobClient.Get, blobClient.GetAll, headerClient.GetByHeight, and headerClient.LocalHead by entries in that table, using the existing variables (data, commitment, height, namespace) in the request/verification closures; ensure each case's verify closure checks the same conditions now duplicated (nil checks, data/commitment equality, blob inclusion, header height comparisons) and call t.Fatalf/t.Fatal within the closure on failure so behavior remains identical.e2e/indexing_test.go (1)
69-128: Consider table-driving the RPC path assertions.These subtests are clear, but a table-driven structure would remove repetition and match repository test conventions.
As per coding guidelines, "Use table-driven tests pattern for test cases".♻️ Refactor sketch
+type rpcCase struct { + name string + method string + params []any + assert func(t *testing.T, r jsonRPCResponse) +} + +cases := []rpcCase{ + // blob.Get, blob.GetAll, header.GetByHeight, header.LocalHead +} + +for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + r := doRPC(t, proc, apexRPCAddr, tc.method, tc.params...) + if r.Error != nil { + t.Fatalf("%s error: %s", tc.name, r.Error.Message) + } + tc.assert(t, r) + }) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/indexing_test.go` around lines 69 - 128, The subtests under Test (calls to doRPC with RPC methods "blob.Get","blob.GetAll","header.GetByHeight","header.LocalHead" and assertions on rpcBlob/results) repeat similar logic; convert them to a table-driven test by creating a slice of test cases (each with name, method string, params, and a small checker function or expected condition) and iterate with for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { r := doRPC(..., tc.method, tc.params...); if r.Error != nil { t.Fatalf(...) }; tc.check(t, r) }) }. Reference the existing helpers and types (doRPC, rpcBlob, commitment, data) when implementing each case’s checker to preserve current assertions (blob content/commitment checks, non-null header checks, blob list contains commitment).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/grpc_test.go`:
- Around line 19-21: The test currently reuses the startup timeout context (ctx,
cancel) for all gRPC calls which can cause unrelated deadline errors; update
each gRPC request in this test (including calls between lines ~81-161) to create
and use a dedicated per-call context via newRPCCtx(), e.g. replace usages of ctx
with ctx := newRPCCtx() and defer cancel() immediately after each call's context
creation so every gRPC assertion has its own timeout and is not impacted by the
shared startup context.
---
Nitpick comments:
In `@e2e/grpc_test.go`:
- Around line 81-161: Convert the repeated subtests into a single table-driven
test: define a slice of test cases (with fields name, call type or a function
closure, request payload, and a verify function) and iterate with t.Run for each
case; replace the separate invocations of blobClient.GetByCommitment,
blobClient.Get, blobClient.GetAll, headerClient.GetByHeight, and
headerClient.LocalHead by entries in that table, using the existing variables
(data, commitment, height, namespace) in the request/verification closures;
ensure each case's verify closure checks the same conditions now duplicated (nil
checks, data/commitment equality, blob inclusion, header height comparisons) and
call t.Fatalf/t.Fatal within the closure on failure so behavior remains
identical.
In `@e2e/indexing_test.go`:
- Around line 69-128: The subtests under Test (calls to doRPC with RPC methods
"blob.Get","blob.GetAll","header.GetByHeight","header.LocalHead" and assertions
on rpcBlob/results) repeat similar logic; convert them to a table-driven test by
creating a slice of test cases (each with name, method string, params, and a
small checker function or expected condition) and iterate with for _, tc :=
range cases { t.Run(tc.name, func(t *testing.T) { r := doRPC(..., tc.method,
tc.params...); if r.Error != nil { t.Fatalf(...) }; tc.check(t, r) }) }.
Reference the existing helpers and types (doRPC, rpcBlob, commitment, data) when
implementing each case’s checker to preserve current assertions (blob
content/commitment checks, non-null header checks, blob list contains
commitment).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: de7be17e-188f-49c5-8bec-299344afbe4c
📒 Files selected for processing (3)
.github/workflows/ci.ymle2e/grpc_test.goe2e/indexing_test.go
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/grpc_test.go (1)
87-177: Refactor repeated subtests into a table-driven structure.The five gRPC assertions duplicate setup/timeout scaffolding; a table-driven loop would reduce repetition and make future endpoint additions easier to maintain.
♻️ Suggested refactor shape
+testCases := []struct { + name string + run func(t *testing.T, rpcCtx context.Context) +}{ + {name: "BlobService.GetByCommitment", run: func(t *testing.T, rpcCtx context.Context) { /* existing assertions */ }}, + {name: "BlobService.Get", run: func(t *testing.T, rpcCtx context.Context) { /* existing assertions */ }}, + {name: "BlobService.GetAll", run: func(t *testing.T, rpcCtx context.Context) { /* existing assertions */ }}, + {name: "HeaderService.GetByHeight", run: func(t *testing.T, rpcCtx context.Context) { /* existing assertions */ }}, + {name: "HeaderService.LocalHead", run: func(t *testing.T, rpcCtx context.Context) { /* existing assertions */ }}, +} + +for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + rpcCtx, cancel := newRPCCtx(t) + defer cancel() + tc.run(t, rpcCtx) + }) +}As per coding guidelines, "Use table-driven tests pattern for test cases".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/grpc_test.go` around lines 87 - 177, The five nearly-identical t.Run blocks should be refactored into a single table-driven test: create a slice of test cases (with fields like name, request, clientMethod identifier e.g. blobClient.GetByCommitment, blobClient.Get, blobClient.GetAll, headerClient.GetByHeight, headerClient.LocalHead, and expected-check type), then loop over them calling t.Run(tc.name, func(t *testing.T){ rpcCtx, cancel := newRPCCtx(t); defer cancel(); resp, err := tc.clientMethod(rpcCtx, tc.request); handle err and perform the appropriate assertions based on tc.expected-check (non-nil blob/header, data/commitment equality, presence in GetAll, height checks) so setup/teardown and timeouts are shared and new endpoints can be added by appending to the table.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@e2e/grpc_test.go`:
- Around line 87-177: The five nearly-identical t.Run blocks should be
refactored into a single table-driven test: create a slice of test cases (with
fields like name, request, clientMethod identifier e.g.
blobClient.GetByCommitment, blobClient.Get, blobClient.GetAll,
headerClient.GetByHeight, headerClient.LocalHead, and expected-check type), then
loop over them calling t.Run(tc.name, func(t *testing.T){ rpcCtx, cancel :=
newRPCCtx(t); defer cancel(); resp, err := tc.clientMethod(rpcCtx, tc.request);
handle err and perform the appropriate assertions based on tc.expected-check
(non-nil blob/header, data/commitment equality, presence in GetAll, height
checks) so setup/teardown and timeouts are shared and new endpoints can be added
by appending to the table.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds two new e2e tests using the existing Tastora setup:
TestIndexingQueryPathscovers the JSON-RPCblob.Get,blob.GetAll,header.GetByHeight, andheader.LocalHeadendpoints;TestGRPCBlobQueryexercises the same read paths via the gRPCBlobServiceandHeaderService. Also renames the CIe2e-submissionjob toe2eand runs all./...so new tests are picked up automatically.Summary by CodeRabbit
Tests
CI