build(test): keep make ci's time limits from killing healthy runs

[taitelee]

Summary

make ci was killing healthy test runs at two go test time limits. This raises both without loosening the unit budget that CI enforces.

• Unit suite: the limit is now a UNIT_TIMEOUT variable, default 15s. make ci passes 60s for its parallel phase, where the suite shares every core with the lint and build jobs and packages such as internal/app and internal/mq (about 10s each alone) ran past 15s. make test-unit and the unit job in CI run the suite on its own and keep the 15s budget.
• Integration suite: the limit goes from 480s to 900s. tests/integration takes about 6m on a CI runner and up to 8m under Docker Desktop, which left 480s only half a minute of headroom.
• Docs: development.md gains a "Time limits" note, including that a package over the 15s budget can pass make ci and still fail in CI.

Test plan

• make ci passes locally (WSL2 + Docker Desktop) on the merge with main
• CI is green, with the unit job still at the 15s limit

Related Issues

Refs #617 (the 15s unit budget, which this keeps for standalone runs)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 201bb69f-a5c4-4569-8f26-2106dbc46822

📥 Commits

Reviewing files that changed from the base of the PR and between 60ff8cc and 85cfa43.

📒 Files selected for processing (6)

• CHANGELOG.md
• Makefile
• docs/src/content/docs/development.md
• internal/cache/redis_integration_test.go
• internal/mq/embedded_test.go
• internal/mq/external_perms_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details

⏰ Context from checks skipped due to timeout. (2)

• GitHub Check: Lint
• GitHub Check: PR title

🧰 Additional context used

📚 Code guidelines (3)

docs/src/content/docs/architecture.md — auto-discovered
AGENTS.md — auto-discovered
CONTRIBUTING.md — auto-discovered

📓 Path-based instructions (6)

Source excerpt: The **only** package that imports NATS/JetStream — a `depguard` rule in `.golangci.yml` fails `make lint` on any `github.com/nats-io` import in every package golangci-lint builds; the `integration`-tagged files under `tests/...

📄 CodeRabbit inference engine (docs/src/content/docs/architecture.md)

Files:

• internal/mq/external_perms_test.go
• internal/mq/embedded_test.go

Source excerpt: **Never hand-write `®` or `™` in prose.**

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• docs/src/content/docs/development.md

Source excerpt: Create `*_test.go` files in the same package as the code under test.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• internal/mq/external_perms_test.go
• internal/cache/redis_integration_test.go
• internal/mq/embedded_test.go

Source excerpt: Create the package under `internal/`.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• internal/mq/external_perms_test.go
• internal/cache/redis_integration_test.go
• internal/mq/embedded_test.go

Source excerpt: **WH001 applies to every tracked Markdown file, with no carve-out** — `AGENTS.md`, `CHANGELOG.md`, `.github/` CI docs and `.claude/` agent prompts included.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• docs/src/content/docs/development.md
• CHANGELOG.md

Source excerpt: **Docs prose**: never hard-wrap Markdown — one paragraph is one line.

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Files:

• docs/src/content/docs/development.md
• CHANGELOG.md

🪛 checkmake (0.3.2)

Makefile

[warning] 768-768: Target body for "test-unit" exceeds allowed length of 5 lines (6).

(maxbodylength)

🪛 LanguageTool

CHANGELOG.md

[typographical] ~100-~100: Consider using an em dash in dialogues and enumerations.
Context: - **make ci no longer kills healthy tes...

(DASH_RULE)

---

[style] ~102-~102: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...nalNATS_AcksUnderBothAckSubjectLayouts no longer fails on an ack that races the end of the test** (internal/mq/external_perms_test.go): the test published a second row to read the ack subject's layout off the history stream, and the worker's consumer, left running by the drain before it, took that row from the work queue and acked it as the test's context ended (context canceled`, 9 of 40 runs alone). It now reads the layout off the histo...

(TOO_LONG_SENTENCE)

🔇 Additional comments (9)

internal/cache/redis_integration_test.go (4)

61-71: Resolved: past comment about ctx is addressed.

startContainer creates ctx once and passes it to runContainer, Host and MappedPort. This matches the author's stated plan, so no further change is needed.

---

