Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compose): wait.ForExit() strategy should work for finished containers #514

Merged

Conversation

Malinskiy
Copy link
Contributor

Fix for the #374 (comment)

@Malinskiy Malinskiy requested a review from a team as a code owner September 1, 2022 06:40
@codecov
Copy link

codecov bot commented Sep 1, 2022

Codecov Report

Merging #514 (a279f4f) into main (004930d) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #514      +/-   ##
==========================================
+ Coverage   68.88%   68.98%   +0.09%     
==========================================
  Files          22       22              
  Lines        2144     2144              
==========================================
+ Hits         1477     1479       +2     
+ Misses        528      526       -2     
  Partials      139      139              
Impacted Files Coverage Δ
compose.go 74.04% <100.00%> (ø)
docker.go 71.03% <0.00%> (+0.20%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mdelapenya
Copy link
Collaborator

Hi @Malinskiy, thanks for your contribution, I've rebased the branch with upstream and if the CI is still ✅ will merge it. And sorry for the late response 😢

Copy link
Collaborator

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for this contribution!

I think you should be aware of #476, which refactors compose to be used natively in Go, instead of using a shelled version of compose. So at some point I'd suggest discussing with the original author about possible changes in that PR (I started it here, but you probably find others)

Thanks again for your time here!

@mdelapenya mdelapenya merged commit f684e45 into testcontainers:main Sep 14, 2022
@Malinskiy Malinskiy deleted the fix/compose-wait-service-not-found branch September 19, 2022 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants