fix: use regex to find container by name

[hwwwi]

Issue

fix #557: [Bug]: Wrong container selected when reuse option enabled

Description

client.ContainerList will return all the container which contains the given name in the filter.
We should check all the names of the container from the list to find the right container.
(Update) The name filter uses the regex pattern to find the container, the order of the container is not guaranteed.

[mdelapenya]

@hwwwi I'm surprised that the Docker client is not retrieving the right one, but to be fair not sure if we checked the filter in the case multiple containers are run at the same time with the same prefix name.

Thanks anyway for opening this PR, I think it must be addresses. Could you please add a test demonstrating the bug? 🙏

[hwwwi]

@hwwwi I'm surprised that the Docker client is not retrieving the right one, but to be fair not sure if we checked the filter in the case multiple containers are run at the same time with the same prefix name.

Thanks anyway for opening this PR, I think it must be addresses. Could you please add a test demonstrating the bug? 🙏

@mdelapenya Thanks for the reply :)
After some research on the name filter of the Docker client, I notice that the filter uses the regular expression.
The result of the ContainerList will return all the containers containing the given name. It's same as docker container ls --filter name=someName.
Instead of checking all the names of the containers, I add a caret and dollar sign on the name for the exact match of the search.
I also add a test to demonstrate the bug.

[codecov]

Codecov Report

Merging #558 (e113f08) into main (5e65c25) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #558   +/-   ##
=======================================
  Coverage   69.16%   69.16%           
=======================================
  Files          22       22           
  Lines        2186     2186           
=======================================
  Hits         1512     1512           
  Misses        535      535           
  Partials      139      139           

Impacted Files	Coverage Δ
docker.go	71.97% <100.00%> (ø)

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

[kiview]

The fix makes sense from a technical perspective. However, regarding the reuse behavior itself, we decided in testcontainers-java that relying on the container name is not a stable indicator of equality(with the name itself potentially leading to conflicts) and we, therefore, use the hash of the container create command as the lookup key.

[mdelapenya]

The fix makes sense from a technical perspective. However, regarding the reuse behavior itself, we decided in testcontainers-java that relying on the container name is not a stable indicator of equality(with the name itself potentially leading to conflicts) and we, therefore, use the hash of the container create command as the lookup key.

@kiview what do you think if we proceed with the merge, and create a discussion/issue to refactor how the reuse container feature works: if I understood correctly, a container should be reused if and only if the container request (Go abstraction) is exactly the same, identified by a unique hash of the struct representing the request.

[kiview]

@mdelapenya

a container should be reused if and only if the container request (Go abstraction) is exactly the same, identified by a unique hash of the struct representing the request.

Yep, semantically that's it. Indeed, we can align with the testcontainers-java implementation here as well. However, you are right that merging the PR does not make the situation worse and, on the contrary, improves on an actual bug. So merging and starting work on a better implementation sounds like a sensible approach to me.