Replies: 3 comments 1 reply
-
Thanks for such a detailed design issue. We highly appreciate your time here. I'd say that the breaking-change alternative seems cleaner to me, even though it comes with the BC. Said that, providing nice docs and good examples about the new usage, would be a perfect combo for the BC alternative. |
Beta Was this translation helpful? Give feedback.
-
I took a look at #322 and took notice to the rename of type ForAllStrategy interface {
WithTimeoutDefault(time.Duration) StrategyTarget // Defines the default StartupTimeout for strategies which do not explcitly define a StartupTimeout
WithDeadline(time.Duration) StrategyTarget // Defines the deadline for all strategies
}
// usage
wait.ForAll(
wait.ForLog("database system is ready to accept connections").WithOccurrence(2),
wait.ForAll(
wait.ForListeningPort(port),
wait.ForSQL(port, drive, URLBuilderFn).WithQuery(";"),
).WithDeadline(10*time.Second),
).WithTimeoutDefault(20*time.Second).
WithDeadline(120*time.Second) Thoughts? |
Beta Was this translation helpful? Give feedback.
-
This discussion can be considered solved, as it has been merged and released into v0.16.0 |
Beta Was this translation helpful? Give feedback.
-
Context
This discussion reviews the existing implementation of
wait.ForAll
and is meant to answer existing questions. Any feature requests and enhancements can additionally be discussed, but should the topic should remain focused onwait.ForAll
.NOTE: For brevity the
wait.ForAll
will be described as an interface. Below is the up-to-date reference of bothStrategy
and the discussion interface describingwait.ForAll
.Question: How does
ForAllStrategy#WithStartupTimeout
behave?In the existing implementation,
ForAllStrategy#WithStartupTimeout
behaves as a deadline for all child wait strategies. This means that if we define a startup timeout of120s
on theForAll
strategy all children will have their default startup timeout of60s
or their explicit timeout.Problems
ForAll#WithStartupTimeout
acts as a Deadline60s
.Comparing Wait Strategies to
testcontainers-java
testcontainers-java
supplies 3 modes of functionality.WITH_OUT_TIMEOUT
- This timeout is applied to each individual strategy so that the container waits for a maximum of durationWITH_INDIVIDUAL_TIMEOUTS_ONLY
- This disables the outer timeout and waits for all inner strategies to hit their timeouts before exitingWITH_MAXIMUM_OUTER_TIMEOUT
- The inner strategies wait with their preconfigured timeout individually and the wait all strategy kills them, if the outer limit is reachedCurrently, in
testcontainers-go
there is only support forWITH_MAXIMUM_OUTER_TIMEOUT
which is defined as the existingForAllStrategy#WithStartupTimeout
behavior. We are unable to haveWITH_OUT_TIMEOUT
orWITH_INDIVIDUAL_TIMEOUTS_ONLY
.Suggested Design
To support all three modes I have two suggested designs. One that would require a breaking change and a second which does not.
Design which includes a breaking change
Java modes with their Go implementation:
Mode: WITH_INDIVIDUAL_TIMEOUTS_ONLY
Mode: WITH_OUTER_TIMEOUT
Mode: WITH_MAXIMUM_OUTER_TIMEOUT
All modes together
Design with backwards compatibility
Beta Was this translation helpful? Give feedback.
All reactions