Standardize Timeout setter for wait strategies

[mniak]

Closes #314

• Renames all timeout setters to WithTimeout.
• Update usage in the repository
• Add deprecated methods with previous signature for backward compatility calling the new ones

[codecov]

Codecov Report

Merging #322 (d6b90d4) into master (dfc50d7) will decrease coverage by 3.31%.
The diff coverage is 82.35%.

❗ Current head d6b90d4 differs from pull request most recent head a4a738c. Consider uploading reports for the commit a4a738c to get more accurate results

@@            Coverage Diff             @@
##           master     #322      +/-   ##
==========================================
- Coverage   60.91%   57.59%   -3.32%     
==========================================
  Files          15       13       -2     
  Lines         985      915      -70     
==========================================
- Hits          600      527      -73     
- Misses        293      312      +19     
+ Partials       92       76      -16     

Impacted Files	Coverage Δ
wait/wait.go	100.00% <ø> (ø)
wait/host_port.go	12.06% <62.50%> (-44.08%)	⬇️
wait/http.go	9.78% <80.00%> (-46.27%)	⬇️
wait/multi.go	42.85% <80.00%> (+42.85%)	⬆️
wait/sql.go	27.27% <88.88%> (+27.27%)	⬆️
wait/log.go	79.41% <100.00%> (+3.65%)	⬆️
container.go	84.37% <0.00%> (-0.92%)	⬇️
compose.go	74.04% <0.00%> (ø)
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5bf62f...a4a738c. Read the comment docs.

[mdelapenya]

LGTM! Please check my comment about deprecation logs, as I would like to know your thoughts

Thanks for addressing this issue!

[mdelapenya]

@mniak I have one NIT (maybe personal OCD): would it be possible to add meaningful commit messages? We are currently merging the PR branch into the master branch, although with current messages I'd vote for squashing these ones into one.

@gianarb do you think we need more elaborated info in a contribution guide?

[mniak]

Sorry, @mdelapenya. I'm very used to always merging PRs with the "squash and merge" button and I forgot this is not a project of mine. I will squash it here by myself.

I think that enforcing squash-and-merge is more reliable than checking the phrasing of the commit messages of contributors, since the final approver can change the phrasing by himself in one atomic commit that represents the entire set of changes in that PR.

Seems to me that squash-and-merge is easier for everyone: contributors and approvers

[mdelapenya]

I think that enforcing squash-and-merge is more reliable than checking the phrasing of the commit messages of contributors, since the final approve can change the phrasing by himself in one atomic commit that represents the entire set of changes in that PR

Indeed. I totally agree, that's why I summoned @gianarb for the need for a contribution guide that simplifies the onboarding experience for contributors. And maybe moving to the squash model makes sense here, but would like to check with this great community first.

[mniak]

Had to move the file http_test.go to integration_tests/http_test.go.

When I tried to add a unit test for checking the timeout setters in the HTTP strategy, I found that this file used a differente package wait_test and thus could not access the private member timeout.
But I could not use the same package because it would result in a cyclical reference between packages (it was using the parent package, that used this package.
Because some tests in the root package testcontainers import the package wait, then the package wait cannot import the package testcontainers.

I then moved the file wait/http_test.go to wait/integration_tests/http_test.go and created a new wait/http_test.go with the test I intended to create (to test the timeout setter)

[mdelapenya]

@mniak could you 🙏 rebase and resolve conflicts? We solved (actually skipped) the LogConsumer test that fails. Thanks!

[mdelapenya]

Hey @mniak could your merge with upstream and resolve conflicts 🙏 ? Sorry that review took so long that conflicts appeared 😞 Let's try to make this happen

[mdelapenya]

Hi @mniak 👋 I'm revisiting this stale PR as we'd like to add it to the next release, and wonder if you are interested in resolving the conflicts. wdyt?

[mdelapenya]

@mniak I think that, after #580, this PR is not needed, as there we redefined the different timeouts and deadlines attributes for the wait strategies.

I'm closing this PR, but please feel free to reopen if you consider it

Thanks in advance for your patience here 🙏