Repository navigation
Fix WaitAllStrategy child timeout preservation in maximum outer timeout mode - #12101
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthrough
ChangesWaitAllStrategy timeout behavior
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The timeout change appears ready for normal validation; no merge-blocking issue was identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
kiview
left a comment
There was a problem hiding this comment.
Thanks, LGTM.
Netlify failures are unrelated. Will need to check, maybe something is broken with our Netlify build.
Summary
WaitAllStrategycurrently overwrites existing child strategy timeouts whenwithStartupTimeout(...)is called inWITH_MAXIMUM_OUTER_TIMEOUTmode. Setting the outer timeout before adding the children preserves their timeouts, making the behavior depend on configuration order.Restrict timeout propagation in
withStartupTimeout(...)toWITH_OUTER_TIMEOUT, matchingwithStrategy(...). This preserves individual child timeouts in maximum outer timeout mode while continuing to update the overall timeout.The default mode's timeout propagation and the restriction on changing the timeout in
WITH_INDIVIDUAL_TIMEOUTS_ONLYremain unchanged. No public API changes are introduced.Regression coverage
WITH_INDIVIDUAL_TIMEOUTS_ONLYstrategy without attempting to overwrite its timeout.The new tests do not require Docker or depend on elapsed time.
Validation
./gradlew :testcontainers:test --tests org.testcontainers.containers.wait.strategy.WaitAllStrategyTest --rerun-tasks./gradlew checkstyleMain checkstyleTest spotlessApplygit diff --checkFixes #12100