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

Test: Add a container to run the tests #479

Merged
merged 8 commits into from
Oct 20, 2023

Conversation

mbway
Copy link
Contributor

@mbway mbway commented Oct 15, 2023

Description

in the process of investigating the issue with #478 I had to run the tests, but this was easier said than done as there are a lot of tricky requirements needed to set up an environment where the tests can be run.

To help with this, I set up a docker image that can be built and used to run the tests with tox.

  • the source code is mounted into the container read-only to avoid root-owned files from being added to the repo on the host
  • there are some aspects like the documentation generation which is annoying to get to be out-of-tree so in some cases hacks/workarounds are required, eg by symlinking or copying parts of the repo to a temporary copy of the repo

Checklist

This pull request is:

  • A documentation / typographical error fix
    • Good to go, no issue or tests are needed
  • A short code fix
    • Please include: Fixes: #<issue number> in the description if it solves an existing issue
      (which must include a complete example of the issue).
    • Please include tests that fail with the main branch and pass with the provided fix.
  • A new feature implementation or update an existing feature
    • Please include: Fixes: #<issue number> in the description if it solves an existing issue
      (which must include a complete example of the feature).
    • Please include tests that cover every lines of the new/updated feature.
    • Please update the documentation to describe the new/updated feature.

Install PostgreSQL and PostGIS::

$ sudo apt-get install postgresql postgis
$ sudo apt-get install postgresql postgresql-14-postgis-3 postgresql-14-postgis-3-scripts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

without installing postgresql-14-postgis-3-scripts running CREATE EXTENSION postgis fails
https://gis.stackexchange.com/questions/71302/running-create-extension-postgis-gives-error-could-not-open-extension-control-fi

@mbway
Copy link
Contributor Author

mbway commented Oct 15, 2023

why isn't psycopg2 included in requirements.txt?

@adrien-berchet
Copy link
Member

Interesting, thanks!

@adrien-berchet
Copy link
Member

Nice work!

I tried to run it locally and I had 2 errors.
The first one is when I run ./test_container/build.sh:

➜ ./test_container/build.sh        
[+] Building 0.1s (2/2) FINISHED                                                                                                                                                                    docker:default
 => [internal] load build definition from Dockerfile                                                                                                                                                          0.0s
 => => transferring dockerfile: 2B                                                                                                                                                                            0.0s
 => [internal] load .dockerignore                                                                                                                                                                             0.0s
 => => transferring context: 2B                                                                                                                                                                               0.0s
ERROR: failed to solve: failed to read dockerfile: open /var/lib/docker/tmp/buildkit-mount3630990460/Dockerfile: no such file or directory

I was able to run this script properly by running ./build.sh from the test_container directory.
But then I have the following error:

➜ ./test_container/run.sh          
starting postgres
starting mysql
 * Starting MySQL database server mysqld                                                                                                                                                                           su: warning: cannot change directory to /nonexistent: No such file or directory
                                                                                                                                                                                                            [ OK ]
waiting for mysql to start
###############################
GeoAlchemy2 Test Container

run tests with `tox --workdir /output -vv`
###############################
root@e97b7e2cfa5e:/geoalchemy2# tox --workdir /output -vv
using tox.ini: /geoalchemy2/tox.ini (pid 395)
using tox-3.21.4 from /usr/lib/python3/dist-packages/tox/__init__.py (pid 395)
GLOB start: packaging 
GLOB sdist-make: /geoalchemy2/setup.py
[412] /geoalchemy2$ /usr/bin/python3 setup.py sdist --formats=zip --dist-dir /output/dist >../output/log/GLOB-0.log
ERROR: invocation failed (exit code 1), logfile: /output/log/GLOB-0.log
==================================================================================================== log start ====================================================================================================
/usr/lib/python3/dist-packages/setuptools/installer.py:27: SetuptoolsDeprecationWarning: setuptools.installer is deprecated. Requirements should be satisfied by a PEP 517 installer.
  warnings.warn(
Traceback (most recent call last):
  File "/geoalchemy2/setup.py", line 4, in <module>
    setup(
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 152, in setup
    _install_setup_requires(attrs)
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 147, in _install_setup_requires
    dist.fetch_build_eggs(dist.setup_requires)
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 812, in fetch_build_eggs
    resolved_dists = pkg_resources.working_set.resolve(
  File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 771, in resolve
    dist = best[req.key] = env.best_match(
  File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 1056, in best_match
    return self.obtain(req, installer)
  File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 1068, in obtain
    return installer(requirement)
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 883, in fetch_build_egg
    return fetch_build_egg(self, req)
  File "/usr/lib/python3/dist-packages/setuptools/installer.py", line 87, in fetch_build_egg
    wheel.install_as_egg(dist_location)
  File "/usr/lib/python3/dist-packages/setuptools/wheel.py", line 95, in install_as_egg
    self._install_as_egg(destination_eggdir, zf)
  File "/usr/lib/python3/dist-packages/setuptools/wheel.py", line 103, in _install_as_egg
    self._convert_metadata(zf, destination_eggdir, dist_info, egg_info)
  File "/usr/lib/python3/dist-packages/setuptools/wheel.py", line 124, in _convert_metadata
    os.mkdir(destination_eggdir)
OSError: [Errno 30] Read-only file system: '/geoalchemy2_read_only/.eggs/setuptools_scm-8.0.4-py3.10.egg'

===================================================================================================== log end =====================================================================================================
ERROR: FAIL could not package project - v = InvocationError('/usr/bin/python3 setup.py sdist --formats=zip --dist-dir /output/dist', 1)

Do you know why?

@mbway
Copy link
Contributor Author

mbway commented Oct 17, 2023

  • the run.sh script can be run from anywhere but build.sh assumed it was being run from the test_container directory. I will push a fix
  • the second issue is because your host already has a .eggs directory that gets mounted in as read only. I will handle this in the entry point
    • for some reason this didn't cause a crash for me but by removing it from the repo copy in the container it should solve the issue regardless

@adrien-berchet
Copy link
Member

I also had to rm -rf GeoAlchemy2.egg-info but now it works. Maybe you can add this too in entrypoint.sh?
Apart from that everything works well! I just see some warnings from setuptools but I think it's not related to this PR.

@adrien-berchet
Copy link
Member

I tested the new version and everything works fine now!

The last improvement I can see is in the TEST.rst: in the current state I think it's not completely clear that we can use either the container or either the actual system. I think we should have one section for the test container (as you wrote it) and another for the system environment which would contain the current 'Install system dependencies', 'Set up the PostGIS database', 'Set the path to the SpatiaLite module', 'Set up the MySQL database' and 'Run Tests' sub sections. So the reader can easily understand which parts applies to the container case and which parts applies for the system case. WDYT?

@mbway
Copy link
Contributor Author

mbway commented Oct 19, 2023

I can make this change

@adrien-berchet adrien-berchet changed the title Build: Test container Test: Add a container to run the tests Oct 19, 2023
@mbway
Copy link
Contributor Author

mbway commented Oct 20, 2023

Are these changes what you were asking for?

@adrien-berchet
Copy link
Member

Are these changes what you were asking for?

Yes, exactly, thanks!

@adrien-berchet adrien-berchet merged commit 14aff4c into geoalchemy:master Oct 20, 2023
8 checks passed
@adrien-berchet
Copy link
Member

Thank you very much for this work @mbway !!!

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.

2 participants