PR#1282: Allow to run unit tests with PostgreSQL - fm-orchestrator

fm-orchestrator

#1282 Allow to run unit tests with PostgreSQL

Merged 4 years ago by mprahl. Opened 4 years ago by cqi.

cqi/fm-orchestrator run-tests-with-pgsql into master

Allow to run tests with PostgreSQL inside container

Chenxiong Qi • 4 years ago

630f7b4

.cico-pr.pipeline

file modified

+2 -2

		`@@ -65,8 +65,8 @@`

		`stage('Run Test Suites') {`
		`timeout(20) {`
		`- onmyduffynode 'docker run -v ~/fm-orchestrator:/src:Z quay.io/factory2/mbs-test-centos'`
		`- onmyduffynode 'docker run -v ~/fm-orchestrator:/src:Z quay.io/factory2/mbs-test-fedora'`
		`+ onmyduffynode 'contrib/run-unittests.sh'`
		`+ onmyduffynode 'contrib/run-unittests.sh --py3'`
		`}`
		`}`

.dockerignore

file added

		`@@ -0,0 +1,5 @@`
		`+ .env/`
		`+ .pytest_cache/`
		`+ .tox/`
		`+ .vagrant/`
		`+ .vscode/`

conf/config.py

file modified

+2 -2

		`@@ -1,4 +1,4 @@`
		`- from os import path`
		`+ from os import environ, path`

		`# FIXME: workaround for this moment till confdir, dbdir (installdir etc.) are`
		`# declared properly somewhere/somehow`
		`@@ -98,7 +98,7 @@`
		`BUILD_LOGS_NAME_FORMAT = "build-{id}.log"`
		`LOG_BACKEND = "console"`
		`LOG_LEVEL = "debug"`
		`- SQLALCHEMY_DATABASE_URI = "sqlite://"`
		`+ SQLALCHEMY_DATABASE_URI = environ.get("DATABASE_URI", "sqlite://")`
		`DEBUG = True`
		`MESSAGING = "in_memory"`
		`PDC_URL = "https://pdc.fedoraproject.org/rest_api/v1"`

contrib/run-unittests.sh

file added

