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

Reuse PortForwarder container if it is running #680

Merged
merged 5 commits into from
Nov 24, 2023
Merged

Reuse PortForwarder container if it is running #680

merged 5 commits into from
Nov 24, 2023

Conversation

cristianrgreco
Copy link
Collaborator

@cristianrgreco cristianrgreco commented Nov 24, 2023

Resolves #677. Closes #678

The current implementation of the PortForwarder relies on a singleton to ensure that multiple forwarders are not started at the same time. The idea is that there is one forwarder per session.

The reliance on a singleton falls apart when test suites run in different processes. Similar then to the reaper implementation, we now check for the existence of a forwarder container and reuse it if it exists, instead of creating a new one. This resolves the conflicts reported in #677. This PR additionally handles the case where we attempt to forward the same port multiple times, by querying the forwarder container for its bound ports on error and skipping if already bound.

To do:

  • Remove duplication, e.g. getting network settings. Consider moving to container/network client
  • See if we can reset just the port forwarder module instead of all modules within the Jest test
  • See if we can unify the port forwarder test suites

Copy link

netlify bot commented Nov 24, 2023

Deploy Preview for testcontainers-node ready!

Name Link
🔨 Latest commit 5847e1b
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-node/deploys/6560a14651846c00088febe9
😎 Deploy Preview https://deploy-preview-680--testcontainers-node.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@cristianrgreco cristianrgreco added bug Something isn't working patch Backward compatible bug fix labels Nov 24, 2023
@cristianrgreco cristianrgreco marked this pull request as ready for review November 24, 2023 13:29
@cristianrgreco cristianrgreco merged commit d593d47 into main Nov 24, 2023
75 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch Backward compatible bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Race condition when chaining tests that use sshd make 2nd test fail
1 participant