Skip to content

refactor: share in-memory gRPC bufconn helpers across fake-RPC tests - #1021

Merged
apstndb merged 2 commits into
mainfrom
refactor/test-bufconn-helper
Sep 16, 2026
Merged

apstndb merged 2 commits into
mainfrom
refactor/test-bufconn-helper

Conversation

@apstndb

@apstndb apstndb commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Summary

Shared in-memory gRPC transport helpers for fake Spanner and DatabaseAdmin servers. Each test keeps its own fake-server behavior; heartbeatRPCServer is not turned into a universal fake. Real TCP/TLS fixtures in tls_transport_test.go and spanner_metrics_test.go are unchanged.

Lifecycle review of the U1 prototype:

  • Dialer keeps listener.Dial() rather than DialContext. No migrated test covers cancelled-dial behavior, and RecreateClient / USE / DETACH still dial the shared listener after t.Context() is cancelled during cleanup.
  • Serve errors are reported inside t.Cleanup after Stop returns, so a Serve goroutine cannot log after cleanup completes.
  • No fixed sleeps.

#1018 was still open and unmerged at publication, so this branch is based on current origin/main without that execute_sql_test.go change. Merge main additively if #1018 lands first.

Base / HEAD

  • Base: cbc1663366464409aba93f0d82cff9f4e9daa87b (origin/main)
  • HEAD: d716daca8d53d61e3e809c5d5135f32965e6ef19

Numstat

Kind Files + Net
Production 0 0 0 0
Test callers 16 33 590 −557
Fixture helper (internal/mycli/bufconn_test.go) 1 171 0 +171
Combined test/fixture 17 204 590 −386

Per-file:

171  0  internal/mycli/bufconn_test.go
  1 35  internal/mycli/ddl_in_transaction_rpc_test.go
  4 25  internal/mycli/directed_read_matrix_test.go
  7 32  internal/mycli/directed_read_wire_test.go
  1 42  internal/mycli/execute_ddl_rpc_test.go
  1 36  internal/mycli/execute_ddl_sequence_kind_rpc_test.go
  1 31  internal/mycli/execute_partitioned_test.go
  1 41  internal/mycli/execute_sql_test.go
  3 62  internal/mycli/fuzzy_finder_candidates_test.go
  3 26  internal/mycli/heartbeat_owner_test.go
  1 43  internal/mycli/query_cache_test.go
  3 37  internal/mycli/show_operation_test.go
  1 41  internal/mycli/statements_query_profile_test.go
  3 21  internal/mycli/statements_run_partition_test.go
  1 43  internal/mycli/stream_width_test.go
  1 42  internal/mycli/string_quote_stream_test.go
  1 33  internal/mycli/sync_proto_bundle_test.go

Eliminated RPC setup is the repeated bufconn.Listen / Serve / grpc.NewClient / client-construction blocks. Retained real integration: TCP listeners in tls_transport_test.go, spanner_metrics_test.go, and the partition TCP fixture in statements_run_partition_test.go.

Scenario mapping

No named Test* functions were added or removed. Every previously named case survives under the same name. Collapsed transport setup maps as follows:

Removed local transport setup Surviving named cases / remaining constructor
newQueryCacheRPCSession bufconn bootstrap TestQueryCachePublication* via newBufconnQuerySession
newEmptySQLRPCSession bufconn bootstrap TestExecuteSQLBufferedAndStreamingEmptyResults via newBufconnQuerySession
stream-width / string-quote / query-profile session bootstrap TestTablePreviewProcessorStreamingPreservesValue, TestExecuteSQLStreamingTablePreservesValue, TestCLIStringQuoteModeStreaming*, TestShowQueryProfile* via newBufconnQuerySession
newDDLAdminSession / newScriptedDDLSession / attachDDLAdmin TestExecuteDdlStatementsRPC, TestSequenceKindRepairRPC, TestDDLInTransaction* via newBufconnAdminSession / newBufconnAdminClient
sync_proto_bundle admin attach TestSyncProtoBundle* via newBufconnAdminClient
newShowOperationTestSession (Operations only; bare systemVariables) TestShowOperation* via dialBufconn
newFuzzyAdminSession / newFuzzySchemaSession (p/i/db identity kept) TestFetchAdminCandidates, TestFetchSchemaObjectCandidates, other TestFuzzy* / TestFetch* / TestResolveCandidates*
startDirectedReadWire / startDirectedReadDial / run-partition dial options TestDirectedRead*, TestRunPartition*, TestPartitionExportsCompleteEnvelope via newBufconnSpannerClient / bufconnClientOptions
startPartitionFanIn TestRunPartitioned*, TestStreamPartitioned*, TestBufferPartitioned* via newBufconnSpannerClient
newHeartbeatHarness listener/server (custom clientOpts kept) TestHeartbeat* via dialBufconn; heartbeatRPCServer unchanged

