test for sql wait strategy #214
Conversation
gianarb
force-pushed
the
chore/write-test-for-sql-wait
branch
from
c3728cb
to
15d2e8f
Compare
The wait.ForSQL wait strategy does not have a testcase and I forgot about how to use it as well. I decided to write it in preparation for the work/discussion ongoing here: #166
gianarb
force-pushed
the
chore/write-test-for-sql-wait
branch
from
15d2e8f
to
7623f53
Compare
Codecov Report
@@ Coverage Diff @@
## main #214 +/- ##
==========================================
+ Coverage 68.88% 75.40% +6.51%
==========================================
Files 22 23 +1
Lines 2144 2500 +356
==========================================
+ Hits 1477 1885 +408
+ Misses 528 484 -44
+ Partials 139 131 -8
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
wait/sql.go
Outdated
| db.SetConnMaxLifetime(0) | ||
| db.SetMaxIdleConns(3) | ||
| db.SetMaxOpenConns(3) |
There was a problem hiding this comment.
Although the code LGTM, I'm a bit reluctant to hardcode this value here without giving the user the ability to set them in the wait strategy, according to their own requirements for the database under test. If we add them as part of the SQL strategy, then I think it's fine.
There was a problem hiding this comment.
Thinking about this twice, these settings should be added in a separate PR. Will send a commit on top removing them, and opening a discussion to make this configurable
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Kudos, SonarCloud Quality Gate passed!
|
|
I think I'm going to close this issue, as it's changing the default timeout values. I'd reopen it if we receive issues for the wait.ForSQL to fail under low timeout values. Thanks for your work here. |
The wait.ForSQL wait strategy does not have a testcase and I forgot
about how to use it as well.
I decided to write it in preparation for the work/discussion ongoing
here: #166