feat: allow overriding the default startup timeout - #3846
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughThe PR adds global startup-timeout configuration through a property or environment variable. Wait strategies use the configured duration, while explicit per-strategy overrides remain authoritative. ChangesStartup timeout configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/features/wait/introduction.md (1)
27-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument environment-variable precedence.
The tests establish that
TESTCONTAINERS_STARTUP_TIMEOUToverridesstartup.timeout, but this sentence only says “or”. State the precedence explicitly when both values are set.🤖 Prompt for 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. In `@docs/features/wait/introduction.md` around lines 27 - 28, Update the wait strategy documentation to state explicitly that TESTCONTAINERS_STARTUP_TIMEOUT takes precedence over the startup.timeout property when both are configured, while preserving the existing exception for strategies using WithStartupTimeout.
🤖 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 `@internal/config/config.go`:
- Around line 166-169: Validate startup timeout values as strictly positive in
the configuration loading logic, including both TESTCONTAINERS_STARTUP_TIMEOUT
parsing and startup.timeout property handling. Reject negative and zero
durations so invalid values do not override the 60-second default, while
preserving any separately defined explicit no-timeout behavior.
In `@options.go`:
- Around line 396-405: Update WithWaitStrategy and WithAdditionalWaitStrategy to
route the configured timeout through WithStartupTimeoutDefault instead of the
*AndDeadline helpers, preserving explicit WithStartupTimeout values on
individual strategies. Keep the explicit deadline helpers for cases requiring a
hard outer deadline, and add tests covering both convenience helpers with a
strategy-specific timeout longer than a shorter global StartupTimeout.
---
Nitpick comments:
In `@docs/features/wait/introduction.md`:
- Around line 27-28: Update the wait strategy documentation to state explicitly
that TESTCONTAINERS_STARTUP_TIMEOUT takes precedence over the startup.timeout
property when both are configured, while preserving the existing exception for
strategies using WithStartupTimeout.
🪄 Autofix
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 Plus
Run ID: 6f58d7d4-32c8-4488-bc0d-b91bc4f49300
📒 Files selected for processing (5)
docs/features/wait/introduction.mdinternal/config/config.gointernal/config/config_test.gooptions.gowait/wait.go
| startupTimeoutEnv := os.Getenv("TESTCONTAINERS_STARTUP_TIMEOUT") | ||
| if timeout, err := time.ParseDuration(startupTimeoutEnv); err == nil { | ||
| config.StartupTimeout = timeout | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate config.go and call sites =="
fd -a 'config\.go' . | sed 's#^\./##'
echo
echo "== internal/config/config.go outline =="
ast-grep outline internal/config/config.go --view expanded || true
echo "== relevant config.go lines =="
cat -n internal/config/config.go | sed -n '130,190p'
echo "== usages of StartupTimeout =="
rg -n "StartupTimeout|cmp\.Or|TESTCONTAINERS_STARTUP_TIMEOUT|WaitDeadline|WaitTimeout" internal . --glob '!vendor/**' --glob '!node_modules/**' | head -200Repository: testcontainers/testcontainers-go
Length of output: 25337
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re, subprocess, os, textwrap
text = open("internal/config/config.go").read()
checks = []
checks.append(("StartupTimeout defaults present", "StartupTimeout" in text))
checks.append(("env parsed with ParseDuration", 'time.ParseDuration(startupTimeoutEnv)' in text))
checks.append(("no positive validation in parsed branch", bool(re.search(r'if\s*timeout,\s*err\s*:=\s*time\.ParseDuration\(startupTimeoutEnv\);\s*err\s*==\s*nil\s*\{\s*config\.StartupTimeout\s*=\s*timeout', text))))
checks.append(("Or in file", "cmp.Or" in text))
print("textual checks:")
for label, ok in checks:
print(f"{label}: {ok}")
py_prog = textwrap.dedent('''
package main
import (
"fmt"
"time"
)
func main() {
for _, s := range []string{"-1s", "0s", "1s", "-0s", "+0s", "NaNs"} {
v, err := time.ParseDuration(s)
fmt.Printf("%q: err=%v value=%q isNegative=%v isPositive=%v cmpOrLike=%v\n",
s, err, v, v < 0, v > 0,
(func(v time.Duration) time.Duration {
configured := true
if configured {
return v
}
return time.Minute
})(v),
)
}
}
''')
tmpdir = subprocess.check_output(["mktemp", "-d"]).decode().strip()
subprocess.run(["go", "mod", "init", "probe"], cwd=tmpdir, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
open(os.path.join(tmpdir, "main.go"), "w").write(py_prog)
res = subprocess.run(["go", "run", "."], cwd=tmpdir, text=True, capture_output=True)
print("probe output:")
print(res.stdout if res.stdout else res.stderr)
PYRepository: testcontainers/testcontainers-go
Length of output: 725
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== internal/config/config_test.go relevant sections =="
cat -n internal/config/config_test.go | sed -n '70,115p'
cat -n internal/config/config_test.go | sed -n '150,185p'
cat -n internal/config/config_test.go | sed -n '330,355p'
echo "== options.go await helpers =="
cat -n options.go | sed -n '380,415p'
echo "== wait deadline implementations =="
rg -n "defaultStartupTimeout|defaultStartupDuration|60(time\.Second|Second|second)|WithStartupTimeout|WaitUntilReady|context\.WithTimeout|time\.NewTimer" wait modules --glob '!**/test*' | head -200
echo "== Go duration parsing/probe =="
python3 - <<'PY'
import subprocess
import tempfile
from pathlib import Path
prog = '''package main
import (
"fmt"
"time"
)
func cmpOrTimeout(v time.Duration, fallback time.Duration) time.Duration {
configured := (v != 0 || (v >= 0 // no actual comparison here, just placeholder) && true) ?
(v != 0) : false // placeholder compiles nothing; replace.
_ = fallback
return v
}
func main() {
for _, s := range []string{"-1s", "0s", "1s", "-0s", "+0s", "NaNs"} {
v, err := time.ParseDuration(s)
fmt.Printf("%q err=%v value=%q positive=%v negative=%v zero=%v\n", s, err != nil, v, v > 0, v < 0, v == 0)
}
}
'''
print("placeholder only")Repository: testcontainers/testcontainers-go
Length of output: 22467
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat > /tmp/check_duration.go <<'GO'
package main
import (
"fmt"
"time"
)
func isPositive(v time.Duration) bool {
return v > 0
}
func main() {
for _, s := range []string{"-1s", "0s", "1s", "-0s", "+0s"} {
v, err := time.ParseDuration(s)
fmt.Printf("%q err=%v value=%q positive=%v\n", s, err != nil, v, isPositive(v))
}
}
GO
go run /tmp/check_duration.go
rm /tmp/check_duration.goRepository: testcontainers/testcontainers-go
Length of output: 378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== go module cmp availability/version =="
rg -n "cmp|cmp.Or|cmp\." go.mod go.sum internal options.go utils.go 2>/dev/null | head -100 || true
echo "== all cmp.Or patterns =="
rg -n "cmp\.Or\(" . --glob '!vendor/**' --glob '!node_modules/**' | head -200
echo "== inspect cmp.Or call sites with surrounding code =="
for f in options.go; do
[ -f "$f" ] && {
ast-grep outline "$f" --match WithWaitStrategy --view expanded || true
cat -n "$f" | sed -n '396,414p'
}
doneRepository: testcontainers/testcontainers-go
Length of output: 2441
🌐 Web query:
Go documentation cmp.Or compares zero values returns first non-zero value
💡 Result:
The function cmp.Or, introduced in Go 1.22, is defined in the cmp package [1][2]. It accepts a variadic list of arguments of a comparable type and returns the first argument that is not equal to the zero value for that type [1][2][3]. If all arguments provided are equal to the zero value, the function returns the zero value [1][2]. The function signature is: func OrT comparable T [1][2] It is commonly used to provide default values (similar to a coalescing operator in other languages) or to simplify multipart comparisons [4][5]. Because it is a function, it does not support short-circuit evaluation; all arguments are evaluated before being passed to cmp.Or [4]. Example usage: cmp.Or("", "default") // returns "default" cmp.Or(0, 5, 10) // returns 5 [6]
Citations:
- 1: https://pkg.go.dev/cmp
- 2: https://go.dev/src/cmp/cmp.go
- 3: cmp: add Or golang/go#60204
- 4: https://blog.carlana.net/post/2024/golang-cmp-or-uses-and-history/
- 5: https://stackoverflow.com/questions/49041845/how-to-assign-the-first-non-empty-nil-value
- 6: https://github.com/golang/go/blob/2403e594/src/cmp/cmp_test.go
Reject non-positive startup timeouts.
time.ParseDuration accepts negative durations, and cmp.Or returns -1s instead of the 60s default because it is non-zero. A negative TESTCONTAINERS_STARTUP_TIMEOUT or startup.timeout propagates into WithDeadline and expires the wait context immediately. Reject values <= 0 at both environment and property values unless an explicit zero/no-timeout contract is handled separately.
🤖 Prompt for 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.
In `@internal/config/config.go` around lines 166 - 169, Validate startup timeout
values as strictly positive in the configuration loading logic, including both
TESTCONTAINERS_STARTUP_TIMEOUT parsing and startup.timeout property handling.
Reject negative and zero durations so invalid values do not override the
60-second default, while preserving any separately defined explicit no-timeout
behavior.
There was a problem hiding this comment.
I personally disagree. A loud failure for a misconfiguration beats silently rewriting it. Happy to change it though.
There was a problem hiding this comment.
@abemedia, agreed. A silent fallback can hide a configuration error.
The concern is that zero and negative values currently become active timeouts. The better fix is to reject these values loudly, not to replace them with 1m.
Config.Read() does not return an error. Therefore, this requires an explicit failure policy, such as panicking with a message that identifies TESTCONTAINERS_STARTUP_TIMEOUT or startup.timeout. An alternative is to change the configuration API to return an error, but that has wider impact.
I withdraw the silent-fallback recommendation. 🐇
✏️ Learnings added
Learnt from: abemedia
Repo: testcontainers/testcontainers-go PR: 3846
File: internal/config/config.go:166-169
Timestamp: 2026-08-10T12:20:32.427Z
Learning: In the Go Testcontainers configuration code, invalid explicit startup-timeout configuration should fail loudly rather than silently fall back to the default. This applies to non-positive `TESTCONTAINERS_STARTUP_TIMEOUT` values and `startup.timeout` property values.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
| // WithWaitStrategy replaces the wait strategy for a container, using the configured | ||
| // startup timeout as deadline, 60 seconds by default. | ||
| func WithWaitStrategy(strategies ...wait.Strategy) CustomizeRequestOption { | ||
| return WithWaitStrategyAndDeadline(60*time.Second, strategies...) | ||
| return WithWaitStrategyAndDeadline(cmp.Or(config.Read().StartupTimeout, 60*time.Second), strategies...) | ||
| } | ||
|
|
||
| // WithAdditionalWaitStrategy appends the wait strategy for a container, using 60 seconds as deadline | ||
| // WithAdditionalWaitStrategy appends the wait strategy for a container, using the | ||
| // configured startup timeout as deadline, 60 seconds by default. | ||
| func WithAdditionalWaitStrategy(strategies ...wait.Strategy) CustomizeRequestOption { | ||
| return WithAdditionalWaitStrategyAndDeadline(60*time.Second, strategies...) | ||
| return WithAdditionalWaitStrategyAndDeadline(cmp.Or(config.Read().StartupTimeout, 60*time.Second), strategies...) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'options\.go|wait|config' | head -200
echo "== options.go relevant =="
sed -n '1,80p' options.go 2>/dev/null || true
sed -n '340,430p' options.go 2>/dev/null || true
echo "== wait all.go references =="
if [ -d testcontainers-go ]; then
rg -n "WithDeadline|StartWithStrategy|MultiStrategy|WithStartupTimeout|wait" testcontainers-go/wait -g '*.go' | head -200
else
echo "testcontainers-go dir absent"
fi
echo "== local config/option helpers =="
rg -n "StartupTimeout|WithStartupTimeout|Read\(\)\.StartupTimeout|WithWaitStrategyAndDeadline|WithAdditionalWaitStrategyAndDeadline|CustomizeRequestOption" -g '*.go' . | head -300Repository: testcontainers/testcontainers-go
Length of output: 37988
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== wait/all.go =="
cat -n wait/all.go
echo "== wait/wait.go deadline helpers =="
sed -n '1,120p' wait/wait.go | cat -n
echo "== options_test for deadline helpers =="
sed -n '740,815p' options_test.go | cat -n
echo "== WaitUntilReady implementations around Deadline =="
rg -n "func .*WaitUntilReady|Deadline|WithDeadline|StartupTimeout|WithStartupTimeoutDefault|multiStrategyTimeout" wait -g '*.go' | head -300
echo "== all_test deadline relevant =="
sed -n '1,140p' wait/all_test.go | cat -n
sed -n '140,220p' wait/any_test.go | cat -nRepository: testcontainers/testcontainers-go
Length of output: 30950
Keep strategy-specific timeouts authoritative.
The convenience helpers call WithWaitStrategyAndDeadline / WithAdditionalWaitStrategyAndDeadline, which wraps the passed strategies in a MultiStrategy and sets an outer context.WithTimeout via WithDeadline. A strategy with WithStartupTimeout(2*time.Minute) can still be cancelled when StartupTimeout is 30s. Route the configured timeout through WithStartupTimeoutDefault so explicit strategy-specific timeouts remain effective. Use the explicit helpers when the configured value must be a hard outer deadline. Add tests that combine WithStartupTimeout with a shorter global StartupTimeout for both helpers.
🤖 Prompt for 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.
In `@options.go` around lines 396 - 405, Update WithWaitStrategy and
WithAdditionalWaitStrategy to route the configured timeout through
WithStartupTimeoutDefault instead of the *AndDeadline helpers, preserving
explicit WithStartupTimeout values on individual strategies. Keep the explicit
deadline helpers for cases requiring a hard outer deadline, and add tests
covering both convenience helpers with a strategy-specific timeout longer than a
shorter global StartupTimeout.
There was a problem hiding this comment.
Here I actually agree but I followed what was already there. If reviewers agree with this finding I'll update it.
There was a problem hiding this comment.
@abemedia, I agree with the finding.
This PR changes the timeout source for the convenience helpers. Therefore, it should preserve the documented behavior of strategy-specific WithStartupTimeout values.
Please update WithWaitStrategy and WithAdditionalWaitStrategy to use WithStartupTimeoutDefault for the configured value. Keep WithWaitStrategyAndDeadline and WithAdditionalWaitStrategyAndDeadline as the hard-deadline APIs. Add the two regression tests described in the review comment.
🐇
You are interacting with an AI system.
1 participant
What does this PR do?
Adds a
TESTCONTAINERS_STARTUP_TIMEOUTenvironment variable, and the matchingstartup.timeoutproperty, to configure the default startup timeout for wait strategies. It follows the existing configuration pattern: aStartupTimeoutfield onconfig.Configwith adefault=1mstruct tag, read from~/.testcontainers.propertiesand overridden by the environment variable.The configured value feeds
wait.defaultStartupTimeout(), which every wait strategy falls back to, and the deadline applied byWithWaitStrategyandWithAdditionalWaitStrategy, replacing the hardcoded 60 seconds in both. Both resolve it withcmp.Or(config.Read().StartupTimeout, 60*time.Second), so the default is unchanged at 60 seconds when nothing is configured. Wait strategies that set their own timeout, and deadlines passed explicitly toWithWaitStrategyAndDeadline, are not affected.Why is it important?
The 60 second default is not always enough for containers that are expensive to start, or for CI runners under load. It can currently only be raised per strategy with
WithStartupTimeout, which is out of reach when a module builds its wait strategy internally, so there is no way to raise it globally without patching each call site.Related issues
How to test this PR
The new setting is covered by unit tests for the environment variable, the property, and the precedence between them:
To see it applied end to end, set it low enough to force a failure and run any module test, then unset it and re-run:
The modules
replacethe root module with../.., so they pick up these changes without a release. The property form works the same way by addingstartup.timeout=1sto~/.testcontainers.properties.Follow-ups
Modules that pass their own deadline to
WithWaitStrategyAndDeadlinedo not honour this setting:ravendb,timeplusandgrafana-lgtm. A possible follow-up could some conditional logic to only set it when the default is below the custom value set there. The other 91 modules build their strategies through the plain helpers and pick it up automatically.