Skip to content

Commit 2469654

Browse files
cppudgeclaude
andcommitted
docs: comment/doc accuracy sweep (audit group D)
Fourteen text-only fixes where a public contract lagged the code it describes: the coverage doc's TLS claims predate the tls-e2e dind job (mutual TLS IS CI-verified); run(client, request) described the reaper as process-global (it is per-daemon, keyed by endpoint URL); ExecOptions denied exec-with-stdin on the named pipe (message-mode pipes half-close fine - every real daemon's pipe is one); the API-pin rationale now cites the actual newest need (volume-prune all, 1.42); compose_image() no longer calls the cli image "ambassador"; the one stale test-index entry in 66 files (DockerAuthCredentialHelper suite rename); the coverage doc's audit date; ContainerSpec's long-outgrown "deliberately small"; the "stateless host config" comments x3 (the client carries a lazy version pin); Kafka's to_generic() throw list gains the topic validation; make_request's pre-Session "for now"; feature-notes' "modules remain planned"; the Http wait documents the in-container HTTPS workaround; docs/conventions.md gains the error-message style rule (lowercase clause, single-quoted values, opportunistic alignment). Pipeline: MSVC 563 units, WSL gcc 557 units, clang-tidy and clang-format clean (text-only change). Opus review: SHIP; the one recommendation applied (Container's mutable-client comment no longer overclaims "an active Session" - no Container path opens one). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0141x8KrNdD9NfmCh1vSMbpg
1 parent daa6737 commit 2469654

14 files changed

Lines changed: 54 additions & 34 deletions

