fix(caching): update test expectations for cache analytics#1416
Merged
jensneuse merged 3 commits intofeat/add-caching-supportfrom Mar 4, 2026
Merged
fix(caching): update test expectations for cache analytics#1416jensneuse merged 3 commits intofeat/add-caching-supportfrom
jensneuse merged 3 commits intofeat/add-caching-supportfrom
Conversation
…aliases - Add clearCacheAnalytics() mechanism to strip CacheAnalytics from response nodes in test framework by default (similar to clearCacheKeyTemplates) - Tests using WithEntityCaching() opt-in to preserve cache analytics - Update federation_caching_test.go with additional L2 cache analytics test - Fix OriginalName and HasAliases fields in datasource test expectations - Add CacheAnalytics and CacheAnalyticsHash values to entity caching tests All v2 and execution tests now pass. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
jensneuse
requested review from
a team,
Noroth,
StarpTech,
devsergiy,
dkorittki and
ysmolski
as code owners
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds end-to-end cache-analytics tests for root-field caching, enriches test fixtures with aliasing and cache-analytics metadata, and adds helpers to strip Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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
`@v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.go`:
- Around line 1800-1803: CI reports a gci formatting failure in the composite
literal that contains the fields Path, Nullable, HasAliases, Fields (the struct
literal where Path: []string{"shippingInfo"}, Nullable: true, HasAliases: true,
Fields: []*resolve.Field{...}); fix it by running the repo’s auto-formatters
(e.g., gofmt/gci or the configured gci formatter or golangci-lint run --fix) to
correct spacing/indentation and import ordering, then stage and commit the
resulting changes so the file passes CI.
- Around line 1984-1991: The CacheAnalytics.KeyFields currently contains
malformed token fragments "{a" and "b}" which break the resolve.KeyField shape;
update the resolve.ObjectCacheAnalytics.KeyFields so composite key parts are
represented as a nested resolve.KeyField for the "info" field (i.e., a single
KeyField with Name "info" that contains nested Fields for "a" and "b") rather
than separate string fragments, leaving the top-level id KeyField intact; locate
the CacheAnalytics struct in the graphql_datasource_federation_test.go and
replace the malformed entries with a proper nested resolve.KeyField for "info"
(using the Fields slice) to restore correct cache analytics/hash behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 800da69c-863e-4477-b9c1-ebc1f7dede0f
📒 Files selected for processing (4)
execution/engine/federation_caching_test.gov2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.gov2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.gov2/pkg/engine/datasourcetesting/datasourcetesting.go
Comment on lines
+1984
to
+1991
| CacheAnalytics: &resolve.ObjectCacheAnalytics{ | ||
| KeyFields: []resolve.KeyField{ | ||
| {Name: "id"}, | ||
| {Name: "info"}, | ||
| {Name: "{a"}, | ||
| {Name: "b}"}, | ||
| }, | ||
| }, |
Contributor
There was a problem hiding this comment.
Use structured nested KeyFields in CacheAnalytics (current values are malformed).
At Line 1988 and Line 1989, "{a" / "b}" are token fragments, not key fields. This breaks the resolve.KeyField shape and can produce wrong cache analytics/hash behavior for composite keys.
🔧 Proposed fix
CacheAnalytics: &resolve.ObjectCacheAnalytics{
KeyFields: []resolve.KeyField{
{Name: "id"},
- {Name: "info"},
- {Name: "{a"},
- {Name: "b}"},
+ {
+ Name: "info",
+ Children: []resolve.KeyField{
+ {Name: "a"},
+ {Name: "b"},
+ },
+ },
},
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| CacheAnalytics: &resolve.ObjectCacheAnalytics{ | |
| KeyFields: []resolve.KeyField{ | |
| {Name: "id"}, | |
| {Name: "info"}, | |
| {Name: "{a"}, | |
| {Name: "b}"}, | |
| }, | |
| }, | |
| CacheAnalytics: &resolve.ObjectCacheAnalytics{ | |
| KeyFields: []resolve.KeyField{ | |
| {Name: "id"}, | |
| { | |
| Name: "info", | |
| Children: []resolve.KeyField{ | |
| {Name: "a"}, | |
| {Name: "b"}, | |
| }, | |
| }, | |
| }, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.go`
around lines 1984 - 1991, The CacheAnalytics.KeyFields currently contains
malformed token fragments "{a" and "b}" which break the resolve.KeyField shape;
update the resolve.ObjectCacheAnalytics.KeyFields so composite key parts are
represented as a nested resolve.KeyField for the "info" field (i.e., a single
KeyField with Name "info" that contains nested Fields for "a" and "b") rather
than separate string fragments, leaving the top-level id KeyField intact; locate
the CacheAnalytics struct in the graphql_datasource_federation_test.go and
replace the malformed entries with a proper nested resolve.KeyField for "info"
(using the Fields slice) to restore correct cache analytics/hash behavior.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.go (1)
1984-1991:⚠️ Potential issue | 🟠 MajorFix malformed
Accountcache analytics key fields (composite key is tokenized incorrectly).At Line 1987-Line 1989,
"{a"and"b}"are token fragments, not structured nested key fields. This can break analytics/hash semantics for composite keys and should useChildrenunderinfoinstead.🔧 Proposed fix
CacheAnalytics: &resolve.ObjectCacheAnalytics{ KeyFields: []resolve.KeyField{ {Name: "id"}, - {Name: "info"}, - {Name: "{a"}, - {Name: "b}"}, + { + Name: "info", + Children: []resolve.KeyField{ + {Name: "a"}, + {Name: "b"}, + }, + }, }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.go` around lines 1984 - 1991, Account's CacheAnalytics KeyFields contains token fragments "{a" and "b}" instead of a nested structure; update the resolve.ObjectCacheAnalytics for Account to represent the composite key under the "info" field using Children on the resolve.KeyField for "info" (i.e., remove the malformed KeyField entries with Name "{a" and "b}" and add a resolve.KeyField{Name: "info", Children: []resolve.KeyField{{Name: "a"}, {Name: "b"}}} so the composite key fields are nested correctly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.go`:
- Around line 1984-1991: Account's CacheAnalytics KeyFields contains token
fragments "{a" and "b}" instead of a nested structure; update the
resolve.ObjectCacheAnalytics for Account to represent the composite key under
the "info" field using Children on the resolve.KeyField for "info" (i.e., remove
the malformed KeyField entries with Name "{a" and "b}" and add a
resolve.KeyField{Name: "info", Children: []resolve.KeyField{{Name: "a"}, {Name:
"b"}}} so the composite key fields are nested correctly).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 422e3b89-25e7-48e1-b7c7-e98db786d62b
📒 Files selected for processing (3)
v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.gov2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.gov2/pkg/engine/resolve/resolve_test.go
💤 Files with no reviewable changes (1)
- v2/pkg/engine/resolve/resolve_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This was referenced Mar 4, 2026
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
Updates test framework and test expectations to handle cache analytics fields that are now populated during planning.
Key Changes:
clearCacheAnalytics()mechanism to datasourcetesting framework to stripCacheAnalyticsfrom response nodes by default (tests usingWithEntityCaching()opt-in)OriginalName,HasAliases)Tests: All v2 and execution tests pass (282 insertions, 1 deletion across 4 files).
Co-Authored-By: Claude Haiku 4.5 noreply@anthropic.com
Summary by CodeRabbit
Tests
Bug Fixes