PR#2386: Run tests in virtualenvs using tox - koji

		`@@ -79,20 +79,11 @@`
		`@echo "All tests are finished for python 2&3"`

		`test2:`
		`- coverage2 erase`
		`- PYTHONPATH=.:plugins/builder/.:plugins/cli/.:cli/.:www/lib coverage2 run \`
		`- --source . -m nose tests/test_builder tests/test_cli tests/test_lib \`
		`- tests/test_plugins/testbuilder.py tests/test_plugins/testcli.py`
		`- coverage2 report`
		`- coverage2 html`
		`+ tox -e py2`
		`@echo Full coverage report at file://${CURDIR}/htmlcov/py2/index.html`

		`test3:`
		`- coverage3 erase --rcfile .coveragerc3`
		`- PYTHONPATH=hub/.:plugins/hub/.:plugins/builder/.:plugins/cli/.:cli/.:www/lib coverage3 run \`
		`- --rcfile .coveragerc3 --source . -m nose`
		`- coverage3 report --rcfile .coveragerc3`
		`- coverage3 html --rcfile .coveragerc3`
		`+ tox -e py3`
		`@echo Full coverage report at file://${CURDIR}/htmlcov/py3/index.html`

		`test-tarball:`
		`@@ -136,7 +127,7 @@`
		`twine upload dist/*`

		`flake8:`
		`- flake8`
		`+ tox -e flake8`

		`tag::`
		`git tag -a $(TAG)`

devtools/containers/README.md

file added

+23

		`@@ -0,0 +1,23 @@`
		`+ Dockerfiles for development`
		`+ ===========================`
		`+`
		`+ To facilitate in development - specifically, running tests, two Dockerfiles are`
		`+ provided:`
		`+`
		+ * [`./centos/Dockerfile`](./centos/Dockerfile) CentOS 6, for testing with python2.6
		+ * [`./fedora/Dockerfile`](./fedora/Dockerfile) Fedora 32, for testing with python3.8
		`+`
		`+ To use them, taking fedora as an example:`
		`+`
		`+ docker build -t koji_test_fedora:latest --no-cache ./devtools/containers/fedora`
		`+ docker run --rm -v $PWD:/koji --name koji_test koji_test_fedora:latest bash -c "cd /koji && tox -e flake8,py3"`
		`+`
		`+ Or CentOS as an example:`
		`+`
		`+ docker build -t koji_test_centos:latest --no-cache ./devtools/containers/centos`
		`+ docker run --rm -v $PWD:/koji --name koji_test koji_test_centos:latest bash -c "cd /koji && tox -e py2"`
		`+`
		`+ When running with Podman and SELinux enabled, use the "--security-opt`
		`+ label=disable" option:`
		`+`
		`+ podman run --rm -v $PWD:/koji --security-opt label=disable --name koji_test koji_test_fedora:latest bash -c "cd /koji && ls -l /koji && tox -e flake8,py3"`

devtools/containers/centos/Dockerfile

file added

