ci: enable 11 integration test packages and require documented skip reasons#4550
Conversation
These tests run fully in-process (mock OIDC, in-memory OTel exporter, a local TCP listener) with no external services, credentials, or Docker. They were auto-registered as skipped by #4436 without per-package triage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TestIntegrationRedis spins up redis via testcontainers, the same Docker infra already used by many enabled packages (e.g. internal/impl/redis). Auto-registered as skipped by #4436 without per-package triage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Redpanda-broker tests use the same testcontainers pattern as the already-enabled kafka and redpanda/migrator packages; the remaining schema-registry tests use in-process httptest mocks. Auto-registered as skipped by #4436 without per-package triage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Self-contained testcontainers stack (minio, iceberg-rest-fixture, duckdb) with local static credentials; no external services. Auto- registered as skipped by #4436 without per-package triage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Self-contained testcontainers stack (redpanda, otel collector, telemetrygen); no external services. These ran in CI until they were accidentally flipped to skipped during the skip string->bool refactor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Self-contained postgres:16 testcontainer (wal_level=logical); the parent postgresql package already runs in CI. These ran in CI until they were accidentally flipped to skipped during the skip string->bool refactor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Single self-contained authzed/spicedb testcontainer (in-memory serve- testing mode); no external services. These ran in CI until they were accidentally flipped to skipped during the skip string->bool refactor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The non-skipped sibling internal/impl/mssqlserver already runs the identical MSSQL testcontainer in CI, so the infra is proven. Add a 10m timeout to match the sibling, since CDC capture can be slow. Auto- registered as skipped by #4436 without per-package triage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The non-skipped sibling internal/impl/oracledb already runs the identical Oracle Free testcontainer in CI, so the infra is proven. Add a 30m timeout to match the sibling, since the Oracle image is slow to start. Auto-registered as skipped by #4436 without per-package triage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Self-contained testcontainer (public ghcr.io/basekick-labs/arc image) with a /health wait; no external services. Auto-registered as skipped by #4436 without per-package triage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Single self-contained neo4j testcontainer (auth disabled); no external services. Auto-registered as skipped by #4436 without per-package triage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The skip flag in packages.json was a bare bool with no rationale, so the skips that accumulated (mostly auto-registered defaults from #4436 and an accidental string->bool refactor) carried no explanation. Add a skip_reason field to the package registry and make the registry's verify command fail if any skip:true entry lacks one, so every skip is a deliberate, reviewable decision. Document the reason for each package that remains skipped: cohere/cyborgdb (external creds), tigerbeetle/zeromq (CGO), doris/ollama/redpanda (resource/flakiness), and the spanner benchmark tool (manual, real-GCP). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ector TestIntegrationOTLPWithSchemaRegistry sends generated telemetry from the telemetrygen container to the collector published on the host via host.docker.internal. That name only resolves automatically on Docker Desktop, so on the Linux CI runner the collector received 0 items and the test failed. Map host.docker.internal to the host gateway on the telemetrygen container so it resolves on Linux too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The MCP integration tests exercise the enterprise-gated authorization policy, but NewServer validates the enterprise license during construction, before the test could inject one (the post-construction InjectTestService call was too late), so they failed with a missing-license error and never ran in CI. Add a WithTestLicense option to NewServer that injects an enterprise test license into the server's resources instead of loading one from the license config, and use it from the integration tests. Production behaviour is unchanged: without the option NewServer still registers the configured license as before. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Commits Review Reviewed the registry verify logic, the JSON/struct tag changes, the otlp container modifier, and the mcp option wiring. The verify loop correctly ranges over the package map and reports undocumented skips; the otlp import matches the path used across the repo; and the mcp option leaves the production license path unchanged. LGTM |
The PR-level integration job derives its package filter from the pull request's changed paths. A PR that only touches packages without integration tests (or only skipped ones) matches nothing, and the runner returned an error, failing the job spuriously. Log a message and exit 0 when no registered, non-skipped package matches the filter, so those PRs are not blocked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // serverOptions holds optional NewServer configuration. | ||
| type serverOptions struct { | ||
| injectTestLicense bool | ||
| } | ||
|
|
||
| // ServerOption customises optional NewServer behaviour. | ||
| type ServerOption func(*serverOptions) | ||
|
|
||
| // WithTestLicense injects an enterprise test license into the server's | ||
| // resources instead of loading one from the license configuration. It exists | ||
| // so tests can exercise enterprise-gated behaviour (such as authorization | ||
| // policies) without a real signed license. | ||
| func WithTestLicense() ServerOption { | ||
| return func(o *serverOptions) { o.injectTestLicense = true } | ||
| } |
There was a problem hiding this comment.
This introduces a functional-options pattern (ServerOption/WithTestLicense/variadic opts), which deviates from the project's documented Go convention:
Config Objects Over Functional Options — Prefer Config structs over the functional options pattern. Config structs are explicit, inspectable, and straightforward. Functional options add indirection without meaningful benefit for this codebase.
(see .claude/agents/godev.md)
Since there's only a single boolean knob, consider threading it through a small config struct (or an explicit parameter) consistent with the surrounding code rather than adding the option machinery.
|
Commits Review
|
2 participants
What
Enable 11 integration-test packages that were silently skipped in the daily CI runner, and add a check that requires every remaining skip to carry a documented reason.
Background
The runner's
packages.jsonmarked 19 packagesskip:truewith no rationale. Investigation showed these were mostly mechanical artifacts — packages auto-registered as skipped by the discovery tool, plus four flipped to skipped during askipstring→bool refactor that dropped the reason strings — not deliberate decisions.Changes
Enabled (CI-verified green), one commit each:
mcp,secrets,confluent,iceberg,otlp,pglogicalstream,spicedb,mssqlserver/replication,oracledb/replication,arc,cypherTwo bugs fixed while enabling these:
mcp: the integration tests exercise the enterprise-gated authorization policy, butNewServervalidated the license during construction — before the test could inject one. Added aWithTestLicenseoption (production behaviour unchanged).otlp:telemetrygenreached the collector viahost.docker.internal, which does not resolve on Linux runners. Mapped it to the host gateway.Documented-skip gate:
Added a
skip_reasonfield and madeintegration_test_registry verifyfail if anyskip:truepackage lacks one. The 8 packages that remain skipped are now documented:cohere/cyborgdb(external creds),tigerbeetle/zeromq(CGO vsCGO_ENABLED=0),doris/ollama/redpanda(resource/flakiness),spanner/benchmark(manual tool).All enabled packages were verified green via a filtered
workflow_dispatchrun.🤖 Generated with Claude Code