Gates

Captured under .worktrees/test-bufconn-helper/.tmp/quality-wave-20260917/ at HEAD d716daca8d53d61e3e809c5d5135f32965e6ef19:

  1. Migrated test families (go test -short -count=1 ./internal/mycli with the 109 surviving names): exit 0
  2. GOTOOLCHAIN=go1.26.8 make check with golangci-lint 2.13.2: exit 0
  3. GOTOOLCHAIN=go1.26.8 make check-race: exit 0

Environment: GOTOOLCHAIN=go1.26.8, Homebrew golangci-lint 2.13.2, DOCKER_HOST=unix:///Users/apstndb/.colima/default/docker.sock, TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE=/var/run/docker.sock.

Open in Web Open in Cursor 

Centralize listener, server, dialer, and cleanup for in-memory Spanner
and DatabaseAdmin fakes. Fake-server behavior stays in each test file.
Serve errors are reported inside Cleanup after Stop returns, and the
dialer keeps listener.Dial because no migrated test covers cancelled
dials.
@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown

Code Metrics Report

📊 View detailed coverage report (available for 7 days)

main (f86e0d2) #1021 (becd62d) +/-
Coverage 87.4% 87.4% 0.0%
Code to Test Ratio 1:2.5 1:2.5 -0.1
Test Execution Time 2m50s 2m40s -10s
Details
  |                     | main (f86e0d2) | #1021 (becd62d) | +/-  |
  |---------------------|----------------|-----------------|------|
  | Coverage            |          87.4% |           87.4% | 0.0% |
  |   Files             |            132 |             132 |    0 |
  |   Lines             |          13294 |           13294 |    0 |
  |   Covered           |          11629 |           11629 |    0 |
- | Code to Test Ratio  |          1:2.5 |           1:2.5 | -0.1 |
  |   Code              |          28902 |           28902 |    0 |
- |   Test              |          73163 |           72726 | -437 |
+ | Test Execution Time |          2m50s |           2m40s | -10s |

Reported by octocov

@apstndb

apstndb commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

Reviewed HEAD d716daca8d53d61e3e809c5d5135f32965e6ef19: no actionable findings in this diff.

Reviewed all migrated fixtures and the shared transport cleanup order, Serve completion, service registration, and per-client options. The existing dial behavior and separate TCP/TLS fixtures are preserved.

Current hosted CI checks pass. This is a source-review conclusion for this HEAD; the PR remains draft pending integration.

Additive base update after #1023 so current main is an ancestor of this
branch. No rebase.
@apstndb

apstndb commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

Reviewed updated HEAD cde96d0ba76c36fb379608c7c0865d5e2cc2fa91: no actionable findings.

The tree exactly matches the clean automatic merge of the previously reviewed change with main f86e0d2e8513679d9ce7f0b45bde86565d81f8dc. The remaining PR diff preserves the accepted behavior; no conflict resolution or additional change was introduced. The local make check passed for this integrated tree. Hosted checks for this exact HEAD remain the merge gate.

@apstndb
apstndb marked this pull request as ready for review September 16, 2026 20:59
@apstndb
apstndb merged commit ce16a54 into main Sep 16, 2026
10 checks passed
@apstndb
apstndb deleted the refactor/test-bufconn-helper branch September 16, 2026 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant