test(sync/grpc): verify config affects grpc client (MaxMsgSize, dial override)

[anxkhn]

This PR

Closes #1562.

grpc_sync.go configures the gRPC client differently depending on the Sync
settings (MaxMsgSize, GrpcDialOptionsOverride, ProviderID, Selector),
but the existing tests only assert side effects: a log line, or that the client
is non-nil. They never assert that the configuration actually changes the
resulting gRPC behavior. That is the gap #1562 calls out ("test whether that
configuration affects the calls to grpc").

This is a test-only change. grpc_sync.go is untouched.

What's added

Three table-driven / focused tests in
core/pkg/sync/grpc/grpc_sync_test.go, built on the deterministic seams already
used in this file (bufconn, the generated mocks, GrpcDialOptionsOverride):

1. Test_InitMaxMsgSizeAffectsClient - proves MaxMsgSize is wired as a
   call option, not just logged. A real loopback listener returns a response
   larger than the configured limit; with MaxMsgSize: 100 the ReSync fails
   with codes.ResourceExhausted, and with MaxMsgSize: 0 the same payload
   succeeds. It runs the production (non-override) dial path verbatim, so
   grpc.MaxCallRecvMsgSize is the only variable between the two cases.
2. Test_InitDialOptionsOverrideBypassesCredentialBuilder - proves the
   GrpcDialOptionsOverride branch is taken and skips CredentialBuilder
   entirely. The mock builder is created with no expected calls, so any call to
   Build fails the test; the resulting client is then exercised end to end.
3. Test_ReSyncSendsConfiguredProviderIDAndSelector - proves ProviderID
   and Selector propagate into the outgoing FetchAllFlagsRequest, captured
   server-side.

A small requestCapturingServer helper is added, mirroring the existing
headerCapturingServer.

Testing

cd core
go build ./...
go test -race -covermode=atomic -cover -short ./pkg/sync/grpc/...

All green under -race (non-flaky over repeated runs). Package coverage for
core/pkg/sync/grpc goes from ~87.3% to ~89.9%. golangci-lint (repo-pinned
v2.7.2) reports 0 issues on the package.

[netlify]

✅ Deploy Preview for polite-licorice-3db33c canceled.

Name	Link
🔨 Latest commit	3cc5105
🔍 Latest deploy log	https://app.netlify.com/projects/polite-licorice-3db33c/deploys/6a43fc6e4cbf900008e39f77

[coderabbitai]

📝 Walkthrough

Walkthrough

Three new test functions and a requestCapturingServer helper are added to grpc_sync_test.go to verify that MaxMsgSize, GrpcDialOptionsOverride, and ProviderID/Selector fields on Sync correctly affect the gRPC client configuration and outgoing request fields.

Changes

gRPC Sync Client Wiring Tests

Layer / File(s)	Summary
Test helper and imports core/pkg/sync/grpc/grpc_sync_test.go	Adds strings, codes, and status imports and the requestCapturingServer struct with a channel-based FetchAllFlags capture and stubbed SyncFlags/GetMetadata handlers.
MaxMsgSize, DialOptionsOverride, and ProviderID/Selector tests core/pkg/sync/grpc/grpc_sync_test.go	Adds Test_InitMaxMsgSizeAffectsClient (expects ResourceExhausted for small limit), Test_InitDialOptionsOverrideBypassesCredentialBuilder (asserts CredentialBuilder.Build is skipped), and Test_ReSyncSendsConfiguredProviderIDAndSelector (asserts request fields match Sync config).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the test-focused gRPC client config coverage added in this PR.
Description check	✅ Passed	The description matches the test-only changes and explains the new gRPC behavior coverage.
Linked Issues check	✅ Passed	The new tests verify MaxMsgSize, dial override, and request propagation as requested by #1562.
Out of Scope Changes check	✅ Passed	The changes stay within the test file and do not introduce unrelated scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

_{Comment @coderabbitai help to get the list of available commands.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

🧹 Nitpick comments (1)

core/pkg/sync/grpc/grpc_sync_test.go (1)

253-258: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Switch this test to grpc.NewClient with a passthrough:/// target. grpc.Dial is deprecated, and grpc.NewClient will otherwise try DNS resolution for localBufCon; this test already uses the same pattern above.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/pkg/sync/grpc/grpc_sync_test.go` around lines 253 - 258, Update the test
setup in grpc_sync_test.go to use grpc.NewClient instead of grpc.Dial for the
localBufCon connection; grpc.Dial is deprecated and grpc.NewClient will
otherwise attempt DNS resolution on the target. Keep the existing context dialer
and insecure credentials, but switch the target to the same passthrough:///
pattern used elsewhere in the test so the client connects directly through
bufCon.Dial without name resolution.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@core/pkg/sync/grpc/grpc_sync_test.go`:
- Around line 253-258: Update the test setup in grpc_sync_test.go to use
grpc.NewClient instead of grpc.Dial for the localBufCon connection; grpc.Dial is
deprecated and grpc.NewClient will otherwise attempt DNS resolution on the
target. Keep the existing context dialer and insecure credentials, but switch
the target to the same passthrough:/// pattern used elsewhere in the test so the
client connects directly through bufCon.Dial without name resolution.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4763e7eb-9262-421c-8fd8-7251ede2b977

📥 Commits

Reviewing files that changed from the base of the PR and between d99a3e3 and 3cc5105.

📒 Files selected for processing (1)

• core/pkg/sync/grpc/grpc_sync_test.go