+28

		`@@ -0,0 +1,28 @@`
		`+ FROM centos:6`
		`+ RUN \`
		`+ yum install -y \`
		`+ gcc \`
		`+ git \`
		`+ make \`
		`+ krb5-devel \`
		`+ libffi-devel \`
		`+ openssl-devel \`
		`+ pyOpenSSL \`
		`+ python-cheetah \`
		`+ python-devel \`
		`+ python-requests \`
		`+ redhat-rpm-config \`
		`+ rpm-build \`
		`+ rpm-python \`
		`+ yum-utils && \`
		`+ yum install -y epel-release && \`
		`+ yum install -y \`
		`+ python-flake8 \`
		`+ python-multilib \`
		`+ python-pip \`
		`+ python-psycopg2 \`
		`+ python-qpid-proton \`
		`+ python-requests-kerberos \`
		`+ yumdownloader && \`
		`+ pip install -U 'pip==9.0.1' && \`
		`+ pip install -U tox`

devtools/containers/fedora/Dockerfile

file added

+22

		`@@ -0,0 +1,22 @@`
		`+ FROM fedora:32`
		`+ RUN \`
		`+ dnf install -y \`
		`+ 'dnf-command(download)' \`
		`+ gcc \`
		`+ git \`
		`+ glib2-devel \`
		`+ glibc-langpack-en \`
		`+ krb5-devel \`
		`+ libffi-devel \`
		`+ libxml2-devel \`
		`+ make \`
		`+ openssl-devel \`
		`+ python3-devel \`
		`+ python3-pip \`
		`+ python3-rpm \`
		`+ python3-tox \`
		`+ redhat-rpm-config \`
		`+ rpm-build \`
		`+ rpm-devel \`
		`+ sqlite-devel \`
		`+ yum-utils`

requirements.txt

file added

		`@@ -0,0 +1,7 @@`
		`+ -e .`
		`+ Cheetah;python_version < '3.0'`
		`+ Cheetah3;python_version >= '3.0'`
		`+ psycopg2-binary;python_version >= '3.0'`
		`+ python-multilib`
		`+ python-qpid-proton`
		`+ rpm-py-installer`

test-requirements.txt

file modified

+4 -1

		`@@ -1,6 +1,9 @@`
		`+ -e .`
		`flake8`
		`flake8-import-order`
		`mock<=2.0.0`
		`- requests-mock`
		`+ requests-mock;python_version >= '2.7'`
		`+ requests-mock<1.5.0;python_version < '2.7'`
		`coverage`
		`nose`
		`+ unittest2;python_version < '3.0'`

tox.ini

file added

+75

		`@@ -0,0 +1,75 @@`
		`+ [tox]`
		`+ envlist = flake8,py2,py3`
		`+`
		`+ [testenv:flake8]`
		`+ deps =`
		`+ flake8`
		`+ # On EL6, pip would get us a flake8 that doesn't work with python2.6. The next`
		`+ # two lines let us use an RPM-based version from EPEL if it is installed`
		`+ sitepackages = true`
		`+ whitelist_externals = flake8`
		`+ # These two lines just speed things up by avoiding unnecessary setup`
		`+ skip_install=true`
		`+ usedevelop=true`
		`+ commands =`
		`+ flake8`
		`+`
		`+ [testenv]`
		`+ deps =`
		`+ -r{toxinidir}/requirements.txt`
		`+ -r{toxinidir}/test-requirements.txt`
		`+ # We need to access python-rpm, at least, and potentially more on EL6`
		`+ sitepackages = true`
		`+ # Tell the rpm-py-installer package to download binaries instead of building`
		`+ # from source`
		`+ setenv =`
		`+ RPM_PY_INSTALL_BIN=true`
		`+ # If rpm's python bindings are missing, don't continue`
		`+ # Also, because coverage might be installed system-wide and it serves as our`
		`+ # entry point, let's make sure it's installed in the virtualenv.`
		`+ commands_pre =`
		`+ {envbindir}/python -c "import rpm"`
		`+ pip install -I coverage`
		`+`
		`+ [testenv:py3]`
		`+ setenv =`
		`+ {[testenv]setenv}`
		`+ PYTHONPATH=hub/.:plugins/hub/.:plugins/builder/.:plugins/cli/.:cli/.:www/lib`
		`+ commands_pre =`
		`+ {[testenv]commands_pre}`
		`+ {envbindir}/coverage3 erase --rcfile .coveragerc3`
		`+ commands =`
		`+ {envbindir}/coverage3 run --rcfile .coveragerc3 --source . -m nose`
		`+ {envbindir}/coverage3 report --rcfile .coveragerc3`
		`+ {envbindir}/coverage3 html -d {toxinidir}/htmlcov/py3 --rcfile .coveragerc3`
		`+`
		`+ [testenv:py2]`
		`+ setenv =`
		`+ {[testenv]setenv}`
		`+ PYTHONPATH=.:plugins/builder/.:plugins/cli/.:cli/.:www/lib`
		`+ PYTHONHASHSEED=0`
		`+ commands_pre =`
		`+ {[testenv]commands_pre}`
		`+ {envbindir}/coverage2 erase`
		`+ commands =`
		`+ {envbindir}/coverage2 run --source . -m nose \`
		`+ tests/test_builder tests/test_cli \`
		`+ tests/test_plugins/test_runroot_builder.py \`
		`+ tests/test_plugins/test_save_failed_tree_builder.py \`
		`+ tests/test_plugins/test_runroot_cli.py \`
		`+ tests/test_plugins/test_save_failed_tree_cli.py`
		`+ {envbindir}/coverage2 report`
		`+ {envbindir}/coverage2 html -d {toxinidir}/htmlcov/py2`
		`+`
		`+ # This is identical to the py2 testenv, but without attempting to install`
		`+ # dependencies from requirements.txt. In practice this will mean RPM-based`
		`+ # dependencies must be installed.`
		`+ [testenv:py2-rpmdeps]`
		`+ deps =`
		`+ -r{toxinidir}/test-requirements.txt`
		`+ setenv =`
		`+ {[testenv:py2]setenv}`
		`+ commands_pre =`
		`+ {[testenv:py2]commands_pre}`
		`+ commands =`
		`+ {[testenv:py2]commands}`