128-151: 📐 Maintainability & Code Quality | 💤 Low value

Fix the retry loop. A failed start can leave a stale container, and the cluster port is hard-coded.

The retry design is sound. Two details need attention.

1. runContainer registers CleanupContainer on every attempt, including failed ones. CleanupContainer accepts a nil or partial container, so this is safe. Cleanup of the failed containers is deferred to test end, so up to four dead containers stay until then. This is acceptable.
2. All attempts use --cluster-port 16379 inside the container. This is container-internal and is not published, so there is no conflict. freePort excludes host port 16379 only. No change is needed.

The retry logic itself is correct. After attempt == clusterPortPicks or a non-refusal error, require.NoError stops the test through t.FailNow, so the log line and the next iteration do not run. The control flow is correct.

One robustness gap remains. The address already in use match in portRefused is broad. It can also match a failure inside the container, for example Redis failing to bind its own port. That case would be retried up to five times before failing. The consequence is only a slower failure, and the log shows the real error.

---

73-84: LGTM!

---

166-179: LGTM!

internal/mq/embedded_test.go (1)

666-672: LGTM!

Also applies to: 1073-1075, 1101-1103, 1544-1549

internal/mq/external_perms_test.go (1)

120-123: LGTM!

Makefile (1)

747-765: LGTM!

Also applies to: 772-772, 786-786, 878-878

docs/src/content/docs/development.md (1)

336-337: LGTM!

CHANGELOG.md (1)

100-100: LGTM!

---

📝 Summary

Summary by CodeRabbit

• Tests
  • Improved reliability of automated tests involving message queues, external brokers, and Redis Cluster.
  • Added configurable time limits for unit and integration test runs, with a longer unit-test limit during CI.
• Documentation
  • Documented test time limits and how to configure them.

Walkthrough

The pull request updates unit and integration test timeouts, adds retries for Redis Cluster host-port refusals, and adjusts embedded- and external-NATS test fixtures.

Changes

Test Timeouts

Layer / File(s)	Summary
Configure test timeouts Makefile, docs/src/content/docs/development.md, CHANGELOG.md	Make targets use configurable timeouts: 15 seconds for standalone unit tests, 60 seconds for the CI parallel phase, and 900 seconds for integration tests. The documentation and changelog describe these settings.

Redis Cluster Test Startup

Layer / File(s)	Summary
Retry refused host ports internal/cache/redis_integration_test.go, CHANGELOG.md	Container startup errors are returned to cluster setup. It retries recognized host-port refusals up to five times, fails immediately for other startup errors, and reports failure after five refused picks.

NATS Test Fixtures

Layer / File(s)	Summary
Update NATS test fixtures internal/mq/embedded_test.go, internal/mq/external_perms_test.go, CHANGELOG.md	Embedded-NATS tests retain additional streams during queue reopen and stream cleanup. The external-NATS ack-layout test reads the drained row from the history stream instead of publishing another row.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Other

Suggested reviewers: jfwoods

Merge Risk: ⚪ Minimal · up to 85cfa

The timeout and test-fixture changes appear mergeable after normal checks; no actionable issue is established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (3 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: adjusting make ci test time limits so healthy runs are not terminated.
Description check	✅ Passed	The description explains the unit and integration test timeout changes, documentation updates, and test plan.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

✨ Simplify code

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[github-actions]

📚 Docs preview is live → https://5134b101-wavehouse-docs.wave-rf.workers.dev

• Commit — 85cfa43: test(cache): pass the context into runContainer
• Author — @taitelee
• Committed — 2026-10-06 14:00 (UTC-04:00)
• Deployed — 2026-10-06 14:11 EDT

[github-code-quality]

Code Coverage Overview

Languages: Go

Go

The overall line coverage in commit 85cfa43 in the build/make-ci-time-l... branch remains at 93%, unchanged from commit 60ff8cc in the main branch.

Show a line coverage summary of the most impacted files.

File	main 60ff8cc	build/make-ci-time-l... 85cfa43	+/-
internal/mq/external.go	84%	85%	+1%
internal/mq/nats_topology.go	91%	92%	+1%

---

_{Updated October 06, 2026 18:32 UTC}

[taitelee]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.