#391 build test fixes

[alexanderankin]

this pull request updates the code so that tests are green when code is correct. it was flaky before, and some code outdated, giving false negative result

• nginx - fix implementation - add waiting
• give elastic more memory so it is more likely to succeed
• selenium - update argument name (their constructor api changed)
• formatting
• comment out pyyaml dep in docker-compose module (also comment all tests)

opened re: #391

[alexanderankin]

ipv6 idea (not includeded in PR):

diff --git a/nginx/testcontainers/nginx/__init__.py b/nginx/testcontainers/nginx/__init__.py
index d0680f1..c4bcbf7 100644
--- a/nginx/testcontainers/nginx/__init__.py
+++ b/nginx/testcontainers/nginx/__init__.py
@@ -37,5 +37,14 @@ class NginxContainer(DockerContainer):
 
     @wait_container_is_ready(urllib.error.URLError)
     def _connect(self, host: str, port: str) -> None:
-        url = urllib.parse.urlunsplit(('http', f'{host}:{port}', '', '', ''))
+        url = urllib.parse.urlunsplit(('http', self._netloc_of(host, port), '', '', ''))
         urllib.request.urlopen(url, timeout=1)
+
+    @staticmethod
+    def _netloc_of(host: str, port: str):
+        import ipaddress
+        try:
+            version = ipaddress.ip_address(host).version
+            return f"{host if version == 4 else f'[{host}]'}:{port}"
+        except ValueError:  # when "localhost" instead of ip
+            return f'{host}:{port}'

[alexanderankin]

one other potential failure is the lack of python-dev package as its necessary to compile some c code for oracle db driver, but i think this is easy enough to add once we see that this is why it fails.

[kiview]

We have dabbled with the IPv4 & IPv6 topic in the other Testcontainers languages, and I think we have a robust approach for this. But let's move it out of the scope of this PR 👍

[kiview]

Great, all tests are green now 👍

@alexanderankin could you please add a description to the PR detailing some of the changes and the rationale behind it, for future documentation?

[kiview]

I don't think these files should be manually edited, since they contain the following comment:

# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
#    pip-compile --output-file=requirements.txt --resolver=backtracking

[alexanderankin]

That is true but I cannot auto generate this with the required os/env.

[kiview]

Unfortunately, the build still fails on MacOS because of issues installing pymssql, but we can fix this in another PR.

[max-pfeiffer]

@kiview @alexanderankin Because of the issue with pymssql on MacOS: I think I can help here. It was failing on my M2 MacBook as well.

It's related to the feetds build dependency for pymssql package. You need to have freetds installed on your machine to be able to build the pymssql package.

I am using MacPorts. This way it works for me:

sudo port install freetds
export CFLAGS="-I/opt/local/include/freetds"
export LDFLAGS="-L/opt/local/lib/"
pip install pymssql==2.2.10

If you use Homebrew you need to point the flags to your library directories.

I just pulled the latest main branch and I can install the dependencies. Thanks a lot for your help here. Much appreciated.

[alexanderankin]

it seems like a strong argument to drop the dependency or make it optional

[max-pfeiffer]

Well, isn't it already optional when using the package? You could just install the features you need: pip install testcontainers-<feature>

As a software engineer I do not mind tackling with these kind of problems when working on the package and/or building it. It's a common problem on MacOs that sometimes dependencies are a bit cumbersome to install and configure.

I would advocate to add a wiki section to the project as a go-to place for such problems. Other projects like pyenv do it like this: https://github.com/pyenv/pyenv/wiki/Common-build-problems
I find that very helpful. I could provide the initial content if you guys agree to this approach.

We still need to fix the tests on MacOs. I noticed a bunch of tests failing when working my pull request #389. This is due to older image versions not supporting ARM CPUs.

Is there any chance we can get #357 merged? I would also volunteer to work on it if help is needed there.

[alexanderankin]

i think there are some neat things in this PR we should merge but its no longer productive to keep this open in its current form: closing.

[alexanderankin]

ive also very recently gotten a mac so "excited" ( 🙂 ) to get familiar with those issues and if i can, help/start to document