+67

		`@@ -0,0 +1,67 @@`
		`+ #!/bin/bash`
		`+ #`
		`+ # Run MBS unit tests matrix`
		`+ # \| SQLite \| PostgreSQL`
		`+ # ------------------------------`
		`+ # py2 \| x \| x`
		`+ # py3 \| x \| x`
		`+ #`
		`+ # Command line options:`
		`+ # --py3: run tests inside mbs-test-fedora container with Python 3. If not`
		`+ # set, tests will run in mbs-test-centos with Python 2 by default.`
		`+ # --with-pgsql: run tests with PostgreSQL, otherwise SQLite is used.`
		`+ #`
		`+ # Please note that, both of them can have arbitrary value as long as one of`
		`+ # them is set. So, generally, it works by just setting to 1 or yes for`
		`+ # simplicity.`
		`+`
		`+ enable_py3=`
		`+ with_pgsql=`
		`+`
		`+ for arg in "$@"; do`
		`+ case $arg in`
		`+ --py3) enable_py3=1 ;;`
		`+ --with-pgsql) with_pgsql=1 ;;`
		`+ esac`
		`+ done`
		`+`
		`+ image_ns=quay.io/factory2`
		`+ postgres_image="postgres:9.5.17"`
		`+ db_container_name="mbs-test-db"`
		`+ source_dir="$(realpath "$(dirname "$0")/..")"`
		`+ volume_mount="${source_dir}:/src:Z"`
		`+ db_name=mbstest`
		`+ db_password=mbstest`
		`+ pgdb_uri="postgresql+psycopg2://postgres:${db_password}@db/${db_name}"`
		`+ db_bg_container=`
		`+`
		`+ if [ -n "$enable_py3" ]; then`
		`+ test_image="${image_ns}/mbs-test-fedora"`
		`+ else`
		`+ test_image="${image_ns}/mbs-test-centos"`
		`+ fi`
		`+`
		`+ container_opts=(--rm -i -t -v "${volume_mount}" --name mbs-test)`
		`+`
		`+ if [ -n "$with_pgsql" ]; then`
		`+ container_opts+=(--link "${db_container_name}":db -e "DATABASE_URI=$pgdb_uri")`
		`+`
		`+ # Database will be generated automatically by postgres container during launch.`
		`+ # Setting this password makes it possible to get into database container`
		`+ # and check the data.`
		`+ db_bg_container=$(`
		`+ docker run --rm --name $db_container_name \`
		`+ -e POSTGRES_PASSWORD=$db_password \`
		`+ -e POSTGRES_DB=$db_name \`
		`+ -d \`
		`+ $postgres_image`
		`+ )`
		`+ fi`
		`+`
		`+ (cd "$source_dir" && docker run "${container_opts[@]}" $test_image)`
		`+`
		`+ rv=$?`
		`+`
		`+ [ -n "$db_bg_container" ] && docker stop "$db_bg_container"`
		`+ exit $rv`
		`+`

docker/Dockerfile-tests

file modified

		`@@ -38,6 +38,7 @@`
		`python-solv \`
		`python-sqlalchemy \`
		`python2-pungi \`
		`+ python-psycopg2 \`
		`# Test-only dependencies`
		`python-flake8 \`
		`python-mock \`

docker/Dockerfile-tests-py3

file modified

		`@@ -28,6 +28,7 @@`
		`python3-solv \`
		`python3-sqlalchemy \`
		`python3-pungi \`
		`+ python3-psycopg2 \`
		`# Test-only dependencies`
		`python3-pytest \`
		`python3-flake8 \`

docs/CONTRIBUTING.rst

file modified

+31 -14

		`@@ -1,28 +1,33 @@`
		`Running Tests`
		`=============`

		`- Since MBS requires Python dependencies that aren't available using PyPi (e.g. libsolv bindings),`
		`- there are container images (based on CentOS and Fedora) that can be used to run the code analysis and unit tests.`
		`+ Since MBS requires Python dependencies that aren't available using PyPi (e.g.`
		`+ libsolv bindings), there are container images (based on CentOS and Fedora) that`
		`+ can be used to run the code analysis and unit tests.`

		- To run the tests, you must first install `podman` with::
		+ * ``docker/Dockerfile-tests`` is based on ``centos:7``, inside which tests run
		`+ with Python 2.`

		`- $ sudo dnf install podman`
		+ * ``docker/Dockerfile-tests-py3`` is based on ``fedora:29``, inside which tests
		`+ run with Python 3.`

		`- From the repo root, run the tests with::`
		+ Both of these images are available from Quay.io under `factory2 organization`_
		+ and named ``mbs-test-centos`` and ``mbs-test-fedora`` individually. Refer to
		`+ section "Updating test images in Quay" to learn how to manage these images.`

		`- $ podman run -t --rm -v $PWD:/src:Z quay.io/factory2/mbs-test-centos`
		`+ .. _factory2: https://quay.io/organization/factory2`

		`- To run the tests with Python 3 use the image based on Fedora::`
		+ To run the tests, just simply run: ``contrib/run-unittests.sh``

		`- $ podman run -t --rm -v $PWD:/src:Z quay.io/factory2/mbs-test-fedora`
		+ By default, this script runs tests inside container ``mbs-test-centos``
		`+ with Python 2 and SQLite database.`

		`- If you need to build the container image locally use::`
		`+ There are options to change the tests enviornment:`

		`- $ podman build -t mbs-test-centos -f docker/Dockerfile-tests .`
		+ * ``--py3``: run tests with Python 3.
		+ * ``--with-pgsql``: run tests with PostgreSQL database.

		`- or::`
		`-`
		`- $ podman build -t mbs-test-fedora -f docker/Dockerfile-tests-py3 .`
		+ For example, ``contrib/run-unittests.sh --py3 --with-pgsql``.

		`Style Guide`
		`===========`
		`@@ -184,7 +189,19 @@`
		`Updating test images in Quay`
		`============================`

		`- The Quay web UI can be used to update the images used for testing:`
		`+ The docker images inside which to run tests could be built locally or via Quay`
		`+ web UI.`
		`+`
		+ For building locally, use ``podman build`` or ``docker build``. For example
		+ with ``podman``::
		`+`
		`+ $ podman build -t quay.io/factory2/mbs-test-centos -f docker/Dockerfile-tests .`
		`+`
		`+ or::`
		`+`
		`+ $ podman build -t quay.io/factory2/mbs-test-fedora -f docker/Dockerfile-tests-py3 .`
		`+`
		`+ To update the images used for testing via Quay web UI:`

		`* https://quay.io/repository/factory2/mbs-test-centos`
		`* https://quay.io/repository/factory2/mbs-test-fedora`

