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