docs/conventions.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ only when it is actually being followed.
3131
using testcontainers::docker::demux_all;
3232
```
3333

34+
## Error messages
35+
- A `throw` message is a lowercase clause (no leading capital, no trailing
36+
period) that names what failed first and quotes interesting values in single
37+
quotes: `throw Error("init script 'seed.sql' has an extension ...")`. Add
38+
context the catcher cannot reconstruct (the resource, the actual value) —
39+
not what the exception type already says. Existing messages are aligned
40+
opportunistically when a throw site is touched anyway; no big-bang sweep.
41+
3442
## Backlog
3543
- Known limitations and deferred work are tracked in [`TODO.md`](TODO.md), not
3644
scattered through code comments. When you spot tech debt during review, add it there.

docs/feature-notes.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,9 @@ stay independent.
5252
**API version pin** (2026-07-05) — every typed `DockerClient` method pins its path to
5353
`/v1.NN`, negotiated once per client instance the way the docker CLI does it: one unversioned
5454
`GET /_ping`, then `min(kClientApiVersion = 1.44, daemon's Api-Version header)`, compared
55-
numerically ("1.9" < "1.44"). 1.44 covers everything the library uses (newest need:
56-
`?platform=` on create, 1.41) and is the negotiation floor of daemons that dropped the old
55+
numerically ("1.9" < "1.44"). 1.44 covers everything the library uses (newest need: the
56+
`all` filter on volume prune, 1.42 — see the kClientApiVersion comment) and is the
57+
negotiation floor of daemons that dropped the old
5758
versions; an old daemon wins with its own version. Copies inherit the negotiated version (the
5859
drop-time DELETE of a `Container` handle re-uses it — no second ping); a daemon that reveals no
5960
parsable version falls back to unversioned paths (its default version, the old behavior). The
@@ -127,8 +128,8 @@ hooks → start → wait → handle) lives in `detail::Runner`; `GenericImage::s
127128
`run(to_request())`. Public `run(request)` / `run(client, request)` bootstrap the Reaper and
128129
delegate; the core skips reaper bootstrap so unit tests drive it against a canned responder.
129130
A hand-built request owns the consistency of the port trio (typed `exposed_ports` vs rendered
130-
`spec.exposed_ports` vs `publish_all_ports`). Modules (Postgres/Redis/…) remain planned as
131-
composition over `GenericImage`.
131+
`spec.exposed_ports` vs `publish_all_ports`). Modules (Postgres/Redis/…, landed 2026-07-12)
132+
are exactly that composition over `GenericImage` — see "Ecosystem modules" below.
132133

133134
**Lifecycle hooks + startup retry**`with_created/starting/started/stopping_hook`
134135
(`LifecycleHook = std::function<void(DockerClient&, const std::string& id)>`) and

docs/public-api-test-coverage.md

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Public API integration-test coverage
22

3-
Audit date: 2026-07-05. Updated the same day after the gap-closing test batch
4-
(verified live: Linux engine and Windows engine CI runs green).
3+
Audit date: 2026-07-05; last updated 2026-07-12 (the module layer and the
4+
pre-0.2.0 audit fixes; verified live: Linux and Windows engine CI runs green).
55

66
This file audits the integration-test coverage of every public interface under
77
`include/testcontainers/` against a real Docker daemon, in each of the two engine
@@ -474,9 +474,10 @@ Notes:
474474
| TLS disabled build (`TC_TLS=OFF`) ||| ✅ TlsTransport.DisabledBuildThrowsClearError + unit TransportTimeout.TlsDisabledConnectThrowsNamedError (CI: linux-minimal job) | ✅ (same tests; daemon-independent) |
475475
| `TransportTimeouts` ||| ❌ (unit-tested) ||
476476

477-
TLS is exercised against an in-process self-signed server, not a real remote
478-
daemon (see feature-notes: end-to-end TLS against a real daemon is not
479-
CI-verified). The npipe (Windows) and unix-socket (Linux) transports are
477+
TLS is exercised against an in-process self-signed server (the tests above)
478+
AND end to end in CI against a real `--tlsverify` daemon — the `tls-e2e`
479+
docker:dind job runs mutual TLS (see feature-notes). The npipe (Windows) and
480+
unix-socket (Linux) transports are
480481
exercised by every daemon-touching test in their respective modes; TCP is the
481482
default CI transport shape for the named-pipe/socket cases.
482483

@@ -545,5 +546,6 @@ connect, build-context variants, typed 404s on Linux are all now covered).
545546
5. **Remaining builder options** — Network `Builder::with_name` /
546547
`with_enable_ipv6` / `with_option`; Volume `Builder::with_driver` /
547548
`with_driver_opt`; `with_platform`; `with_create_body_patch`.
548-
6. **`TransportTimeoutError` against a real wedged endpoint** and end-to-end TLS
549-
to a real remote daemon (feature-notes.md records both as not CI-verified).
549+
6. **`TransportTimeoutError` against a real wedged endpoint** (feature-notes.md
550+
records it as not CI-verified; end-to-end TLS to a real daemon is covered by
551+
the `tls-e2e` dind job since 2026-07-11).

include/testcontainers/Container.hpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,11 @@ class Container {
287287
/// is best-effort and must never propagate, especially from the destructor).
288288
void fire_stopping() noexcept;
289289

290-
// Mutable: the client is just a stateless host config that opens a fresh
291-
// connection per call, so issuing requests through it is logically const
292-
// from the handle's point of view (const accessors like get_host_port /
293-
// is_running / logs need it).
290+
// Mutable: requests through the client are logically const from the
291+
// handle's point of view (const accessors like get_host_port / is_running
292+
// / logs need it). The client is connection-per-request; its lazy state
293+
// (the negotiated API-version pin) is instance-internal under the usual
294+
// one-instance-one-thread rule.
294295
mutable DockerClient client_;
295296
std::string id_;
296297
bool dropped_ = false;

include/testcontainers/ContainerRequest.hpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,10 @@ Container run(const ContainerRequest& request);
9898

9999
/// As above, but on a caller-supplied client — for a custom daemon endpoint or
100100
/// tuned transport timeouts. The reaper is booted on `client`'s daemon (not the
101-
/// environment one); note it is process-global and binds to the FIRST daemon it
102-
/// starts against — later runs against a different daemon in the same process
103-
/// get labels but no crash-safe reaping. The client is copied into the
104-
/// returned handle.
101+
/// environment one); reapers are per-daemon, keyed by endpoint URL, so a
102+
/// second daemon used in the same process gets its own crash-safe reaper (two
103+
/// URL spellings of one daemon count as two endpoints — harmless, two
104+
/// reapers). The client is copied into the returned handle.
105105
Container run(DockerClient client, const ContainerRequest& request);
106106

107107
} // namespace testcontainers

include/testcontainers/DockerComposeContainer.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,8 @@ class DockerComposeContainer {
290290
/// The selected client kind (default Local).
291291
ComposeClientKind client_kind() const noexcept { return client_kind_; }
292292

293-
/// The containerised ambassador image.
293+
/// The containerised client's cli image ("ambassador" is the socat relay
294+
/// — see ambassador_image()).
294295
const std::string& compose_image() const noexcept { return compose_image_; }
295296

296297
/// The environment variables set on the compose invocation.

include/testcontainers/ExecOptions.hpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,10 @@ struct ExecOptions {
2929
/// interaction.
3030
bool detach = false;
3131
/// When set, attach stdin, feed these bytes, then half-close so the reader
32-
/// sees EOF. Requires a half-closable transport (TCP / unix socket): on the
33-
/// Windows named-pipe and TLS transports exec throws instead (no EOF signal
34-
/// is possible there, so a reader like `cat` would hang forever).
32+
/// sees EOF. Requires a half-closable transport — TCP, unix socket, or a
33+
/// message-mode Windows named pipe (every real daemon's pipe is one): on
34+
/// the TLS transport and byte-mode pipes exec throws instead (no EOF
35+
/// signal is possible there, so a reader like `cat` would hang forever).
3536
std::optional<std::string> stdin_data;
3637
/// Initial console size of a `tty` exec, rows x columns. Commands that
3738
/// format to the terminal width see this size from their first output

include/testcontainers/Network.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,9 @@ class Network {
223223
/// Best-effort remove, swallowing any error. Marks the handle dropped.
224224
void drop() noexcept;
225225

226-
// Mutable for the same reason as Container: the client is a stateless host
227-
// config that opens a fresh connection per call.
226+
// Mutable for the same reason as Container: requests through the client
227+
// are logically const; its lazy state (the negotiated API-version pin) is
228+
// instance-internal under the one-instance-one-thread rule.
228229
mutable DockerClient client_;
229230
std::string id_;
230231
std::string name_;

include/testcontainers/Volume.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,9 @@ class Volume {
138138
/// Best-effort remove, swallowing any error. Marks the handle dropped.
139139
void drop() noexcept;
140140

141-
// Mutable for the same reason as Container/Network: the client is a stateless
142-
// host config that opens a fresh connection per call.
141+
// Mutable for the same reason as Container/Network: requests through the
142+
// client are logically const; its lazy state (the negotiated API-version
143+
// pin) is instance-internal under the one-instance-one-thread rule.
143144
mutable DockerClient client_;
144145
std::string name_;
145146
bool dropped_ = false;

include/testcontainers/WaitFor.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ struct Exit {
4343
struct Healthcheck {};
4444

4545
/// Wait until an HTTP GET to the mapped host port returns `expected_status`.
46+
/// Plain HTTP only — probe an HTTPS-serving container in-container instead,
47+
/// e.g. `successful_shell_command("curl -fsk https://localhost:8443/health")`.
4648
struct Http {
4749
std::string path = "/"; ///< request path
4850
ContainerPort port; ///< container port to probe

0 commit comments

Comments
 (0)