TCP B9: Run the integration suite against Cloud by alex-clickhouse · Pull Request #611 · ClickHouse/clickhouse-cs

alex-clickhouse · 2026-09-03T13:58:54Z

Sits at the top of the tcp/** stack, on #610 (tcp/epic-t4-docs). This diff is only the Cloud test wiring.

Split out of #563, which needed a Cloud service to prove the TLS transport but shipped a five-test smoke set with its own fixture and its own environment variable to do it. It sits at the top rather than next to #563 so that it can see every integration fixture in the stack, not just the ones that existed at B9.

Split out of B9. That change needed a Cloud service to prove the TLS transport, but shipped a five-test smoke set with its own fixture and its own environment variable to do it. This runs the whole integration suite there instead, through the plumbing every other run already uses.

What this does

The suite's usual connection string points at the service. TcpServerFixture resolves its server in three steps, in precedence order:

Source	Purpose
`CLICKHOUSE_TCP_CONNECTION`	A whole native connection string. Carries TLS, so a Cloud run needs no second code path.
`CLICKHOUSE_TCP_HOST` (+ `_PORT`, `_USER`, `_PASSWORD`)	A server already running locally.
Testcontainers	Pinned by `CLICKHOUSE_VERSION`, matching the main suite.

CLICKHOUSE_TEST_ENVIRONMENT=cloud makes the fixture refuse a connection string that does not set UseTls, or that bypasses certificate validation. Without that a misconfigured job would keep passing over a plaintext connection to a public endpoint, proving nothing about the transport it exists to cover.

Category("Cloud") marks the tests the Cloud job also runs, not tests that need a Cloud service. This is the convention ClickHouse.Driver.Tests already uses: the standard matrix applies no filter and the Cloud job selects Category=Cloud. So reusable-tcp.yml drops its Category!=Cloud filter, and these tests now run on the container matrix too. Genuinely Cloud-only tests, if JWT ever needs them, gate on their own environment variable with Assert.Ignore, the way the HTTP suite's bearer-token tests do.

Tests/Cloud is deleted rather than widened. Everything the smoke set asserted is already in the integration suite: handshake and ping, a multi-block query, a MergeTree insert round trip, pooled session reuse over a TEMPORARY table (ConnectionPoolIntegrationTests), and concurrency. Keeping it would have restated that coverage over a slower transport.

Three tests hand-built ClickHouseTcpClientOptions from the fixture's host and port, which silently drops TLS. They derive from TcpServerFixture.Options() now.

Two things a Cloud service does that a container does not

Both were found by running this, not by reading docs.

allow_suspicious_low_cardinality_types cannot be changed on Cloud. The server answers Setting ... should not be changed rather than applying its own value, so the eleven tests needing a LowCardinality over a numeric or DateTime inner cannot run there at all. TcpServerFixture.SkipIfCloudLocksASetting ignores a test that asks for a setting on that list. It is keyed on the settings a test applies, not on a list of test names, so a new case carrying a locked setting is covered by writing it. flatten_nested and the Variant/Dynamic settings are all changeable; the lock is specific to this one.

Worth knowing for the next person who debugs this: the rejection terminates the connection, so ten of the eleven surfaced as ObjectDisposedException: The connection has been terminated and the real error appeared once. Grep for should not be changed, not for the first-reported failure.

Concurrent inserts read short. InsertRowsAsync_ConcurrentInsertsIntoOneTable_AllRowsLand counted 200 of 400. All eight inserts reported success, so the rows were committed and the read saw half: each insert commits on whichever replica its connection reached. The count reads with select_sequential_consistency now, which keeps the assertion live on Cloud rather than skipping it. It passes there, and the setting does nothing on a single server.

The `Memory` engine, which B9 predicted would block this

B9 kept the Cloud set to five tests on the grounds that most integration tables are ENGINE = Memory, whose data is node-local, while a pooled client would spread an insert and its read-back across replicas. That turned out not to hold. MinPoolSize defaults to 0, so nothing is prewarmed, and a checkout takes from the idle list — which for sequential operations holds the connection just returned. One connection is one session and one replica. Two full Cloud runs agree, with 53 Memory tables in the suite.

The shape that would break is a test that makes its connection retire — a server error, an enumerator dropped mid-response — and then reads a Memory table it wrote before the retirement. No test does that today, so this documents the constraint on TcpServerFixture rather than converting 53 CREATE TABLEs. Converting them was the alternative and I decided against it: MergeTree with no ordering key would put the read-back order of every one of those tests at the mercy of part-level parallelism.

Testing

Cloud, by workflow_dispatch on this branch: 1357 passed, 11 skipped, 0 failed, with all nine container jobs green alongside it.

Locally, all three server-resolution paths pass at 2801 tests each. The widened set was also run over a real TLS transport — a container with tcp_port_secure and a self-signed certificate, reached through CLICKHOUSE_TCP_CONNECTION with TlsCaCertificatePath pinned to it — where all 1368 pass. That run is what caught TcpServerFixture.ConnectionString dropping the TLS keys, which failed exactly one test and nothing else. The CLICKHOUSE_TEST_ENVIRONMENT=cloud guard was confirmed load-bearing by pointing it at a plaintext connection string and watching the fixture refuse it.

One consequence of that guard: because it also refuses skip-verify and a pinned authority, cloud mode cannot be rehearsed locally — that would need a publicly trusted certificate for localhost. The guard is CI-facing and the strictness is the point, so the skip path is verified in CI only.

Known limits, deliberately left

The Cloud job takes about 11 minutes. Measured: five per-type corpus methods account for 587 s of 663 s across 1126 of 1368 tests, and it is all network latency — only 17 tests exceed one second. Parallelising the suite would cut it to roughly two minutes, but it would also create tables far more aggressively against a shared service, so it is not being done here.
Category("Cloud") is on 14 of the 37 integration fixtures — the ones that existed at B9, where this work was written. The other 23 arrived in epics between B9 and here and are not tagged, so the Cloud job does not run them yet. Tagging them is the obvious next step now that this branch sits at the top, but several want checking against the service first rather than assuming: ReadonlyUserIntegrationTests (CREATE USER/GRANT), QBitIntegrationTests and GeometryIntegrationTests (version-gated), CompressionIntegrationTests, ClickHouseTcpTracingIntegrationTests, and PublicSurfaceIntegrationTests. Left out of this change deliberately, rather than tagged blind and discovered through a red job on a shared service.
Re-running just the Cloud set means dispatching tests-tcp.yml and paying for the whole nine-job matrix, because tests-cloud-tcp.yml is workflow_call-only and a workflow must exist on the default branch to be dispatchable. Giving it its own workflow_dispatch trigger is worth doing once this lands.

No CHANGELOG/RELEASENOTES entry, consistent with the rest of the tcp/** stack: neither file mentions ClickHouse.Driver.Tcp today, the assembly is IsPackable=false and gated behind an experimental diagnostic, and epic R3 writes the client up as one piece when it ships.

🤖 Generated with Claude Code

cursor

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 2d1478b. Configure here.}

codecov · 2026-09-04T09:05:29Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot

🟡 Changes recommended

TcpServerFeatures performs sync-over-async blocking in a static initializer, which can deadlock under a SynchronizationContext and/or stall discovery longer than necessary without bounded timeouts.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR wires the native TCP integration test suite to run against a real ClickHouse Cloud service by (1) teaching the integration fixture to resolve a server via a full connection string (including TLS), (2) tagging a subset of integration fixtures as Category("Cloud") and adjusting/skipping cases that can’t work on multi-replica Cloud, and (3) adding a reusable GitHub Actions workflow to execute that Cloud category.

Changes:

Extend TcpServerFixture to resolve the test server from CLICKHOUSE_TCP_CONNECTION / host overrides / Testcontainers, and enforce TLS invariants in Cloud mode.
Mark selected integration fixtures as Category("Cloud") and add Cloud-specific skips/settings (e.g., locked settings; sequential consistency for replica-read correctness).
Add a new tests-cloud-tcp.yml reusable workflow and wire it into tests-tcp.yml for push/dispatch runs.

File summaries

File	Description
ClickHouse.Driver.Tcp.Tests/Utilities/TcpServerFeatures.cs	Falls back to querying the configured server for `version()` when `CLICKHOUSE_VERSION` is not pinned.
ClickHouse.Driver.Tcp.Tests/Utilities/QueryLog.cs	Skips query-log-based assertions on Cloud where `system.query_log` is replica-local.
ClickHouse.Driver.Tcp.Tests/Integration/TlsTransportIntegrationTests.cs	Updates documentation comment to reflect Cloud-category coverage expectations.
ClickHouse.Driver.Tcp.Tests/Integration/TimezoneColumnIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/TcpServerFixture.cs	Implements Cloud connection-string precedence, TLS enforcement in Cloud mode, and Cloud-locked-settings skipping; centralizes options/factory creation.
ClickHouse.Driver.Tcp.Tests/Integration/PocoWriteIntegrationTests.cs	Adds `Category("Cloud")` and skips cases requiring Cloud-locked settings.
ClickHouse.Driver.Tcp.Tests/Integration/PocoReadIntegrationTests.cs	Adds `Category("Cloud")` and skips cases requiring Cloud-locked settings.
ClickHouse.Driver.Tcp.Tests/Integration/NestedArrayInsertIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/MultiBlockStateIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/MidStreamFailureIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/InsertSlicingIntegrationTests.cs	Adds `Category("Cloud")` and skips cases requiring Cloud-locked settings.
ClickHouse.Driver.Tcp.Tests/Integration/InsertInterruptionIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ConnectionPoolIntegrationTests.cs	Reads row counts with `select_sequential_consistency` to be correct on Cloud replicas.
ClickHouse.Driver.Tcp.Tests/Integration/CompressionIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ColumnReadProjectionIntegrationTests.cs	Adds `Category("Cloud")` and skips a Cloud-locked LowCardinality setting case.
ClickHouse.Driver.Tcp.Tests/Integration/ColumnarReadSurfaceIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpTracingIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpSessionIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpParameterIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpExceptionIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpConnectionQueryIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpConnectionMetadataIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpConnectionIntegrationTests.cs	Avoids Cloud endpoint timeouts in an unreachable-port test by using loopback.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpConnectionInsertIntegrationTests.cs	Adds `Category("Cloud")`, skips Cloud-locked settings cases, and switches some Memory tables to MergeTree for multi-connection reads.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpClientQueryIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpClientIntegrationTests.cs	Adds `Category("Cloud")` and routes some option construction through `TcpServerFixture.Options()`.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpCancellationIntegrationTests.cs	Adds `Category("Cloud")`.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpCallbackIntegrationTests.cs	Adds `Category("Cloud")`.
.github/workflows/tests-tcp.yml	Triggers Cloud TCP workflow on push/dispatch and includes it in path filters.
.github/workflows/tests-cloud-tcp.yml	New reusable workflow to run `Category=Cloud` TCP tests against ClickHouse Cloud using a TLS connection string.
.github/workflows/reusable-tcp.yml	Updates setup-dotnet action version and ensures Cloud category also runs in container jobs (no exclusion filter).

Review details

Files reviewed: 31/31 changed files
Comments generated: 1
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+            using var client = new ClickHouseTcpClient(ClickHouseTcpClientOptions.FromConnectionString(connectionString));
+            object version = client.ExecuteScalarAsync("SELECT version()").GetAwaiter().GetResult();
+            return Parse(version as string);
+        }


Split out of the B9 TLS change, which needed a Cloud service to prove the transport but shipped only a five-test smoke set to do it. The suite's usual connection string points at the service instead. CLICKHOUSE_TCP_CONNECTION supersedes the CLICKHOUSE_TCP_HOST variables and the container, and carries TLS, so no second fixture and no second code path is needed. CLICKHOUSE_TEST_ENVIRONMENT=cloud makes the fixture refuse a connection string without UseTls, so a run cannot pass in the clear against a public endpoint. The Cloud category marks the tests the Cloud job also runs, not tests that need a Cloud service, matching ClickHouse.Driver.Tests. The standard matrix runs them too, against the container, so reusable-tcp.yml drops its Category!=Cloud filter. Tests/Cloud goes: the integration suite already covers ping, a multi-block query, a MergeTree round trip, session reuse over a TEMPORARY table, and concurrency. Two things a Cloud service does that a container does not, both found by running this: Cloud refuses to let a query change allow_suspicious_low_cardinality_types, answering "should not be changed" rather than applying a value. SkipIfCloudLocksASetting ignores a test asking for a setting on that list, keyed on the settings a test applies rather than on test names. Concurrent inserts commit on whichever replica their connection reached, so a count needs select_sequential_consistency to include all of them. On a single server there is nothing to wait for and the setting does nothing. Cloud: 1357 passed, 11 skipped, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Ten more fixtures, chosen for what a Cloud service does differently rather than for coverage: session identity through a load balancer, the Cancel packet reaching the server through it, an interrupted insert giving its pool slot back, compression in both directions inside the tunnel, an unframed Exception packet among framed data blocks, per-block prefixes where the server decides the block split, a non-UTC column read back against the server's own rendering, the trace id the server records for a client that set one, the error codes a different server build carries, and the schema of the Log and ProfileEvents blocks it sends. Left out on purpose. Per-type and conversion fixtures repeat what the corpus already round-trips there. Version-gated types (QBit, Geometry) would run unconditionally, because a Cloud run pins no CLICKHOUSE_VERSION and unpinned means "assume every feature". ReadonlyUserIntegrationTests needs CREATE USER and a readonly profile, which is Cloud's access model rather than the client's. Client-side lifecycle, dependency injection and logging fixtures do not depend on the deployment. TlsTransportIntegrationTests stays out for a concrete reason, now recorded on it: its proxy dials the fixture's host and port in the clear, which on Cloud is the secure port. Cloud category: 1368 -> 1806 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The first run of the widened category failed 27. Four causes, all in what the service permits rather than in the client. Ask the server for its version when nothing pins one. TcpServerFeatures read CLICKHOUSE_VERSION and treated "unset" as "assume every feature", which is right for a container tagged latest and wrong for Cloud, where no version can be pinned in the workflow without guessing. QBit(Int8) is gated at 26.7 and the service is older, so eight corpus cases ran and were rejected. It now asks the configured server, which also means a local run no longer needs the variable set by hand. allow_experimental_nullable_tuple_type is locked as well, under either of its two names, which takes out Nullable(Tuple(...)) and Nullable(Point) -- a Point being a Tuple underneath. Three tests build their own settings dictionary rather than carrying the corpus one, so they now call the guard directly. system.query_log is local to each replica. Both the flush and the lookup go over whichever connection the pool hands out, so a record written on one replica is looked for on another, and no retry budget fixes it. QueryLog ignores the calling test there, with clusterAllReplicas noted as what would be needed to keep the assertion. InsertAsync_DenseVariantOfAnotherVariantsAlternatives reads what it wrote over a second connection, which is one of the two shapes a Memory table cannot survive on a multi-replica service. Its two tables are MergeTree now. That shape was recorded on TcpServerFixture as hypothetical; it was not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

InsertAsync_DenseVariantWhoseValueNoTargetAlternativeAccepts reads its source over a second connection and counts the target over a third, so its two Memory tables read empty on a multi-replica service: the refusal under test never happened, because the loop that would have triggered it had no rows. MergeTree, like its sibling. Cloud: 27 failures down to this one, now 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

alex-clickhouse requested a review from mzitnik as a code owner September 3, 2026 13:58

alex-clickhouse mentioned this pull request Sep 3, 2026

TCP B9: Add TLS to the native client #563

Open

alex-clickhouse force-pushed the tcp/epic-b9-cloud branch from f23e741 to 50a78cf Compare September 3, 2026 14:41

alex-clickhouse changed the base branch from tcp/epic-b9-tls to tcp/epic-t4-docs September 3, 2026 14:41

cursor Bot reviewed Sep 3, 2026

View reviewed changes

Comment thread ClickHouse.Driver.Tcp.Tests/Integration/PocoWriteIntegrationTests.cs

cursor Bot reviewed Sep 3, 2026

View reviewed changes

Comment thread ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpConnectionInsertIntegrationTests.cs

Copilot AI lite review requested due to automatic review settings September 4, 2026 09:02

alex-clickhouse force-pushed the tcp/epic-b9-cloud branch from 72c8c7a to 3598070 Compare September 4, 2026 09:02

Copilot started reviewing on behalf of alex-clickhouse September 4, 2026 09:02 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

alex-clickhouse and others added 4 commits September 4, 2026 11:19

alex-clickhouse force-pushed the tcp/epic-b9-cloud branch from 3598070 to 44a6d8b Compare September 4, 2026 09:36

Navigation Menu

Uh oh!

TCP B9: Run the integration suite against Cloud - #611

TCP B9: Run the integration suite against Cloud#611
alex-clickhouse wants to merge 4 commits into
tcp/epic-t4-docsfrom
tcp/epic-b9-cloud

alex-clickhouse commented Sep 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

codecov Bot commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

alex-clickhouse commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this does

Two things a Cloud service does that a container does not

The Memory engine, which B9 predicted would block this

Testing

Known limits, deliberately left

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Sep 4, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alex-clickhouse commented Sep 3, 2026 •

edited

Loading

The `Memory` engine, which B9 predicted would block this