tox.ini

file modified

		`@@ -19,6 +19,8 @@`
		`[testenv]`
		`usedevelop = true`
		`sitepackages = true`
		`+ # Allow to switch database backend for running tests.`
		`+ passenv = DATABASE_URI`
		`whitelist_externals =`
		`flake8`
		`py.test-3`

no initial comment

cqi commented 4 years ago

With this change, developer is able to run unit tests with either SQLite or PostgreSQL locally inside container, or run the tests in CI Jenkins job.

rebased onto 5ffb048303dde26583bef69a59bc15d3607246b7

4 years ago

csomh commented 4 years ago

Hey @cqi!

Maybe you could have a look at tox-docker, as MBS already uses tox as a way to run the unit tests.

ResultsDB did a similar setup---running the tests against PostgreSQL---not so long ago:

https://pagure.io/taskotron/resultsdb/pull-request/134
https://pagure.io/taskotron/resultsdb/pull-request/137

The basic idea would be to keep things as simple as possible, in order to not to make contributors confused.

cqi commented 4 years ago

Thanks @csomh. I had a quick look at tox-docker and tried it. Seems it is not applicable for the case this patch handles. This patch ensures it is possible to run tests against SQLite (current behavior) or PostgreSQL, not only the latter one. When specify multiple images under docker, tox-docker starts them all and links them together. I don't see a way to allow developer to choose a test environment combination, like using tox option -e to choose testenvs. For example, with SQLite, only start container quay.io/factory2/mbs-test-centos, and with PostgreSQL, postgres container has to be launched as well.

Meanwhile, by using tox-docker, the source code has to be updated to assign a hardcoded PostgreSQL connection URI to conf.sqlalchemy_connection_uri according to environment variable POSTGRES_5432_TCP. I don't think this is what I want to put into code. Environment variable DATABASE_URI is more flexible for setting an alternative database connection URI than construct the URI inside source code.

What I understand is MBS has provided different ways to build development environment and run tests, one is to install docker and run tests inside container, and another one is to launch a Vagrant machine then run tests inside that VM. tox could run tests inside both of these two environments. Docker container is not the only one. I'm not sure if using tox-docker can still cover the case.

csomh commented 4 years ago

@cqi you are right: considering the way testing is currently done, tox-docker is not the right approach.

Please update docs/CONTRIBUTING.rst to tell about this way of running the tests.

mprahl commented on line 12 of conf/config.py 4 years ago

Could you please use double-quotes here? It's part of the new style guide:
https://pagure.io/fm-orchestrator/blob/master/f/docs/CONTRIBUTING.rst

mprahl commented 4 years ago

@cqi what do you think about using docker-compose instead of maintaining a custom script that somewhat duplicates what docker-compose does?

For example, this is what we use for the estuary-api:
https://github.com/release-engineering/estuary-api/blob/master/docker/docker-compose-tests.yml
https://github.com/release-engineering/estuary-api/blob/master/scripts/run-tests.sh

cqi commented 4 years ago

@mprahl I considered docker-compose actually and fianlly I found out it is not helpful too much for the case this patch handles. A major issue is this patch make it optional to run tests against SQLite or PostgreSQL, not the latter only. The script introduced in this patch is simple enough to orchestrate containers with postgres or not. If use docker-compose, we have to write a separate compose file for running tests with postgres, and still use docker run ... to run tests with SQLite. As a result, the way running tests looks like inconsistent and we also introduce extra dependency of docker-compose in the development environment.

As csomh mentioned above, I'm going to update CONTRIBUTION.rst for this new method to run tests. So, for most of the time, running tests inside container is simple to execute contrib/run-unittests.sh.

rebased onto 27deb91aac2d250d9bfc59233aa17da2f84f53a5

4 years ago

rebased onto b1dd2ea3e071beae07f25a2e2403e7e57f090282

4 years ago

1 new commit added

Run unittests with new script in CI

4 years ago

rebased onto 0619cc93327eff8fcb1c175893d0438b5a46db7e

4 years ago

cqi commented 4 years ago

I haven't added contrib/run-unittests.sh --py3 --with-pgsql into stage Run Test Suites in last commit, because python-psycopg2 is required to be installed in the image, but current mbs-test-fedora does not have it yet. After this PR is accepted and merged, new images have to be updated in Quay.io, then add the test step afterwards.