zmc commented 3 years ago

I wanted to try my hand at fixing some koji bugs, but had trouble getting the tests to run so I ended up getting them to use tox, and virtualenvs. This of course won't make the tests faster or greener, but does modernize the infrastructure a bit.

In my testing, this preserved the ability to use entirely RPM versions of the dependencies, or entirely PyPA versions, or a mix. I don't know how it will fare in Jenkins, but I'll fix issues that pop up once the CI runs on this PR.

I've also provided some Dockerfiles that were very helpful for me.

tkopecek commented 3 years ago

Linked issue #2391

1 new commit added

rpm-py-installer: Download binaries

3 years ago

ktdreyer commented 3 years ago

The containers make it easier to run the Koji tests on macos :thumbsup:

I've tested this on my Fedora system and the py3 tests work. However the py2 tests fail with the centos:6 container. Here's the output: https://gist.github.com/ktdreyer/bd6f591ee923550fbf2c38a181f3a86f

I have some changes at https://pagure.io/fork/ktdreyer/koji/c/3d72decf2c03597a9a207e0cf575dbe5b087a558?branch=tox-tests-more-docs and I found out in https://pagure.io/pagure/issue/4929 that you have to explicitly enable "Pull Requests" on your fork so I can send a pull request: https://pagure.io/fork/zmc/koji/settings#projectoptions-tab

Edited 3 years ago by ktdreyer

1 new commit added

test-requirements: Refine requests-mock versions

3 years ago

zmc commented 3 years ago

@ktdreyer would you mind testing on py2 again with the commit that I just pushed?

Thanks for adding more docs! Adding those changes via a PR to a repo/branch which currently has a PR filed on another repo/branch feels like new territory to me - have you done that before? I could also just cherry-pick your commit.

ktdreyer commented 3 years ago

Thanks @zmc. I tested with the latest requests-mock pinning for py2, and that removes the test error. Now we just have this test failure: https://gist.github.com/ktdreyer/bd6f591ee923550fbf2c38a181f3a86f . I'm still trying to work out what's wrong there.

Re: the pull request in Pagure, sure, you can just cherry-pick my commit there. Whatever's easiest.

1 new commit added

devtools: add instructions for CentOS, podman, and SELinux

3 years ago

zmc commented 3 years ago

@ktdreyer I've seen that failure too, and I don't see it in the Jenkins output. Confusing. It's worth noting that the CI invokes the test in a way I don't see represented in the repo itself - meaning it's not using make test and contains its own invocations of nosetests directly.

ktdreyer commented 3 years ago

hmm, I wonder what we're doing differently than Jenkins is doing

tkopecek commented 3 years ago

Cool! I finally got to this PR. I've used tox locally, but never got it to state which would be PRable.

Jenkins setup is little bit hackish as it that instance is not so much supported. So, I'm constantly fighting with what can be installed via rpms what must be updated via pip, etc. See current config:
https://docs.pagure.org/koji/configuring_jenkins/#configuration

Maybe different version of libcomps is used in that env?

We expect that py2 tests will pass on centos6 if epel is enabled and everything is installed from rpms. So, using virtualenv could be the test, but makes sense only fo cli/lib. builder tests should be run in pure centos/epel env.

Similar situation is for f(32/rawhide). Tests should pass on rpm-based installation. Does it make sense to create some "empty" env, which will use local resources without setting up virtualenv? Not sure what is the best practice for tox here.

