Add waitForSqlWithHost implementation. #310
Conversation
This implementation mostly copies the prior waitForSql implementation, the difference being that the waitForSqlWithHost allows the user to construct a connection string using the host of the DB container. This is especially useful in a scenario like CI (where our tests are being executed in a container) or even remote Docker hosts. To preserve backwards-compatibility, and because of the way that `waitForSql` works, `waitForSqlWithHost` replicates the existing behaviour mostly. In a v1 of testcontainers-go, you would probably want to unify these two differing implementations.
Codecov Report
@@ Coverage Diff @@
## main #310 +/- ##
==========================================
- Coverage 69.79% 67.55% -2.25%
==========================================
Files 21 21
Lines 1947 2000 +53
==========================================
- Hits 1359 1351 -8
- Misses 472 530 +58
- Partials 116 119 +3
Continue to review full report at Codecov.
|
|
Also, we might want to wait for #214 to be merged in before we look at this. |
|
@gianarb - might be great to get your feedback on this if you get a chance! |
| } | ||
|
|
||
| //Timeout sets the maximum waiting time for the strategy after which it'll give up and return an error | ||
| func (w *waitForSqlWithHost) Timeout(duration time.Duration) *waitForSqlWithHost { |
There was a problem hiding this comment.
In PR #322 we are considering to change the name of this method to WithTimeout in the other wait strategies.
It could be renamed here as well
There was a problem hiding this comment.
I think that's a great change, I'll do that when I get a chance to.
|
@michielboekhoff I think this PR has been superseded by #524 Please let me know if I can close it, as its original intention has been already implemented and merged into main (not released yet) |
Yep, seems like it. I've closed this PR. |
This implementation mostly copies the prior waitForSql implementation, the difference being that the
waitForSqlWithHostallows the user to construct a connection string using the host of the DB container. This is especially useful in a scenario like CI (where tests are being executed in a container) or even remote Docker hosts.To preserve backwards compatibility, and because of the way that
waitForSqlworks,waitForSqlWithHostreplicates the existing behaviour mostly. In a v1 of testcontainers-go, you would probably want to unify these two differing implementations.One thing I would like to (personally) see in a v1 of testcontainers-go is that the underlying strategy struct is not exposed to the calling code (instead opting to return the
Strategyinterface) so that we can change its implementation, whereas right now we cannot, as all the fields are exposed to calling code.