From dd8c69e822c390747c247731924432cb26892d06 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 19 2020 19:45:59 +0000 Subject: [PATCH 1/2] Port pagure's test suite to pytest This allows to do things like pytest -n 6 tests/ --cov pagure to run all the tests on 6 cores with coverage. pytest seems to be faster than nose and pytest is maintained, unlike nose... Signed-off-by: Pierre-Yves Chibon --- diff --git a/dev/containers/centos7-rpms-py2 b/dev/containers/centos7-rpms-py2 index 391f5d6..c2b5fde 100644 --- a/dev/containers/centos7-rpms-py2 +++ b/dev/containers/centos7-rpms-py2 @@ -51,6 +51,7 @@ RUN \ sed -i -e "s|python3-openid;python_version>=\"3.0\"||" \ /pagure/requirements.txt && \ sed -i "/^email_validator.*/d" /pagure/requirements.txt && \ + sed -i -e 's|"alembic-3"|"alembic"|' /pagure/tests/test_alembic.py && \ cd /pagure && python setup.py build diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index 37559bf..f7277f4 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -330,7 +330,7 @@ from pagure.api import group # noqa: E402 from pagure.api import plugins # noqa: E402 if pagure_config.get("PAGURE_CI_SERVICES", False): - from pagure.api.ci import jenkins # noqa: E402 + from pagure.api.ci import jenkins # noqa: E402, F401 @API.route("/version/") @@ -603,6 +603,9 @@ def api(): if pagure_config.get("PAGURE_CI_SERVICES", False): ci_methods = [] if "jenkins" in pagure_config["PAGURE_CI_SERVICES"]: + if "jenkins" not in locals(): + # We run into this situation in the tests + from pagure.api.ci import jenkins # noqa: E402, F811 ci_methods.append(jenkins.jenkins_ci_notification) if ci_methods: diff --git a/requirements-testing.txt b/requirements-testing.txt index 494f448..678439e 100644 --- a/requirements-testing.txt +++ b/requirements-testing.txt @@ -1,14 +1,14 @@ bcrypt beautifulsoup4 black; python_version >= '3.6' # Only available on py3.6+ -coverage cryptography eventlet fedmsg flake8 mock -nose>=0.10.4 -nosexcover +pytest +pytest-cov +pytest-xdist python-fedora trololio diff --git a/tests/__init__.py b/tests/__init__.py index a3c3e19..580962f 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -233,14 +233,6 @@ def user_set(APP, user, keep_get_user=False): pagure.flask_app._get_user = old_get_user -tests_state = { - "path": tempfile.mkdtemp(prefix="pagure-tests-"), - "broker": None, - "broker_client": None, - "results": {}, -} - - def create_user(session, username, fullname, emails): """ Create an user with the provided information. Note that `emails` should be a list of emails. @@ -267,61 +259,6 @@ def _populate_db(session): create_user(session, "foo", "foo bar", ["foo@bar.com"]) -def store_eager_results(*args, **kwargs): - """A wrapper for EagerResult that stores the instance.""" - result = EagerResult(*args, **kwargs) - tests_state["results"][result.id] = result - return result - - -def setUp(): - # In order to save time during local test execution, we create sqlite DB - # file only once and then we populate it and empty it for every test case - # (as opposed to creating DB file for every test case). - session = pagure.lib.model.create_tables( - "sqlite:///%s/db.sqlite" % tests_state["path"], - acls=pagure_config.get("ACLS", {}), - ) - tests_state["db_session"] = session - - # Create a broker - broker_url = os.path.join(tests_state["path"], "broker") - - tests_state["broker"] = broker = subprocess.Popen( - [ - "/usr/bin/redis-server", - "--unixsocket", - broker_url, - "--port", - "0", - "--loglevel", - "warning", - "--logfile", - "/dev/null", - ], - stdout=None, - stderr=None, - ) - broker.poll() - if broker.returncode is not None: - raise Exception("Broker failed to start") - tests_state["broker_client"] = redis.Redis(unix_socket_path=broker_url) - - # Store the EagerResults to be able to retrieve them later - tests_state["eg_patcher"] = mock.patch("celery.app.task.EagerResult") - eg_mock = tests_state["eg_patcher"].start() - eg_mock.side_effect = store_eager_results - - -def tearDown(): - tests_state["db_session"].close() - tests_state["eg_patcher"].stop() - broker = tests_state["broker"] - broker.kill() - broker.wait() - shutil.rmtree(tests_state["path"]) - - class SimplePagureTest(unittest.TestCase): """ Simple Test class that does not set a broker/worker @@ -330,6 +267,12 @@ class SimplePagureTest(unittest.TestCase): populate_db = True config_values = {} + def store_eager_results(self, *args, **kwargs): + """A wrapper for EagerResult that stores the instance.""" + result = EagerResult(*args, **kwargs) + self.results[result.id] = result + return result + @mock.patch("pagure.lib.notify.fedmsg_publish", mock.MagicMock()) def __init__(self, method_name="runTest"): """ Constructor. """ @@ -338,6 +281,7 @@ class SimplePagureTest(unittest.TestCase): self.path = None self.gitrepo = None self.gitrepos = None + self.results = {} def perfMaxWalks(self, max_walks, max_steps): """ Check that we have not performed too many walks/steps. """ @@ -366,15 +310,41 @@ class SimplePagureTest(unittest.TestCase): perfrepo.REQUESTS = [] def setUp(self): - if self.path: - # This prevents test state leakage. - # This should be None if the previous runs' tearDown didn't finish, - # leaving behind a self.path. - # If we continue in this case, not only did the previous worker and - # redis instances not exit, we also might accidentally use the - # old database connection. - # @pingou, don't delete this again... :) - raise Exception("Previous test failed!") + + self.dbfolder = tempfile.mkdtemp(prefix="pagure-tests-") + self.dbpath = "sqlite:///%s/db.sqlite" % self.dbfolder + session = pagure.lib.model.create_tables( + self.dbpath, acls=pagure_config.get("ACLS", {}), + ) + self.db_session = session + + # Create a broker + broker_url = os.path.join(self.dbfolder, "broker") + + self.broker = broker = subprocess.Popen( + [ + "/usr/bin/redis-server", + "--unixsocket", + broker_url, + "--port", + "0", + "--loglevel", + "warning", + "--logfile", + "/dev/null", + ], + stdout=None, + stderr=None, + ) + broker.poll() + if broker.returncode is not None: + raise Exception("Broker failed to start") + self.broker_client = redis.Redis(unix_socket_path=broker_url) + + # Store the EagerResults to be able to retrieve them later + self.eg_patcher = mock.patch("celery.app.task.EagerResult") + eg_mock = self.eg_patcher.start() + eg_mock.side_effect = self.store_eager_results self.perfReset() @@ -399,7 +369,7 @@ class SimplePagureTest(unittest.TestCase): "docs_folder": "%s/repos/docs" % self.path, "enable_tickets": True, "tickets_folder": "%s/repos/tickets" % self.path, - "global_path": tests_state["path"], + "global_path": self.dbfolder, "authbackend": "gitolite3", "repobridge_binary": "/usr/libexec/repobridge", "repospanner_gitport": str(8443 + sys.version_info.major), @@ -410,11 +380,11 @@ class SimplePagureTest(unittest.TestCase): } config_values.update(self.config_values) self.config_values = config_values - config_path = os.path.join(self.path, "config") - if not os.path.exists(config_path): - with open(config_path, "w") as f: + self.config_path = os.path.join(self.path, "config") + if not os.path.exists(self.config_path): + with open(self.config_path, "w") as f: f.write(CONFIG_TEMPLATE % self.config_values) - os.environ["PAGURE_CONFIG"] = config_path + os.environ["PAGURE_CONFIG"] = self.config_path pagure_config.update(reload_config()) imp.reload(pagure.lib.tasks) @@ -426,7 +396,7 @@ class SimplePagureTest(unittest.TestCase): self.app = self._app.test_client() self.gr_patcher = mock.patch("pagure.lib.tasks.get_result") gr_mock = self.gr_patcher.start() - gr_mock.side_effect = lambda tid: tests_state["results"][tid] + gr_mock.side_effect = lambda tid: self.results[tid] # Refresh the DB session self.session = pagure.lib.query.create_session(self.dbpath) @@ -436,6 +406,11 @@ class SimplePagureTest(unittest.TestCase): self.session.rollback() self._clear_database() + self.db_session.close() + self.eg_patcher.stop() + self.broker.kill() + self.broker.wait() + # Remove testdir try: shutil.rmtree(self.path) @@ -444,7 +419,13 @@ class SimplePagureTest(unittest.TestCase): # fail during the first attempt. So just try a second time if that's # the case. shutil.rmtree(self.path) + try: + shutil.rmtree(self.dbfolder) + except: + pass + self.path = None + self.dbpath = None del self.app del self._app @@ -454,10 +435,7 @@ class SimplePagureTest(unittest.TestCase): return doc or None def _prepare_db(self): - self.dbpath = "sqlite:///%s" % os.path.join( - tests_state["path"], "db.sqlite" - ) - self.session = tests_state["db_session"] + self.session = self.db_session pagure.lib.model.create_default_status( self.session, acls=pagure_config.get("ACLS", {}) ) @@ -551,7 +529,7 @@ class Modeltests(SimplePagureTest): def tearDown(self): # pylint: disable=invalid-name """ Remove the test.db database if there is one. """ - tests_state["broker_client"].flushall() + self.broker_client.flushall() super(Modeltests, self).tearDown() def create_project_full(self, projectname, extra=None): diff --git a/tests/test_alembic.py b/tests/test_alembic.py index 1198bdf..9f79937 100644 --- a/tests/test_alembic.py +++ b/tests/test_alembic.py @@ -31,7 +31,7 @@ class TestAlembic(unittest.TestCase): """ proc1 = subprocess.Popen( - ["alembic", "history"], cwd=REPO_PATH, stdout=subprocess.PIPE + ["alembic-3", "history"], cwd=REPO_PATH, stdout=subprocess.PIPE ) proc2 = subprocess.Popen( ["grep", " (head), "], stdin=proc1.stdout, stdout=subprocess.PIPE diff --git a/tests/test_pagure_flask_dump_load_ticket.py b/tests/test_pagure_flask_dump_load_ticket.py index c970640..aefd8a8 100644 --- a/tests/test_pagure_flask_dump_load_ticket.py +++ b/tests/test_pagure_flask_dump_load_ticket.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2015 - Copyright Red Hat Inc + (c) 2015-2020 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -28,6 +28,8 @@ sys.path.insert( import pagure.lib.query import tests +from pagure.config import config as pagure_config, reload_config + class PagureFlaskDumpLoadTicketTests(tests.Modeltests): """ Tests for flask application for dumping and re-loading the JSON of @@ -166,8 +168,20 @@ class PagureFlaskDumpLoadTicketTests(tests.Modeltests): shutil.rmtree(newpath) # Test reloading the JSON - self.tearDown() - self.setUp() + + # Re-create the DB from scratch + self.session.rollback() + self._clear_database() + self.db_session.close() + del self.session + del self.db_session + + os.unlink(os.path.join(self.dbfolder, "db.sqlite")) + + self.db_session = pagure.lib.model.create_tables( + self.dbpath, acls=pagure_config.get("ACLS", {}), + ) + self._prepare_db() tests.create_projects(self.session) # Create repo diff --git a/tests/test_pagure_repospanner.py b/tests/test_pagure_repospanner.py index 2d2332e..9189710 100644 --- a/tests/test_pagure_repospanner.py +++ b/tests/test_pagure_repospanner.py @@ -179,7 +179,7 @@ class PagureRepoSpannerTests(tests.Modeltests): # TODO: Find free ports configvals = { "path": self.path, - "crosspath": tests.tests_state["path"], + "crosspath": self.dbfolder, "gitport": 8443 + sys.version_info.major, "rpcport": 8445 + sys.version_info.major, "codepath": codepath, From fe3625f0750ddbdf4469481a306fea889c32bbc0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 19 2020 19:46:51 +0000 Subject: [PATCH 2/2] From now on use tox to run the tests in the containers Signed-off-by: Pierre-Yves Chibon --- diff --git a/dev/containers/runtests_py2.sh b/dev/containers/runtests_py2.sh index a4f5262..cc7bc5b 100644 --- a/dev/containers/runtests_py2.sh +++ b/dev/containers/runtests_py2.sh @@ -22,4 +22,4 @@ echo "Last commits:" git --no-pager log -2 fi -python2 /pagure/runtests.py run -f --py2 $TESTCASE +tox . -e py27 $TESTCASE diff --git a/dev/containers/runtests_py3.sh b/dev/containers/runtests_py3.sh index d7e8781..03b41bb 100644 --- a/dev/containers/runtests_py3.sh +++ b/dev/containers/runtests_py3.sh @@ -22,4 +22,4 @@ echo "Last commits:" git --no-pager log -2 fi -python3 /pagure/runtests.py run -f --py3 $TESTCASE +tox . -e py38 $TESTCASE diff --git a/tox.ini b/tox.ini index 4b0c806..042c400 100644 --- a/tox.ini +++ b/tox.ini @@ -1,36 +1,26 @@ [tox] -envlist = py{27,35,36,37}-flask{100,01{1,2}} +envlist = py{27,35,36,37,38} skipsdist = True +# If the user is missing an interpreter, don't fail +skip_missing_interpreters = True [testenv] usedevelop = True +# Rely on site packages +sitepackages = True deps = - flask011: flask>=0.11,<0.12 - flask012: flask>=0.12,<0.13 - flask100: flask>=1.0,<1.1 -rrequirements-testing.txt - ci: pygit2<1.1.0 - ci: -rrequirements-ev.txt - ci: psycopg2<2.8 - ci: python-openid - ci: python-openid-teams - ci: python-openid-cla -install_command = pip install --no-use-pep517 {opts} {packages} + -rrequirements-ev.txt + python-openid + python-openid-teams + python-openid-cla + pytest-xdist + pytest-cov +install_command = + pip install --no-use-pep517 {opts} {packages} setenv = PAGURE_CONFIG={toxinidir}/tests/test_config PYTHONPATH={toxinidir} commands = - #nosetests --with-coverage --cover-erase --cover-package=pagure --with-pagureperf {posargs} - #nosetests {posargs} - python {toxinidir}/runtests.py run -f {posargs} - - -[testenv:timetests] -basepython = python3 -deps = - -rrequirements-testing.txt - nose-timer -commands = - nosetests --with-timer --timer-top-n 10 --timer-ok 500ms --timer-warning 1 --timer-json-file tests-duration.json {posargs} - + pytest-3 -n auto tests/