Anyway, running all tests on centos6 is no more supported (only builder + lib should be run there).

zmc commented 3 years ago

@tkopecek Regarding the EL6 test failure, unless I'm mistaken there's no libcomps available, so the tests would use yumcomps (and the failing test is test_import_comps_sample_yumcomps). The the docker image I'm using has yum-3.2.29-81.el6.centos.0.1.noarch, which is the latest. I'm unable to see what yum version the Jenkins builds are using, so I'm not sure how to diagnose the failure.

I'm readying a commit that will cause the EL6 tests to avoid installing requirements.txt and thus require RPMs of all dependencies to be installed. It'll still install test-requirements.txt, unless we think that's a bad idea. I'd like to figure out the test failure before adding it to the PR, though.

2 new commits added

tox: Add py2-rpmdeps testenv
Add koji's python reqs to centos6 docker image

3 years ago

zmc commented 3 years ago

It's now possible to test with RPM-only dependencies via the py2-rpmdeps testenv.

I'd love some feedback on how to figure out what's causing the comps test to fail.

tkopecek commented 3 years ago

Difference is that newer tox set PYTHONHASHSEED to random value (same behaviour as python 3.3+). Yum is affected by this (which I see as a yum bug) but this is probably fixed in newer versions as I don't see that behaviour elsewhere. As we're slowly leaving py2 I would suggest to set PYTHONHASHSEED=0 for py2 tests which works for me.

8 new commits added

tox: Set PYTHONHASHSEED=0 for py2
tox: Add py2-rpmdeps testenv
Add koji's python reqs to centos6 docker image
devtools: add instructions for CentOS, podman, and SELinux
test-requirements: Refine requests-mock versions
rpm-py-installer: Download binaries
Add Dockerfiles to aid in testing
Run tests in virtualenvs using tox

3 years ago

zmc commented 3 years ago

@tkopecek Looks like you were right! Both py2 and py2-rpmdeps pass now.

zmc commented 3 years ago

@tkopecek any more feedback?

tkopecek commented 3 years ago

It (make test) fails for me in f32 now. I've not time to look at it properly but it looks like a problem with rpm-py-installer py2/py3 versions.

writing manifest file '/tmp/pip-install-IaVpvr/rpm-py-installer/pip-egg-info/rpm_py_installer.egg-info/SOURCES.txt'
    [INFO] Installing...
    [INFO] Created working directory '/tmp/tmpKSGi8D-rpm-py-installer'
    Traceback (most recent call last):
      File "install.py", line 2202, in <module>
        main()
      File "install.py", line 2197, in main
        app.run()
      File "install.py", line 40, in run
        self.rpm_py.download_and_install()
      File "install.py", line 157, in download_and_install
        self.installer.install_from_rpm_py_package()
      File "install.py", line 948, in install_from_rpm_py_package
        self._download_and_extract_rpm_py_package()
      File "install.py", line 1098, in _download_and_extract_rpm_py_package
        self.rpm.download_and_extract(package_name)
      File "install.py", line 1684, in download_and_extract
        self.download(package_name)
      File "install.py", line 1764, in download
        Cmd.sh_e(cmd, stdout=subprocess.PIPE)
      File "install.py", line 1985, in sh_e
        raise exc
    UnicodeEncodeError: 'ascii' codec can't encode character u'\xed' in position 3: ordinal not in range(128)

Edited 3 years ago by tkopecek

zmc commented 3 years ago

Odd, make test succeeds for me inside the F32 container I made to run these, as does pip install rpm-py-installer inside a py2 virtualenv.

Would you mind putting the entire output including the invocation into a paste somewhere for me to look at?

tkopecek commented 3 years ago

Same container (from devtools/containers/fedora): https://pastebin.com/TQiMRBR9

2 new commits added

Add rpm-devel to fedora container
tox: Fix typo in py2 env

3 years ago

zmc commented 3 years ago

Thanks, @tkopecek - I see it's an issue with getting python-rpm available for python2 on F32. I've just added rpm-devel to the container, so if you rebuild it and then try the same thing again, the bindings should be compiled. Your same workflow passed both py2 and py3 for me just now, with this change.

tkopecek commented 3 years ago

Coverage and flake8 excludes needs to be updated - they now parse also .tox directory.