chore: stabilize TestContainerLogWithErrClosed and re-enable it for rootful CI

[malikov73]

What does this PR do?

Stabilizes TestContainerLogWithErrClosed and re-enables it for rootful CI. The test has been skipped on all of GitHub Actions since #1925 (Nov 2023).

It used fixed time.Sleep calls to wait for asynchronously streamed log messages and then asserted exact message counts — a race against CI load that made it flaky (#411) and got it skipped.

Changes:

• waitLogsQuiet helper — the initial baseline count is taken after the log stream has been quiet for 1s (bounded by a 30s deadline), instead of hoping a fixed 1s is enough for nginx startup logs to drain.
• requireNewMsgs helper — replaces the three sleep-then-assert-exact-count patterns: polls until the expected new messages arrive (up to 1 min), lets the stream settle for 1s so unexpected extra lines still surface, then asserts the exact count. The test stays exactly as strict as before — duplicate or stray messages still fail it. Polling the mutex-guarded Msgs() is used rather than the Accepted-channel handshake because the final assertions are negative ("no extra messages"), which a channel receive can't express without the same settle window.
• The fixed 3s wait after the iptables reset stays: it is not an assertion race but the deliberate window that gives the log producer time to observe the TCP reset and restart its stream. Now documented with a comment.
• The blanket GITHUB_RUN_ID skip is narrowed to rootless Docker only (XDG_RUNTIME_DIR, matching the existing rootless skips in docker_test.go) — chore: skip TestContainerLogWithErrClosed as flaky on rootless docker #1925's stated rationale was rootless flakiness, and privileged dind + iptables is environmentally incompatible there, same category as the existing rootless-Podman skip. The test returns to the rootful matrix, which this PR's own CI run validates.

Note: the // todo: remove this temporary fix (test is flaky) retry loop around dind.Endpoint() is a separate workaround and is left untouched.

Why is it important?

This is the only test covering the log-producer reconnect path (errClosed handling), and it has been effectively unprotected in CI for over two years. Found while dogfooding a flaky-test static analyzer I've been building — it flagged all three sleep-then-assert sites in this function.

If you'd rather keep #1924 open until rootless is also covered, happy to switch Closes → Relates.

Related issues

• Closes Reenable TestContainerLogWithErrClosed after flakiness is detected #1924
• Relates Flaky test: TestContainerLogWithErrClosed #411
• Relates chore: skip TestContainerLogWithErrClosed as flaky on rootless docker #1925

How to test this PR

go test -race -run 'TestContainerLogWithErrClosed$' -count=1 .

Locally (Docker via OrbStack, macOS, rootful): 11/11 consecutive green runs with -race after the change.

[netlify]

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	451d84d
🔍 Latest deploy log	https://app.netlify.com/projects/testcontainers-go/deploys/6a5b335751de1100082690cd
😎 Deploy Preview	https://deploy-preview-3817--testcontainers-go.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1adcfcc6-7a02-4f1a-b801-6f50c1ca3ab8

📥 Commits

Reviewing files that changed from the base of the PR and between 17799b1 and 451d84d.

📒 Files selected for processing (1)

• logconsumer_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• logconsumer_test.go

---

Summary by CodeRabbit

• Bug Fixes

  • Improved reliability of container log consumption tests by waiting for log output to stabilize before asserting results.
  • Added exact verification of newly consumed log lines after restart and connection reset scenarios.
  • Updated rootless Docker detection to use a more reliable host/socket check, reducing unnecessary flakes.

• Tests

  • Added a unit test to validate the rootless Docker host detection logic.

Walkthrough

The log consumer test adds quiet-period polling and exact message-delta assertions, replaces fixed-sleep counting, and detects rootless Docker through the extracted Docker host socket.

Changes

Log Consumer Test Stabilization

Layer / File(s)	Summary
Message stability helpers logconsumer_test.go	Adds helpers that wait for stable message counts and assert exact numbers of newly consumed log messages.
Stable container log assertions logconsumer_test.go	Uses stable baselines and exact one- and two-message assertions, and skips rootless Docker environments based on the Docker host socket. Includes unit coverage for rootless host detection.

Estimated code review effort: 3 (Moderate) | ~15–30 minutes

Suggested labels: bug

Poem

A rabbit watched the log stream hum,
Then waited till the counts were calm.
One hop, two hops, messages right,
Rootless clouds now skip the night.
Exact little logs—what delight!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: stabilizing the log test and re-enabling it for rootful CI.
Description check	✅ Passed	The description is detailed and directly matches the test-stabilization and re-enablement changes.
Linked Issues check	✅ Passed	The PR addresses #1924 by fixing flakiness and restoring TestContainerLogWithErrClosed for rootful CI.
Out of Scope Changes check	✅ Passed	The helpers, rootless detection, and test updates all support the stated stabilization goal and appear in scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@logconsumer_test.go`:
- Around line 260-263: Update TestContainerLogWithErrClosed to detect rootless
Docker using the resolved Docker host or socket configuration rather than
checking XDG_RUNTIME_DIR. Skip only when that resolved endpoint indicates a
rootless Docker daemon, and allow the test to run in normal PAM/systemd
sessions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2860feb0-487b-4300-a7ab-e6154c85230b

📥 Commits

Reviewing files that changed from the base of the PR and between d8cf042 and 17799b1.

📒 Files selected for processing (1)

• logconsumer_test.go