From 8a9f4fc51958af6655024c5cf1d5ae5903e464cc Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 1/55] tests: Drop py312 from tox tests, pinned Pillow version isn't compatible --- diff --git a/tox.ini b/tox.ini index c662fd2..0ad7e54 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py{39,310,311,312} +envlist = py{39,310,311} skipsdist = True # If the user is missing an interpreter, don't fail skip_missing_interpreters = True From 3fb943ae8211bd395c0f29b979dbcb40a3d5e07c Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 2/55] tests: Additional packages in fedora pip base container to meet requirements of 'Pillow' python package (compile wheel) --- diff --git a/dev/containers/base-fedora-pip-py3 b/dev/containers/base-fedora-pip-py3 index 9433227..ec39d18 100644 --- a/dev/containers/base-fedora-pip-py3 +++ b/dev/containers/base-fedora-pip-py3 @@ -24,6 +24,9 @@ RUN dnf -y install \ which \ git \ glibc-langpack-en \ + libtiff-devel libjpeg-devel openjpeg2-devel zlib-devel \ + freetype-devel lcms2-devel libwebp-devel tcl-devel tk-devel \ + harfbuzz-devel fribidi-devel libraqm-devel libimagequant-devel libxcb-devel \ && dnf clean all # Pre-build tox environments and keep em in the base container @@ -35,4 +38,4 @@ RUN mkdir /tox \ && cd /pagure \ && tox --notest \ && cd / \ - && rm -rf /pagure \ No newline at end of file + && rm -rf /pagure From dd1681a499a1339565ad7060cd22a7d82687939e Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 3/55] tests: bump pip base container to F39 --- diff --git a/dev/containers/base-fedora-pip-py3 b/dev/containers/base-fedora-pip-py3 index ec39d18..005ec06 100644 --- a/dev/containers/base-fedora-pip-py3 +++ b/dev/containers/base-fedora-pip-py3 @@ -1,4 +1,4 @@ -FROM quay.io/fedora/fedora:36-x86_64 +FROM quay.io/fedora/fedora:39-x86_64 LABEL org.opencontainers.image.authors="pagure community" LABEL org.opencontainers.image.url="https://pagure.io/pagure" From ed52a0069e066e09f62322c3442aa608a8d50232 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 4/55] build: Remove version pinning from pygit2 --- diff --git a/requirements.txt b/requirements.txt index 5675a3c..9daa335 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,7 +22,7 @@ markdown <= 3.3.7 munch <= 2.5.0 Pillow <= 9.2.0 psutil <= 5.9.2 -pygit2 >= 0.26.0, <=1.8.0 +pygit2 python3-openid <= 3.2.0 python-openid-cla == 1.2 python-openid-teams == 1.1 From eb32c5b6943dcdc5943dd8313f465f03ad88d82a Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 5/55] build: Enable verbose/debug mode for git clone in fedora base pip container --- diff --git a/dev/containers/base-fedora-pip-py3 b/dev/containers/base-fedora-pip-py3 index 005ec06..3661987 100644 --- a/dev/containers/base-fedora-pip-py3 +++ b/dev/containers/base-fedora-pip-py3 @@ -33,7 +33,7 @@ RUN dnf -y install \ # to avoid re-build on every run, even if tox config not changed RUN mkdir /tox \ && echo Repo: ${REPO}, Branch: ${BRANCH} \ - && git clone -b ${BRANCH} ${REPO} /pagure \ + && GIT_TRACE=1 git clone -b ${BRANCH} ${REPO} /pagure \ && ln -s /tox /pagure/.tox \ && cd /pagure \ && tox --notest \ From 2d7c6620359cc4f9391a248c2b4ce1866891370c Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 6/55] build: Fix issue that build-args are not passed correctly to podman in run-tests-container.py --- diff --git a/dev/run-tests-container.py b/dev/run-tests-container.py index 34b3f66..a1dd9e3 100755 --- a/dev/run-tests-container.py +++ b/dev/run-tests-container.py @@ -26,9 +26,10 @@ def _build_container(container_name, container_type, result_path, # fmt: on # kwargs can be used to pass '--build-arg' build_args = [] - for arg in kwargs.values(): + #for arg in kwargs.values(): + for key, value in kwargs.items(): build_args.append("--build-arg") - build_args.append(arg) + build_args.append("{}={}".format(key, value)) volume = [] if container_volume: From 80fa744a28dcfe5def2c76dd2891882c9b9243ef Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 7/55] build: Fix issue that local volume wasn't mounted inside base container during build initiated by run-tests-container.py --- diff --git a/dev/run-tests-container.py b/dev/run-tests-container.py index a1dd9e3..237392e 100755 --- a/dev/run-tests-container.py +++ b/dev/run-tests-container.py @@ -34,7 +34,7 @@ def _build_container(container_name, container_type, result_path, volume = [] if container_volume: volume.append("-v") - volume.append(volume) + volume.append(container_volume) container_file = "" if container_type == "base": @@ -200,6 +200,7 @@ if __name__ == "__main__": else: container_names = ["centos", "fedora", "pip"] + mount_wrkdir = False # get full path of git repo in current directory # and set var to mount it into the container if args.repo == "/wrkdir": @@ -244,10 +245,14 @@ if __name__ == "__main__": "Container does not exist, building: %s" % containers[container_name]["base"] ) + container_volume = None + if mount_wrkdir: + container_volume = "{}:/wrkdir:z,ro".format(wrkdir_path) if _build_container( container_name, "base", result_path, + container_volume, branch="{}".format(os.environ.get("BRANCH") or args.branch), repo="{}".format(os.environ.get("REPO") or args.repo), ): From 02de0c05f066d626d2e894882f79bb706d860b25 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 8/55] build: Verbose / Debug output from tox command during base fedora pip container build --- diff --git a/dev/containers/base-fedora-pip-py3 b/dev/containers/base-fedora-pip-py3 index 3661987..c5c9c94 100644 --- a/dev/containers/base-fedora-pip-py3 +++ b/dev/containers/base-fedora-pip-py3 @@ -36,6 +36,6 @@ RUN mkdir /tox \ && GIT_TRACE=1 git clone -b ${BRANCH} ${REPO} /pagure \ && ln -s /tox /pagure/.tox \ && cd /pagure \ - && tox --notest \ + && tox --notest -vv \ && cd / \ && rm -rf /pagure diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 377bf7f..f614048 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -24,7 +24,7 @@ import pygit2 import six # from sqlalchemy.orm.session import Session -from pygit2.remote import RemoteCollection +from pygit2.remotes import RemoteCollection from sqlalchemy.exc import SQLAlchemyError import pagure.exceptions From 1cf9ec5a298e04acc738f343b29c26b36ba273a3 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 9/55] fix: pygit2 Repository.create_remote replaced by Repository.remotes.create 1.14.0 (2024-01-26) Breaking changes: Remove deprecated Repository.create_remote(...) function, use instead Repository.remotes.create(...) 1.2.1 (2020-05-01) Deprecations: Deprecate Repository.create_remote(...), use instead Repository.remotes.create(...) --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index f614048..fbe66ad 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1626,7 +1626,7 @@ def merge_pull_request(session, request, username, domerge=True): _log.info( " Adding remote: %s pointing to: %s", reponame, repopath ) - remote = new_repo.create_remote(reponame, repopath) + remote = new_repo.remotes.create(reponame, repopath) # Fetch the commits remote.fetch() @@ -1926,7 +1926,7 @@ def rebase_pull_request(session, request, username): _log.info( " Adding remote: %s pointing to: %s", upstream, upstream_path ) - remote = new_repo.create_remote(upstream, upstream_path) + remote = new_repo.remotes.create(upstream, upstream_path) # Fetch the commits remote.fetch() diff --git a/tests/__init__.py b/tests/__init__.py index 3bdbf63..9f03615 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1173,7 +1173,7 @@ def add_pull_request_git_repo( # Add the main project as remote repo upstream_path = os.path.join(folder, "repos", repo.path) - remote = clone_repo.create_remote("upstream", upstream_path) + remote = clone_repo.remotes.create("upstream", upstream_path) remote.fetch() # Edit the sources file again diff --git a/tests/test_pagure_flask_docs.py b/tests/test_pagure_flask_docs.py index 0752d3d..b828681 100644 --- a/tests/test_pagure_flask_docs.py +++ b/tests/test_pagure_flask_docs.py @@ -85,7 +85,7 @@ class PagureFlaskDocstests(tests.SimplePagureTest): ) # Push the changes to the bare repo - remote = repo.create_remote( + remote = repo.remotes.create( "origin", os.path.join(self.path, "repos", "docs", "test.git") ) diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index 8d33263..1630128 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -5440,7 +5440,7 @@ More information PagureRepo.push(ori_remote, refname) # Push to the fork repo - remote = clone_repo.create_remote("pingou_fork", gitrepo2) + remote = clone_repo.remotes.create("pingou_fork", gitrepo2) PagureRepo.push(remote, refname) # Add 1 commits to the fork repo diff --git a/tests/test_pagure_lib_git_diff_pr.py b/tests/test_pagure_lib_git_diff_pr.py index b677338..bda38cc 100644 --- a/tests/test_pagure_lib_git_diff_pr.py +++ b/tests/test_pagure_lib_git_diff_pr.py @@ -125,7 +125,7 @@ class PagureFlaskForkPrtests(tests.Modeltests): PagureRepo.push(ori_remote, refname) # Push to the fork repo - remote = clone_repo.create_remote("pingou_fork", gitrepo2) + remote = clone_repo.remotes.create("pingou_fork", gitrepo2) PagureRepo.push(remote, refname) # Do another 3 commits to the main repo From 234162b0143c1b544a4d7d653e78d90d169d46a1 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 10/55] tests: bump rpm base container to F39 --- diff --git a/dev/containers/base-fedora-rpms-py3 b/dev/containers/base-fedora-rpms-py3 index b50036d..05f138e 100644 --- a/dev/containers/base-fedora-rpms-py3 +++ b/dev/containers/base-fedora-rpms-py3 @@ -1,4 +1,4 @@ -FROM quay.io/fedora/fedora:36-x86_64 +FROM quay.io/fedora/fedora:39-x86_64 LABEL org.opencontainers.image.authors="pagure community" LABEL org.opencontainers.image.url="https://pagure.io/pagure" @@ -36,4 +36,4 @@ RUN curl ${SPECFILE} -o /pagure.spec \ && dnf install -y --enablerepo=updates-testing `grep "Requires:" /pagure.spec | \ awk '{split($0, a, " "); print a[2]}' | grep -v "%{name}" | \ sed -e "s|%{python_pkgversion}|3|"` \ - && dnf clean all \ No newline at end of file + && dnf clean all From c1740e351d5c497e1c45d7167b47aa4e0df38e13 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 11/55] build: Replace python3-cchardet with python3-chardet --- diff --git a/dev/containers/base-fedora-rpms-py3 b/dev/containers/base-fedora-rpms-py3 index 05f138e..a495da0 100644 --- a/dev/containers/base-fedora-rpms-py3 +++ b/dev/containers/base-fedora-rpms-py3 @@ -19,7 +19,7 @@ RUN dnf -y --enablerepo=updates-testing install \ python3-flake8 \ python3-pytest-xdist \ python3-flask-oidc \ - python3-cchardet \ + python3-chardet \ python3-fedora-messaging \ python3-pip \ redis \ From 8a1a7ba9f789ba446bab63783f7b963246861cb8 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 12/55] tests: Fix issue 'No module named pygit2.remote' --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index fbe66ad..98d72d0 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -23,8 +23,11 @@ import arrow import pygit2 import six -# from sqlalchemy.orm.session import Session -from pygit2.remotes import RemoteCollection +try: + from pygit2.remote import RemoteCollection +except ImportError: + from pygit2.remotes import RemoteCollection + from sqlalchemy.exc import SQLAlchemyError import pagure.exceptions diff --git a/tests/test_pagure_lib_git.py b/tests/test_pagure_lib_git.py index 78d3f94..8194962 100644 --- a/tests/test_pagure_lib_git.py +++ b/tests/test_pagure_lib_git.py @@ -3639,8 +3639,12 @@ index 0000000..60f7480 # make sure the function works fine even if there's a leftover # ref from previous failed run of the function - with patch("pygit2.remote.RemoteCollection.delete"): - pagure.lib.git.update_pull_ref(fake_pr, fork) + try: + with patch("pygit2.remote.RemoteCollection.delete"): + pagure.lib.git.update_pull_ref(fake_pr, fork) + except ImportError: + with patch("pygit2.remotes.RemoteCollection.delete"): + pagure.lib.git.update_pull_ref(fake_pr, fork) self.assertIsNotNone(fork.remotes["pingou_1234567"]) tests.add_content_git_repo(projects[1], append="foobarbaz") newesthex = fork.references["refs/heads/master"].peel().hex From d109f854e6f4d873e55cb51b9baff0b7a9d538e0 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 13/55] tests: bump CentOS Stream from 8 to 9 --- diff --git a/dev/containers/base-centos-stream8-rpms-py3 b/dev/containers/base-centos-stream8-rpms-py3 deleted file mode 100644 index e547f1b..0000000 --- a/dev/containers/base-centos-stream8-rpms-py3 +++ /dev/null @@ -1,41 +0,0 @@ -FROM quay.io/centos/centos:stream8 - -LABEL org.opencontainers.image.authors="pagure community" -LABEL org.opencontainers.image.url="https://pagure.io/pagure" -LABEL org.opencontainers.image.source="https://pagure.io/pagure/blob/master/f/dev/containers" -LABEL org.opencontainers.image.documentation="https://docs.pagure.org/pagure/index.html" - -ARG specfile=https://pagure.io/pagure/raw/master/f/files/pagure.spec - -ENV SPECFILE=$specfile - -RUN dnf -y install \ - epel-release epel-next-release 'dnf-command(config-manager)' \ - && dnf -y config-manager --enable epel-testing epel-next-testing powertools \ - && dnf -y install \ - python3-setuptools \ - python3-beautifulsoup4 \ - python3-coverage \ - python3-mock \ - python3-docutils \ - python3-flake8 \ - python3-pytest-xdist \ - python3-flask-oidc \ - python3-cchardet \ - python3-fedora-messaging \ - python3-pip \ - redis \ - which \ - git \ - && dnf clean all - -RUN pip3 install pagure-messages - -# Install all the requirements from the spec file and replace the macro -# %{python_pkgversion} by '3' which thus installs all the py3 version of -# the dependencies. -RUN curl ${SPECFILE} -o /pagure.spec \ - && dnf install -y `grep "Requires:" /pagure.spec | \ - awk '{split($0, a, " "); print a[2]}' | grep -v "%{name}" | \ - sed -e "s|%{python_pkgversion}|3|"` \ - && dnf clean all \ No newline at end of file diff --git a/dev/containers/base-centos-stream9-rpms-py3 b/dev/containers/base-centos-stream9-rpms-py3 new file mode 100644 index 0000000..a619f76 --- /dev/null +++ b/dev/containers/base-centos-stream9-rpms-py3 @@ -0,0 +1,46 @@ +FROM quay.io/centos/centos:stream9 + +LABEL org.opencontainers.image.authors="pagure community" +LABEL org.opencontainers.image.url="https://pagure.io/pagure" +LABEL org.opencontainers.image.source="https://pagure.io/pagure/blob/master/f/dev/containers" +LABEL org.opencontainers.image.documentation="https://docs.pagure.org/pagure/index.html" + +ARG specfile=https://pagure.io/pagure/raw/master/f/files/pagure.spec + +ENV SPECFILE=$specfile + +RUN dnf -y install 'dnf-command(config-manager)' \ + && dnf -y config-manager --set-enabled crb \ + && dnf -y install epel-release \ + && dnf -y install \ + python3-setuptools \ + python3-beautifulsoup4 \ + python3-coverage \ + python3-docutils \ + python3-flake8 \ + python3-pytest-xdist \ + python3-chardet \ + fedora-messaging \ + python3-pip \ + redis \ + which \ + git \ + gcc \ + && dnf clean all + +RUN pip3 install pagure-messages flask-oidc binaryornot celery straight-plugin trololio jenkins + +# Install all the requirements from the spec file and replace the macro +# %{python_pkgversion} by '3' which thus installs all the py3 version of +# the dependencies. +RUN curl ${SPECFILE} -o /pagure.spec \ + && sed -i 's/python%{python_pkgversion}-binaryornot//' /pagure.spec \ + && sed -i 's/python%{python_pkgversion}-celery//' /pagure.spec \ + && sed -i 's/python%{python_pkgversion}-flask-oidc//' /pagure.spec \ + && sed -i 's/python%{python_pkgversion}-straight-plugin.*//' /pagure.spec \ + && sed -i 's/python%{python_pkgversion}-trololio//' /pagure.spec \ + && sed -i 's/python%{python_pkgversion}-jenkins//' /pagure.spec \ + && dnf install -y `grep "Requires:" /pagure.spec | \ + awk '{split($0, a, " "); print a[2]}' | grep -v "%{name}" | \ + sed -e "s|%{python_pkgversion}|3|"` \ + && dnf clean all diff --git a/dev/containers/code-centos-stream8-rpms-py3 b/dev/containers/code-centos-stream8-rpms-py3 deleted file mode 100644 index 5639bb2..0000000 --- a/dev/containers/code-centos-stream8-rpms-py3 +++ /dev/null @@ -1,16 +0,0 @@ -FROM base-centos-stream8-rpms-py3:latest - -LABEL org.opencontainers.image.authors="pagure community" -LABEL org.opencontainers.image.url="https://pagure.io/pagure" -LABEL org.opencontainers.image.source="https://pagure.io/pagure/blob/master/f/dev/containers" -LABEL org.opencontainers.image.documentation="https://docs.pagure.org/pagure/index.html" - -ARG repo=https://pagure.io/pagure.git -ARG branch=master - -ENV REPO=$repo -ENV BRANCH=$branch - -COPY entrypoint_rpms.sh /entrypoint.sh -RUN chmod +x /entrypoint.sh -ENTRYPOINT /entrypoint.sh \ No newline at end of file diff --git a/dev/containers/code-centos-stream9-rpms-py3 b/dev/containers/code-centos-stream9-rpms-py3 new file mode 100644 index 0000000..5639bb2 --- /dev/null +++ b/dev/containers/code-centos-stream9-rpms-py3 @@ -0,0 +1,16 @@ +FROM base-centos-stream8-rpms-py3:latest + +LABEL org.opencontainers.image.authors="pagure community" +LABEL org.opencontainers.image.url="https://pagure.io/pagure" +LABEL org.opencontainers.image.source="https://pagure.io/pagure/blob/master/f/dev/containers" +LABEL org.opencontainers.image.documentation="https://docs.pagure.org/pagure/index.html" + +ARG repo=https://pagure.io/pagure.git +ARG branch=master + +ENV REPO=$repo +ENV BRANCH=$branch + +COPY entrypoint_rpms.sh /entrypoint.sh +RUN chmod +x /entrypoint.sh +ENTRYPOINT /entrypoint.sh \ No newline at end of file diff --git a/dev/run-tests-container.py b/dev/run-tests-container.py index 237392e..29ace6c 100755 --- a/dev/run-tests-container.py +++ b/dev/run-tests-container.py @@ -175,9 +175,9 @@ if __name__ == "__main__": containers = { "centos": { - "name": "pagure-tests-centos-stream8-rpms-py3", - "base": "base-centos-stream8-rpms-py3", - "code": "code-centos-stream8-rpms-py3", + "name": "pagure-tests-centos-stream9-rpms-py3", + "base": "base-centos-stream9-rpms-py3", + "code": "code-centos-stream9-rpms-py3", }, "fedora": { "name": "pagure-tests-fedora-rpms-py3", From d06940971d544afe4d40798ed7a71fb5699c0050 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 14/55] tests: fix test_view_issue_ticket_access - AssertionError --- diff --git a/tests/test_pagure_flask_ui_issues_acl_checks.py b/tests/test_pagure_flask_ui_issues_acl_checks.py index b8af162..0ea82d4 100644 --- a/tests/test_pagure_flask_ui_issues_acl_checks.py +++ b/tests/test_pagure_flask_ui_issues_acl_checks.py @@ -371,7 +371,7 @@ class PagureFlaskIssuesACLtests(tests.Modeltests): 'href="/test/issue/1/edit" title="Edit this issue">\n', output_text, ) - self.assertIn( + self.assertTrue( '' "Log in\n to comment on this ticket.", output_text, From 19d2b27bcb8e0bc1e8c78f75085aabe6db3720a7 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 15/55] tests: fix issues related to log in url in 'tests/test_pagure_flask_ui_issues_acl_checks.py' --- diff --git a/tests/test_pagure_flask_ui_issues_acl_checks.py b/tests/test_pagure_flask_ui_issues_acl_checks.py index 0ea82d4..c3e8c0f 100644 --- a/tests/test_pagure_flask_ui_issues_acl_checks.py +++ b/tests/test_pagure_flask_ui_issues_acl_checks.py @@ -109,7 +109,7 @@ class PagureFlaskIssuesACLtests(tests.Modeltests): output.get_data(as_text=True), ) self.assertTrue( - '' + '' "Log in\n to comment on this ticket." in output.get_data(as_text=True) ) @@ -371,8 +371,8 @@ class PagureFlaskIssuesACLtests(tests.Modeltests): 'href="/test/issue/1/edit" title="Edit this issue">\n', output_text, ) - self.assertTrue( - '' + self.assertIn( + '' "Log in\n to comment on this ticket.", output_text, ) @@ -627,7 +627,7 @@ class PagureFlaskIssuesACLtests(tests.Modeltests): output_text, ) self.assertTrue( - '' + '' "Log in\n to comment on this ticket.", output_text, ) @@ -880,7 +880,7 @@ class PagureFlaskIssuesACLtests(tests.Modeltests): output.get_data(as_text=True), ) self.assertTrue( - '' + '' "Log in\n to comment on this ticket." in output.get_data(as_text=True) ) From 1a5c449c8314d976798d80b97a44908c80be75e3 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 16/55] tests: fix issues related to log in url in 'tests/test_pagure_flask_ui_issues.py' --- diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index 6d763cf..ada27c5 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -1315,7 +1315,7 @@ class PagureFlaskIssuestests(tests.Modeltests): output_text, ) self.assertIn( - '' + '' "Log in\n to comment on this ticket.", output_text, ) @@ -1358,7 +1358,7 @@ class PagureFlaskIssuestests(tests.Modeltests): output_text, ) self.assertIn( - '' + '' "Log in\n to comment on this ticket.", output_text, ) @@ -1502,7 +1502,7 @@ class PagureFlaskIssuestests(tests.Modeltests): output_text, ) self.assertIn( - '' + '' "Log in\n to comment on this ticket.", output_text, ) @@ -1576,7 +1576,7 @@ class PagureFlaskIssuestests(tests.Modeltests): output_text, ) self.assertTrue( - '' + '' "Log in\n to comment on this ticket." in output_text ) From a2b3d9431e59bc1994f4aad669a70e7fa20dc32e Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 17/55] tests: fix issues related to log in url in 'tests/test_pagure_flask_ui_issues_open_access.py' --- diff --git a/tests/test_pagure_flask_ui_issues_open_access.py b/tests/test_pagure_flask_ui_issues_open_access.py index 6975564..e993799 100644 --- a/tests/test_pagure_flask_ui_issues_open_access.py +++ b/tests/test_pagure_flask_ui_issues_open_access.py @@ -213,7 +213,7 @@ class PagureFlaskIssuesOpenAccesstests(tests.Modeltests): output_text, ) self.assertIn( - '' + '' "Log in\n to comment on this ticket.", output_text, ) @@ -313,7 +313,7 @@ class PagureFlaskIssuesOpenAccesstests(tests.Modeltests): output_text, ) self.assertTrue( - '' + '' "Log in\n to comment on this ticket." in output_text ) From 2dfe6da4279cf807ae4189840432cdfa2723c8d4 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 18/55] tests: fix issues related to log in url in 'tests/test_pagure_flask_ui_login.py' --- diff --git a/tests/test_pagure_flask_ui_login.py b/tests/test_pagure_flask_ui_login.py index ea6ac99..02db2fc 100644 --- a/tests/test_pagure_flask_ui_login.py +++ b/tests/test_pagure_flask_ui_login.py @@ -1102,7 +1102,9 @@ class PagureFlaskLogintests(tests.SimplePagureTest): in ( "http://localhost/login/", "/login/?next=http%3A%2F%2Flocalhost%2Fsettings%2F", + "/login/?next=http://localhost/settings/", "http://localhost/login/?next=http%3A%2F%2Flocalhost%2Fsettings%2F", + "http://localhost/login/?next=http://localhost/settings/", ) ) # session did not expire From 3f6cd263a1a2f798cb3ea30aed226b89d865f718 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 19/55] tests: fix test_commit_to_patch_empty_commit --- diff --git a/tests/test_pagure_lib_git.py b/tests/test_pagure_lib_git.py index 8194962..595de6d 100644 --- a/tests/test_pagure_lib_git.py +++ b/tests/test_pagure_lib_git.py @@ -3729,6 +3729,7 @@ class PagureLibGitCommitToPatchtests(tests.Modeltests): ) self.third_commit = repo.revparse_single("HEAD") + print(self.third_commit) def test_commit_to_patch_first_commit(self): """Test the commit_to_patch function of pagure.lib.git.""" @@ -4079,6 +4080,7 @@ index 9f44358..2a552bb 100644 repo = pygit2.init_repository(self.gitrepo) patch = pagure.lib.git.commit_to_patch(repo, self.third_commit) + print(patch) exp = "" self.assertEqual(patch, exp) From c719bf090e95de6db043195df104f1e51c96c927 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 20/55] fix: Test for empty string in addition to None Type in 'pagure.lib.git.commit_to_patch' When 'test_commit_to_patch_empty_commit' runs, 'commit.tree.diff_to_tree(swap=True)' in 'pagure.lib.git.commit_to_patch' returns 'None' if pygit2 version is 1.8.0. With pygit2 version 1.14.x the return value is an empty string. --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 98d72d0..3e76ad0 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -88,7 +88,7 @@ def commit_to_patch( # First commit in the repo diff = commit.tree.diff_to_tree(swap=True) - if diff.patch is None: + if diff.patch is None or diff.patch == "": continue if find_similar and diff: From 7de1bc750da2096e2b01ebd1fd305b59afafc30e Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 21/55] tests: Add argument --toxenv to run-tests-container.py. Run tests with tox against all environments defined in tox.ini by default. Customizable by the new argument. Passes the value to the '-e' argument of 'tox'. --- diff --git a/dev/containers/tox_py3.sh b/dev/containers/tox_py3.sh index 68755b0..291a930 100644 --- a/dev/containers/tox_py3.sh +++ b/dev/containers/tox_py3.sh @@ -23,4 +23,10 @@ git --no-pager log -2 fi export LANG="en_US.UTF-8" -tox -v -e "${PYVER:-py39}" -- ${TESTCASE:-tests/} + +echo TOXENV: "${TOXENV}" +if [ -z "${TOXENV}" ]; then + tox -v -- ${TESTCASE:-tests/} +else + tox -v -e "${TOXENV}" -- ${TESTCASE:-tests/} +fi diff --git a/dev/run-tests-container.py b/dev/run-tests-container.py index 29ace6c..3b5ab22 100755 --- a/dev/run-tests-container.py +++ b/dev/run-tests-container.py @@ -112,6 +112,13 @@ def setup_parser(): "test_case", nargs="?", default="", help="Run the given test case" ) parser.add_argument( + "--toxenv", + default="", + help="Pass a custom value for the target python environment to tox tests. " + "Only used by the pip test container. " + "Default: Use all Environments defined in tox.ini", + ) + parser.add_argument( "--fedora", action="store_true", help="Run the tests in fedora environment (DEFAULT)", @@ -145,7 +152,6 @@ def setup_parser(): help="Gives you a shell into the container instead " "of running the tests", ) - parser.add_argument( "--repo", dest="repo", @@ -301,6 +307,8 @@ if __name__ == "__main__": "REPO={}".format(os.environ.get("REPO") or args.repo), "-e", "TESTCASE={}".format(args.test_case or ""), + "-e", + "TOXENV={}".format(os.environ.get("TOXENV") or args.toxenv), ] if args.shell: From fe64dc92d361c9d8e6d236f5438b900b7a9f77f1 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 22/55] tests: Add python 3.12 back in tox.ini --- diff --git a/tox.ini b/tox.ini index 0ad7e54..c662fd2 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py{39,310,311} +envlist = py{39,310,311,312} skipsdist = True # If the user is missing an interpreter, don't fail skip_missing_interpreters = True From f28a5d6279fc4420d7f256e370bf4800931f870e Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 23/55] build: Don't add tox_py3.sh to fedora pip code container, that file comes from the repo during testing. --- diff --git a/dev/containers/code-fedora-pip-py3 b/dev/containers/code-fedora-pip-py3 index 400db81..147aa01 100644 --- a/dev/containers/code-fedora-pip-py3 +++ b/dev/containers/code-fedora-pip-py3 @@ -11,7 +11,6 @@ ARG branch=master ENV REPO=$repo ENV BRANCH=$branch -COPY tox_py3.sh /tox_py3.sh COPY entrypoint_pip.sh /entrypoint.sh RUN chmod +x /entrypoint.sh ENTRYPOINT /entrypoint.sh \ No newline at end of file diff --git a/dev/containers/entrypoint_pip.sh b/dev/containers/entrypoint_pip.sh index b67e768..4078369 100644 --- a/dev/containers/entrypoint_pip.sh +++ b/dev/containers/entrypoint_pip.sh @@ -1,7 +1,6 @@ #!/bin/bash cd / \ && GIT_TRACE=1 git clone -b ${BRANCH} ${REPO} /pagure \ -&& cp /tox_py3.sh /pagure/dev/containers/tox_py3.sh \ && chmod +x /pagure/dev/containers/tox_py3.sh \ && ln -s /tox /pagure/.tox \ && cd /pagure \ diff --git a/dev/containers/tox_py3.sh b/dev/containers/tox_py3.sh index 291a930..8f94860 100644 --- a/dev/containers/tox_py3.sh +++ b/dev/containers/tox_py3.sh @@ -26,7 +26,7 @@ export LANG="en_US.UTF-8" echo TOXENV: "${TOXENV}" if [ -z "${TOXENV}" ]; then - tox -v -- ${TESTCASE:-tests/} -else tox -v -e "${TOXENV}" -- ${TESTCASE:-tests/} +else + tox -v -- ${TESTCASE:-tests/} fi From 14a36e6828932e366b23f4e134c37c01f1d0a160 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 24/55] fix: Replace 'imp' with 'importlib' for Python 3.12 compatibility. --- diff --git a/tests/__init__.py b/tests/__init__.py index 9f03615..8d2ee91 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -10,7 +10,7 @@ from __future__ import unicode_literals, absolute_import -import imp +import importlib import json import logging import os @@ -388,9 +388,9 @@ class SimplePagureTest(unittest.TestCase): os.environ["PAGURE_CONFIG"] = self.config_path pagure_config.update(reload_config()) - imp.reload(pagure.lib.tasks) - imp.reload(pagure.lib.tasks_mirror) - imp.reload(pagure.lib.tasks_services) + importlib.reload(pagure.lib.tasks) + importlib.reload(pagure.lib.tasks_mirror) + importlib.reload(pagure.lib.tasks_services) self._app = pagure.flask_app.create_app({"DB_URL": self.dbpath}) From 9f770fafd4f7117903c0ef4bb6ca8d3d2191a65d Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 25/55] fix: Mock 'imp' module to satisfy 'straight.plugin' till https://github.com/ironfroggy/straight.plugin/pull/30 is published. --- diff --git a/pagure/lib/plugins.py b/pagure/lib/plugins.py index 62ab94d..df2e513 100644 --- a/pagure/lib/plugins.py +++ b/pagure/lib/plugins.py @@ -10,10 +10,18 @@ from __future__ import absolute_import, unicode_literals -from straight.plugin import load - from pagure.lib.model_base import BASE +# latest straight.plugin release still use 'imp', +# which is dropped in python 3.12. mock 'imp' as workaround till +# https://github.com/ironfroggy/straight.plugin/pull/30 is published. +import sys +import mock + +sys.modules["imp"] = mock.Mock() + +from straight.plugin import load # noqa: E402 + def get_plugin_names(blacklist=None, without_backref=False): """Return the list of plugins names. diff --git a/requirements.txt b/requirements.txt index 9daa335..a8d0d87 100644 --- a/requirements.txt +++ b/requirements.txt @@ -34,7 +34,7 @@ six <= 1.16.0 setuptools sqlalchemy >= 0.8, <=1.4.46 -straight.plugin == 1.5.0 +straight.plugin whitenoise <= 6.2.0 wtforms <= 3.0.1 From 7bebc035a1df7c18cbb41bf9b4f4874dc44943ca Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 26/55] build: Remove version pinning from Celery to support Py311 and Py312 --- diff --git a/requirements.txt b/requirements.txt index a8d0d87..2922ee2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ bcrypt <= 3.2.2 binaryornot == 0.4.4 bleach <= 5.0.1 blinker <= 1.5 -celery <= 5.2.6 +celery chardet <= 4.0.0 cryptography <= 36.0.0 docutils <= 0.17.1 From b0793d2f55e6abf59d5f41a63b85e90347c37041 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 27/55] build: Pin sqlalchemy to any version below major release 2.0.0, latst 1.4x releases provide Py311 and Py312 support --- diff --git a/requirements.txt b/requirements.txt index 2922ee2..72ffe6e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -33,7 +33,7 @@ six <= 1.16.0 # required for backward compatibility setuptools -sqlalchemy >= 0.8, <=1.4.46 +sqlalchemy < 2.0.0 straight.plugin whitenoise <= 6.2.0 wtforms <= 3.0.1 From d06f744df3b9516c881d07b6942490369a5591be Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 28/55] build: Version pinning for redis-py package removed --- diff --git a/requirements.txt b/requirements.txt index 72ffe6e..cb7c44a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,7 +26,7 @@ pygit2 python3-openid <= 3.2.0 python-openid-cla == 1.2 python-openid-teams == 1.1 -redis <= 3.5.3 +redis requests <= 2.28.1 six <= 1.16.0 From 4a6c14d0563fc5c178071838425715375b22aa52 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 29/55] tests: Debug celery/redis task_id None issue --- diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index c6f1042..2eaa8d7 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -61,6 +61,13 @@ elif "REDIS_HOST" in pagure_config and "REDIS_PORT" in pagure_config: conn = Celery("tasks", broker=broker_url, backend=broker_url) conn.conf.update(pagure_config["CELERY_CONFIG"]) +from celery.backends.base import BaseKeyValueStoreBackend +def get_key_for_task(self, task_id, key=''): + key_t = self.key_t + return key_t('').join([ + self.task_keyprefix, key_t(task_id), key_t(key), + ]) +BaseKeyValueStoreBackend.get_key_for_task = get_key_for_task @after_setup_task_logger.connect def augment_celery_log(**kwargs): diff --git a/pagure/lib/tasks_utils.py b/pagure/lib/tasks_utils.py index f65d7eb..0a8ec77 100644 --- a/pagure/lib/tasks_utils.py +++ b/pagure/lib/tasks_utils.py @@ -28,6 +28,7 @@ def pagure_task(function): """Decorated function, actually does the work.""" if self is not None: try: + print(self.request.id) self.update_state(state="RUNNING") except TypeError: pass diff --git a/tests/test_pagure_admin.py b/tests/test_pagure_admin.py index b64ad67..5488bfd 100644 --- a/tests/test_pagure_admin.py +++ b/tests/test_pagure_admin.py @@ -1959,9 +1959,9 @@ class PagureAdminDeleteProjectTests(tests.Modeltests): "action_user": "pingou", } ) - with tests.capture_output() as output: - pagure.cli.admin.do_delete_project(args) - output = output.getvalue() + #with tests.capture_output() as output: + output = pagure.cli.admin.do_delete_project(args) + #output = output.getvalue() self.assertEqual( "Are you sure you want to delete: somenamespace/test?\n" " This cannot be undone!\n" From 315ec2def8cfc8e8509af76b160121af9e303623 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 30/55] tests: Monkey Patch as temporary workaround for celery task_id = None issues. Needs a better solution later --- diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index 2eaa8d7..1496bc0 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -61,6 +61,10 @@ elif "REDIS_HOST" in pagure_config and "REDIS_PORT" in pagure_config: conn = Celery("tasks", broker=broker_url, backend=broker_url) conn.conf.update(pagure_config["CELERY_CONFIG"]) + +### Monkey Patch - start ### +# Issues that celery task_id is None are related to https://github.com/celery/celery/commit/726b664840b6a1fcea9225b254a393e665363ad0 +# Monkey Path to introduce the previous logic till it's clear how the Pagure code has to be adjusted, from celery.backends.base import BaseKeyValueStoreBackend def get_key_for_task(self, task_id, key=''): key_t = self.key_t @@ -68,6 +72,8 @@ def get_key_for_task(self, task_id, key=''): self.task_keyprefix, key_t(task_id), key_t(key), ]) BaseKeyValueStoreBackend.get_key_for_task = get_key_for_task +### Monkey Patch - end ### + @after_setup_task_logger.connect def augment_celery_log(**kwargs): From 38598bc9bcb0ddbeae7d521f003f20019af1ce90 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 31/55] tests: Undo previous code change during debugging --- diff --git a/pagure/lib/tasks_utils.py b/pagure/lib/tasks_utils.py index 0a8ec77..f65d7eb 100644 --- a/pagure/lib/tasks_utils.py +++ b/pagure/lib/tasks_utils.py @@ -28,7 +28,6 @@ def pagure_task(function): """Decorated function, actually does the work.""" if self is not None: try: - print(self.request.id) self.update_state(state="RUNNING") except TypeError: pass diff --git a/tests/test_pagure_admin.py b/tests/test_pagure_admin.py index 5488bfd..b64ad67 100644 --- a/tests/test_pagure_admin.py +++ b/tests/test_pagure_admin.py @@ -1959,9 +1959,9 @@ class PagureAdminDeleteProjectTests(tests.Modeltests): "action_user": "pingou", } ) - #with tests.capture_output() as output: - output = pagure.cli.admin.do_delete_project(args) - #output = output.getvalue() + with tests.capture_output() as output: + pagure.cli.admin.do_delete_project(args) + output = output.getvalue() self.assertEqual( "Are you sure you want to delete: somenamespace/test?\n" " This cannot be undone!\n" From f36011603e74e4f2e24f0ceeb83e9dd980b10a20 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 32/55] tests: Fix failed test test_get_stats_commits_empty_git --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index d445998..45874c4 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -745,6 +745,7 @@ def get_stats_commits(): return response repopath = repo.repopath("main") + print(repopath) task = pagure.lib.tasks.commits_author_stats.delay(repopath) diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index 1496bc0..eb3a55b 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -881,15 +881,18 @@ def commits_author_stats(self, session, repopath): stats = collections.defaultdict(int) number_of_commits = 0 authors_email = set() - for commit in repo_obj.walk( - repo_obj.head.peel().oid.hex, pygit2.GIT_SORT_NONE - ): - # For each commit record how many times each combination of name and - # e-mail appears in the git history. - number_of_commits += 1 - email = commit.author.email - author = commit.author.name - stats[(author, email)] += 1 + try: + for commit in repo_obj.walk( + repo_obj.head.peel().oid.hex, pygit2.GIT_SORT_NONE + ): + # For each commit record how many times each combination of name and + # e-mail appears in the git history. + number_of_commits += 1 + email = commit.author.email + author = commit.author.name + stats[(author, email)] += 1 + except pygit2.errors.GitError as e: + return e for (name, email), val in list(stats.items()): if not email: @@ -942,15 +945,20 @@ def commits_history_stats(self, session, repopath): repo_obj = pygit2.Repository(repopath) dates = collections.defaultdict(int) - for commit in repo_obj.walk( - repo_obj.head.peel().oid.hex, pygit2.GIT_SORT_NONE - ): - delta = ( - datetime.datetime.utcnow() - arrow.get(commit.commit_time).naive - ) - if delta.days > 365: - break - dates[arrow.get(commit.commit_time).date().isoformat()] += 1 + + try: + for commit in repo_obj.walk( + repo_obj.head.peel().oid.hex, pygit2.GIT_SORT_NONE + ): + delta = ( + datetime.datetime.utcnow() + - arrow.get(commit.commit_time).naive + ) + if delta.days > 365: + break + dates[arrow.get(commit.commit_time).date().isoformat()] += 1 + except pygit2.errors.GitError as e: + return e return [(key, dates[key]) for key in sorted(dates)] From 5a62625c8d2f0e584caffc2aafb194039ce0b8e5 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 33/55] fix: Black formatting issues, findings of tests/test_style.py::TestStyle::test_code_with_black --- diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index eb3a55b..d295727 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -66,11 +66,19 @@ conn.conf.update(pagure_config["CELERY_CONFIG"]) # Issues that celery task_id is None are related to https://github.com/celery/celery/commit/726b664840b6a1fcea9225b254a393e665363ad0 # Monkey Path to introduce the previous logic till it's clear how the Pagure code has to be adjusted, from celery.backends.base import BaseKeyValueStoreBackend -def get_key_for_task(self, task_id, key=''): + + +def get_key_for_task(self, task_id, key=""): key_t = self.key_t - return key_t('').join([ - self.task_keyprefix, key_t(task_id), key_t(key), - ]) + return key_t("").join( + [ + self.task_keyprefix, + key_t(task_id), + key_t(key), + ] + ) + + BaseKeyValueStoreBackend.get_key_for_task = get_key_for_task ### Monkey Patch - end ### From 9aa1ae0aa58e5ae0c9d229fe82c85fb494b176b7 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 34/55] fix: Flake8 formatting issues, findings tests/test_style.py::TestStyle::test_code_with_flake8 --- diff --git a/pagure/lib/plugins.py b/pagure/lib/plugins.py index df2e513..1eb98bb 100644 --- a/pagure/lib/plugins.py +++ b/pagure/lib/plugins.py @@ -22,7 +22,6 @@ sys.modules["imp"] = mock.Mock() from straight.plugin import load # noqa: E402 - def get_plugin_names(blacklist=None, without_backref=False): """Return the list of plugins names. diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index d295727..bc230dc 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -62,10 +62,12 @@ conn = Celery("tasks", broker=broker_url, backend=broker_url) conn.conf.update(pagure_config["CELERY_CONFIG"]) -### Monkey Patch - start ### -# Issues that celery task_id is None are related to https://github.com/celery/celery/commit/726b664840b6a1fcea9225b254a393e665363ad0 -# Monkey Path to introduce the previous logic till it's clear how the Pagure code has to be adjusted, -from celery.backends.base import BaseKeyValueStoreBackend +# Monkey Patch - start +# Issues that celery task_id is None are related to +# https://github.com/celery/celery/commit/726b664840b6a1fcea9225b254a393e665363ad0 +# Monkey Path to introduce the previous logic +# till it's clear how the Pagure code has to be adjusted. +from celery.backends.base import BaseKeyValueStoreBackend # noqa: E402 def get_key_for_task(self, task_id, key=""): @@ -80,7 +82,7 @@ def get_key_for_task(self, task_id, key=""): BaseKeyValueStoreBackend.get_key_for_task = get_key_for_task -### Monkey Patch - end ### +# Monkey Patch - end @after_setup_task_logger.connect From 8c30108368e282dafde8d0d8f4aef4c52cdba36b Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 35/55] tests: 'assertEquals' deprecated and removed, replaced by 'assertEqual' --- diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 2227dc6..c83954f 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -3482,7 +3482,7 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): tests.create_tokens_acl(self.session, "aaabbbcccddd", "modify_project") project = pagure.lib.query._get_project(self.session, "test") - self.assertEquals( + self.assertEqual( project.access_users, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) @@ -3746,7 +3746,7 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): headers = {"Authorization": "token aaabbbcccddd"} project = pagure.lib.query._get_project(self.session, "test") - self.assertEquals( + self.assertEqual( project.access_users, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) @@ -3795,7 +3795,7 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): # Ensure `foo` was properly added: project = pagure.lib.query._get_project(self.session, "test") user_foo = pagure.lib.query.search_user(self.session, username="foo") - self.assertEquals( + self.assertEqual( project.access_users, { "admin": [], @@ -3877,7 +3877,7 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): # Ensure `foo` was properly removed self.session = pagure.lib.query.create_session(self.dbpath) project = pagure.lib.query._get_project(self.session, "test") - self.assertEquals( + self.assertEqual( project.access_users, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) @@ -3891,7 +3891,7 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): # Ensure `foo` was properly added: project = pagure.lib.query._get_project(self.session, "test") user_foo = pagure.lib.query.search_user(self.session, username="foo") - self.assertEquals( + self.assertEqual( project.access_users, { "admin": [], @@ -3958,7 +3958,7 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): # Ensure `foo` was properly removed self.session = pagure.lib.query.create_session(self.dbpath) project = pagure.lib.query._get_project(self.session, "test") - self.assertEquals( + self.assertEqual( project.access_users, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) @@ -4039,7 +4039,7 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): # Ensure `baz` was properly added self.session = pagure.lib.query.create_session(self.dbpath) project = pagure.lib.query._get_project(self.session, "test") - self.assertEquals( + self.assertEqual( project.access_users, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) @@ -4047,7 +4047,7 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): project.access_groups, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) - self.assertEquals(len(project.access_groups["ticket"]), 1) + self.assertEqual(len(project.access_groups["ticket"]), 1) # Remove the group from the project data = {"user_type": "group", "name": "baz", "acl": None} @@ -4106,11 +4106,11 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): # Ensure `baz` was properly removed self.session = pagure.lib.query.create_session(self.dbpath) project = pagure.lib.query._get_project(self.session, "test") - self.assertEquals( + self.assertEqual( project.access_users, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) - self.assertEquals( + self.assertEqual( project.access_groups, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) @@ -4191,11 +4191,11 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): # Ensure `baz` was properly removed self.session = pagure.lib.query.create_session(self.dbpath) project = pagure.lib.query._get_project(self.session, "test") - self.assertEquals( + self.assertEqual( project.access_users, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) - self.assertEquals( + self.assertEqual( project.access_groups, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) @@ -4214,7 +4214,7 @@ class PagureFlaskApiProjectOptionsTests(tests.Modeltests): tests.create_tokens_acl(self.session, "aaabbbcccddd", "modify_project") project = pagure.lib.query._get_project(self.session, "test") - self.assertEquals( + self.assertEqual( project.access_users, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 9603726..055830f 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -5450,8 +5450,8 @@ index 0000000..fb7093d tests.add_readme_git_repo(os.path.join(self.path, "repos", "test.git")) user = pagure.lib.query.search_user(self.session, username="pingou") - self.assertEquals(len(user.emails), 2) - self.assertEquals(user.default_email, "bar@pingou.com") + self.assertEqual(len(user.emails), 2) + self.assertEqual(user.default_email, "bar@pingou.com") user = tests.FakeUser(username="pingou") with tests.user_set(self.app.application, user): diff --git a/tests/test_pagure_flask_ui_roadmap.py b/tests/test_pagure_flask_ui_roadmap.py index f7eaa3b..9c8b4a4 100644 --- a/tests/test_pagure_flask_ui_roadmap.py +++ b/tests/test_pagure_flask_ui_roadmap.py @@ -662,7 +662,7 @@ class PagureFlaskRoadmaptests(tests.Modeltests): output_text, ) self.assertIn('', output_text) - self.assertEquals( + self.assertEqual( output_text.count(''), 1 ) diff --git a/tests/test_pagure_utils.py b/tests/test_pagure_utils.py index e39b694..fad9cdd 100644 --- a/tests/test_pagure_utils.py +++ b/tests/test_pagure_utils.py @@ -60,19 +60,19 @@ class PagureUtilsTests(tests.SimplePagureTest): """Test lookup_deploykey with a non-deploykey username.""" project = pagure.lib.query._get_project(self.session, "test") res = pagure.utils.lookup_deploykey(project, "pingou") - self.assertEquals(res, None) + self.assertEqual(res, None) def test_lookup_deploykey_different_project(self): """Test lookup_deploykey with a username for another project.""" project = pagure.lib.query._get_project(self.session, "test2") res = pagure.utils.lookup_deploykey(project, "deploykey_test_1") - self.assertEquals(res, None) + self.assertEqual(res, None) def test_lookup_deploykey_non_existent_key(self): """Test lookup_deploykey with a non-existing deploykey.""" project = pagure.lib.query._get_project(self.session, "test") res = pagure.utils.lookup_deploykey(project, "deploykey_test_2") - self.assertEquals(res, None) + self.assertEqual(res, None) def test_lookup_deploykey(self): """Test lookup_deploykey with a correct username.""" From d7803cdd4f1dda389867e50a17fbda23f12e2273 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 36/55] tests: 'assertNotEquals' deprecated and removed, replaced by 'assertNotEqual' --- diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index c83954f..9cef058 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -4043,7 +4043,7 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): project.access_users, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) - self.assertNotEquals( + self.assertNotEqual( project.access_groups, {"admin": [], "collaborator": [], "commit": [], "ticket": []}, ) diff --git a/tests/test_pagure_utils.py b/tests/test_pagure_utils.py index fad9cdd..71595be 100644 --- a/tests/test_pagure_utils.py +++ b/tests/test_pagure_utils.py @@ -78,5 +78,5 @@ class PagureUtilsTests(tests.SimplePagureTest): """Test lookup_deploykey with a correct username.""" project = pagure.lib.query._get_project(self.session, "test") res = pagure.utils.lookup_deploykey(project, "deploykey_test_1") - self.assertNotEquals(res, None) + self.assertNotEqual(res, None) self.assertFalse(res.pushaccess) From 8aa006b9df9e1cd8144dd442821ad65f45b2e508 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 37/55] build: Remove version pinning of flake8 python package --- diff --git a/requirements-testing.txt b/requirements-testing.txt index 547e862..7f865b2 100644 --- a/requirements-testing.txt +++ b/requirements-testing.txt @@ -4,7 +4,7 @@ black <= 22.8.0 cryptography <= 36.0.0 eventlet <= 0.33.2 fedmsg <= 1.1.2 -flake8 <= 4.0.1 +flake8 # Important: Until https://github.com/puiterwijk/flask-oidc/pull/144 is merged and a new version of flask-oidc is released, # it's necessary to ensure `itsdangerous` is pinned to a version lower as 2.1 as well. From 24028804c79afa547dfd11833ef485374538c580 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 38/55] tests: Run by default on all logical CPUs in rpm tests containers --- diff --git a/dev/containers/base-fedora-rpms-py3 b/dev/containers/base-fedora-rpms-py3 index a495da0..6533155 100644 --- a/dev/containers/base-fedora-rpms-py3 +++ b/dev/containers/base-fedora-rpms-py3 @@ -25,6 +25,7 @@ RUN dnf -y --enablerepo=updates-testing install \ redis \ which \ git \ + python3-pytest-timeout \ && dnf clean all RUN pip install pagure-messages diff --git a/dev/containers/runtests_py3.sh b/dev/containers/runtests_py3.sh index 84598e7..651b71c 100644 --- a/dev/containers/runtests_py3.sh +++ b/dev/containers/runtests_py3.sh @@ -24,4 +24,4 @@ fi sed -i -e "s|#!/usr/bin/env python|#!/usr/bin/env python3|" pagure/hooks/files/hookrunner -pytest-3 -n auto ${TESTCASE:-tests/} +pytest-3 -n auto -vvv ${TESTCASE:-tests/} From 771ae3100ec3789905a08bf66fc2d57a38fdc46c Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 39/55] build: Remove version pinning from pytest* packages --- diff --git a/requirements-testing.txt b/requirements-testing.txt index 7f865b2..9de8cef 100644 --- a/requirements-testing.txt +++ b/requirements-testing.txt @@ -13,9 +13,9 @@ itsdangerous < 2.1 mock <= 4.0.3 pagure-messages >= 0.0.1 -pytest <= 6.2.5 -pytest-cov <= 4.0.0 -pytest-xdist <= 2.5.0 +pytest +pytest-cov +pytest-xdist python-fedora == 1.1.1 trololio == 1.0 From 28995de39276c26b092c91628d60b72885374e5d Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 40/55] tests: Revert 80c9020eae7260851266b539d0aff1fee8222458 --- diff --git a/dev/containers/runtests_py3.sh b/dev/containers/runtests_py3.sh index 651b71c..84598e7 100644 --- a/dev/containers/runtests_py3.sh +++ b/dev/containers/runtests_py3.sh @@ -24,4 +24,4 @@ fi sed -i -e "s|#!/usr/bin/env python|#!/usr/bin/env python3|" pagure/hooks/files/hookrunner -pytest-3 -n auto -vvv ${TESTCASE:-tests/} +pytest-3 -n auto ${TESTCASE:-tests/} From ee9713375e857f054664dc82e14fa86bbd2fe1e6 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 41/55] fix: Address unit test issues and deprecation warnings Original patch from Sep 24 2023 04:43:04 +0000 slightly adjusted by Committer. --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 3e76ad0..1999516 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -88,7 +88,7 @@ def commit_to_patch( # First commit in the repo diff = commit.tree.diff_to_tree(swap=True) - if diff.patch is None or diff.patch == "": + if not diff.patch: continue if find_similar and diff: diff --git a/pagure/lib/query.py b/pagure/lib/query.py index c7e33c3..a206daf 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -4409,11 +4409,17 @@ def text2markdown(text, extended=True, readme=False): # Install our markdown modifications extensions.append("pagure.pfmarkdown") + extensions_configs = { + "markdown.extensions.codehilite": {"guess_lang": False}, + } + if md_version >= (3, 4): + extensions_configs["markdown.extensions.tables"] = { + "use_align_attribute": True + } + md_processor = markdown.Markdown( extensions=extensions, - extension_configs={ - "markdown.extensions.codehilite": {"guess_lang": False} - }, + extension_configs=extensions_configs, output_format="xhtml5", ) diff --git a/pagure/pfmarkdown.py b/pagure/pfmarkdown.py index 4f2a3e2..5cf8baa 100644 --- a/pagure/pfmarkdown.py +++ b/pagure/pfmarkdown.py @@ -31,6 +31,7 @@ import markdown.preprocessors import markdown.util import pygit2 import six +import xml.etree.ElementTree as etree import pagure.lib.query from pagure.config import config as pagure_config @@ -100,7 +101,7 @@ class MentionPattern(markdown.inlinepatterns.Pattern): if not user: return text - element = markdown.util.etree.Element("a") + element = etree.Element("a") base_url = pagure_config["APP_URL"] if base_url.endswith("/"): base_url = base_url[:-1] @@ -338,7 +339,7 @@ class StrikeThroughPattern(markdown.inlinepatterns.Pattern): text = markdown.util.AtomicString(m.group(2)) - element = markdown.util.etree.Element("del") + element = etree.Element("del") element.text = text return element @@ -359,7 +360,7 @@ class AutolinkPattern2(markdown.inlinepatterns.Pattern): url = url[1:] if url.endswith(">"): url = url[:-1] - el = markdown.util.etree.Element("a") + el = etree.Element("a") el.set("href", self.unescape(url)) el.text = markdown.util.AtomicString(url) return el @@ -376,11 +377,11 @@ class ImagePatternLazyLoad(ImagePattern): el = out # Add a noscript tag with the untouched img tag - noscript = markdown.util.etree.Element("noscript") + noscript = etree.Element("noscript") noscript.append(el) # Modify the origina img tag - img = markdown.util.etree.Element("img") + img = etree.Element("img") img.set("data-src", el.get("src")) img.set("src", "") img.set("alt", el.get("alt")) @@ -388,7 +389,7 @@ class ImagePatternLazyLoad(ImagePattern): # Create a global span in which we add both the new img tag and the # noscript one - outel = markdown.util.etree.Element("span") + outel = etree.Element("span") outel.append(img) outel.append(noscript) @@ -607,7 +608,7 @@ def _obj_anchor_tag(user, namespace, repo, obj, text): else: title = obj.title - element = markdown.util.etree.Element("a") + element = etree.Element("a") element.set("href", url) element.set("title", title) element.text = text diff --git a/requirements.txt b/requirements.txt index cb7c44a..ac2eee4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -18,7 +18,7 @@ flask <= 2.2.2 werkzeug < 3.0.0 flask-wtf <= 1.0.0 kitchen == 1.2.6 -markdown <= 3.3.7 +markdown munch <= 2.5.0 Pillow <= 9.2.0 psutil <= 5.9.2 diff --git a/tests/test_pagure_lib_encoding_utils.py b/tests/test_pagure_lib_encoding_utils.py index dad98c9..7b06620 100644 --- a/tests/test_pagure_lib_encoding_utils.py +++ b/tests/test_pagure_lib_encoding_utils.py @@ -71,7 +71,7 @@ class TestGuessEncodings(unittest.TestCase): # The last one in the list (which apparently has only one) self.assertEqual(result[-1].encoding, "WINDOWS-1250") else: - if chardet.__version__[0] in ("3", "4"): + if chardet.__version__[0] in ("3", "4", "5"): # The first three have different confidence values expexted_list = ["utf-8", "ISO-8859-9", "ISO-8859-1"] # This is the one with the least confidence From 59edf699016d1049fdaecd1ff728ad8eeb355b64 Mon Sep 17 00:00:00 2001 From: Klaus Koder Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 42/55] Drop the dependency on python-trololio (in favor of asyncio) On Fedora 38, using python 3.11 'trololio' is broken and no updates have been made to it for almost 10 years. Time to use 'raw' asyncio. Details trololio feature two issues. 1) Trololio depend on deprecated `imp` module: ``` >>> from imp import find_module, load_module :1: DeprecationWarning: the imp module is deprecated in favour of importlib and slated for removal in Python 3.12; see the module's documentation for alternative uses ``` 2) Trololio's embedded asyncio module fails. So trololio methods are never defined. For example: ``` $ cat trolo-smoke.py from trololio import asyncio, coroutine @coroutine def coro(): yield From(asyncio.sleep(1)) $ python trolo-smoke.py AttributeError: module 'trololio.asyncio' has no attribute 'coroutine'. Did you mean: 'coroutines'? ``` The same issue can be seen in the `pagure-ev` logs ``` pagure_stream_server.py[192893]: File "/usr/libexec/pagure-ev/pagure_stream_server.py", line 140, in pagure_stream_server.py[192893]: @asyncio.coroutine pagure_stream_server.py[192893]: ^^^^^^^^^^^^^^^^^ pagure_stream_server.py[192893]: AttributeError: module 'asyncio' has no attribute 'coroutine'. Did you mean: 'coroutines'? ``` Signed-off-by: Dominik Wombacher --- diff --git a/doc/install_pagure_ci.rst b/doc/install_pagure_ci.rst index 3b9ae78..68f7538 100644 --- a/doc/install_pagure_ci.rst +++ b/doc/install_pagure_ci.rst @@ -22,7 +22,6 @@ Configure your system python-jenkins python-redis - python-trololio .. note:: We ship a systemd unit file for pagure_ci but we welcome patches for scripts for other init systems. diff --git a/doc/install_pagure_ev.rst b/doc/install_pagure_ev.rst index 614f312..db6f78f 100644 --- a/doc/install_pagure_ev.rst +++ b/doc/install_pagure_ev.rst @@ -16,7 +16,6 @@ The eventsource server is easy to set-up. :: python-redis - python-trololio .. note:: We ship a systemd unit file for pagure_milter but we welcome patches for scripts for other init systems. diff --git a/doc/install_pagure_loadjson.rst b/doc/install_pagure_loadjson.rst index 4b13f06..407b672 100644 --- a/doc/install_pagure_loadjson.rst +++ b/doc/install_pagure_loadjson.rst @@ -14,7 +14,6 @@ Configure your system :: python-redis - python-trololio .. note:: We ship a systemd unit file for pagure_loadjson but we welcome patches for scripts for other init systems. diff --git a/doc/install_pagure_logcom.rst b/doc/install_pagure_logcom.rst index f92e0b3..4512dc4 100644 --- a/doc/install_pagure_logcom.rst +++ b/doc/install_pagure_logcom.rst @@ -14,7 +14,6 @@ Configure your system :: python-redis - python-trololio .. note:: We ship a systemd unit file for pagure_logcom but we welcome patches for scripts for other init systems. diff --git a/doc/install_pagure_webhooks.rst b/doc/install_pagure_webhooks.rst index 9aff374..9800817 100644 --- a/doc/install_pagure_webhooks.rst +++ b/doc/install_pagure_webhooks.rst @@ -17,7 +17,6 @@ Configure your system :: python-redis - python-trololio .. note:: We ship a systemd unit file for pagure_webhook but we welcome patches for scripts for other init systems. diff --git a/files/pagure.spec b/files/pagure.spec index f0eca32..5ae9444 100644 --- a/files/pagure.spec +++ b/files/pagure.spec @@ -144,7 +144,6 @@ This is useful for example to allow commenting on a ticket by email. Summary: EventSource server for pagure BuildArch: noarch Requires: %{name} = %{version}-%{release} -Requires: python%{python_pkgversion}-trololio %{?systemd_requires} %description ev Pagure comes with an eventsource server allowing live update of the pages diff --git a/pagure-ev/pagure_stream_server.py b/pagure-ev/pagure_stream_server.py index 6908ea7..907fd45 100644 --- a/pagure-ev/pagure_stream_server.py +++ b/pagure-ev/pagure_stream_server.py @@ -26,7 +26,7 @@ import os import redis -import trololio +import asyncio from six.moves.urllib.parse import urlparse @@ -137,14 +137,12 @@ def get_obj_from_path(path): return getfunc(repo, objid) -@trololio.coroutine -def handle_client(client_reader, client_writer): +async def handle_client(client_reader, client_writer): data = None while True: # give client a chance to respond, timeout after 10 seconds - line = yield trololio.From( - trololio.asyncio.wait_for(client_reader.readline(), timeout=10.0) - ) + line = await asyncio.wait_for(client_reader.readline(), timeout=10.0) + if not line.decode().strip(): break line = line.decode().rstrip() @@ -204,16 +202,16 @@ def handle_client(client_reader, client_writer): client_writer.write(("event: ping\n\n").encode()) oncall = 0 oncall += 1 - yield trololio.From(client_writer.drain()) - yield trololio.From(trololio.asyncio.sleep(1)) + await client_writer.drain() + await asyncio.sleep(1) else: log.info("Sending %s", msg["data"]) client_writer.write(("data: %s\n\n" % msg["data"]).encode()) - yield trololio.From(client_writer.drain()) + await client_writer.drain() except OSError: log.info("Client closed connection") - except trololio.ConnectionResetError as err: + except ConnectionResetError as err: log.exception("ERROR: ConnectionResetError in handle_client") except Exception as err: log.exception("ERROR: Exception in handle_client") @@ -225,8 +223,7 @@ def handle_client(client_reader, client_writer): client_writer.close() -@trololio.coroutine -def stats(client_reader, client_writer): +async def stats(client_reader, client_writer): try: log.info("Clients: %s", SERVER.active_count) @@ -234,9 +231,9 @@ def stats(client_reader, client_writer): ("HTTP/1.0 200 OK\n" "Cache: nocache\n\n").encode() ) client_writer.write(("data: %s\n\n" % SERVER.active_count).encode()) - yield trololio.From(client_writer.drain()) + await client_writer.drain() - except trololio.ConnectionResetError as err: + except ConnectionResetError as err: log.info(err) finally: client_writer.close() @@ -248,8 +245,8 @@ def main(): _get_session() try: - loop = trololio.asyncio.get_event_loop() - coro = trololio.asyncio.start_server( + loop = asyncio.get_event_loop() + coro = asyncio.start_server( handle_client, host=None, port=pagure.config.config["EVENTSOURCE_PORT"], @@ -259,7 +256,7 @@ def main(): "Serving server at {}".format(SERVER.sockets[0].getsockname()) ) if pagure.config.config.get("EV_STATS_PORT"): - stats_coro = trololio.asyncio.start_server( + stats_coro = asyncio.start_server( stats, host=None, port=pagure.config.config.get("EV_STATS_PORT"), @@ -273,7 +270,7 @@ def main(): loop.run_forever() except KeyboardInterrupt: pass - except trololio.ConnectionResetError as err: + except ConnectionResetError as err: log.exception("ERROR: ConnectionResetError in main") except Exception: log.exception("ERROR: Exception in main") diff --git a/requirements-ev.txt b/requirements-ev.txt index e704325..e69de29 100644 --- a/requirements-ev.txt +++ b/requirements-ev.txt @@ -1 +0,0 @@ -trololio == 1.0 diff --git a/requirements-testing.txt b/requirements-testing.txt index 9de8cef..7f3f50f 100644 --- a/requirements-testing.txt +++ b/requirements-testing.txt @@ -18,7 +18,6 @@ pytest-cov pytest-xdist python-fedora == 1.1.1 -trololio == 1.0 # Seems that mock doesn't list this one funcsigs <= 1.0.2 From 9b74b3cfa2099e98500d9d8434ac2f2cf29023d3 Mon Sep 17 00:00:00 2001 From: Klaus Koder Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 43/55] Fix PagureHooksPagureHooktests failure when system wide config exist When a pagure configuration file exist, for example, under `/etc/pagure/pagure.cfg` and the `APP_URL` key in that file is different from `http://localhost.localdomain/` then this test-case will fail (intermediately?). The root cause seem to be that `pagure_hook.py` keep a local version of the configuration from the time when the file was loaded, long time before the test-case create its configuration file. ``` pagure_config = pagure.config.reload_config() ``` The test case generate the configuration in `tests.setUp()`. A the timing looks like this: ``` $ pytest tests/ -k test_generate_revision_change_log_full_url_fork Using configuration file `/etc/pagure/pagure.cfg` tests/test_pagure_hooks_pagure_hook.py Detailed log of new commits: [...] ``` That looks a bit inefficient, notice how the local version of `pagure_config` inside `pagure_hook` is loaded from `/etc/pagure/pagure.cfg` even before the test case have been chosen. I can think of 3 ways to fix the issue. 1) Replace `"http://localhost.localdomain/"` with `mock.ANY`. 2) Pull the value of `pagure_config["APP_URL"]` from `pagure_hook` and use that in the test. 3) Fix the value of `pagure_config["APP_URL"]` using `mock.patch`. Here version 3. Signed-off-by: Dominik Wombacher --- diff --git a/tests/test_pagure_hooks_pagure_hook.py b/tests/test_pagure_hooks_pagure_hook.py index 5123efa..162ecda 100644 --- a/tests/test_pagure_hooks_pagure_hook.py +++ b/tests/test_pagure_hooks_pagure_hook.py @@ -93,6 +93,10 @@ class PagureHooksPagureHooktests(tests.SimplePagureTest): # Add a README to the git repo - First commit tests.add_readme_git_repo(self.folder) + @mock.patch.dict( + "pagure.hooks.pagure_hook.pagure_config", + {"APP_URL": "http://localhost.localdomain/"}, + ) @mock.patch("pagure.hooks.pagure_hook.fixes_relation") def test_generate_revision_change_log_short_url(self, fixes_relation): """Test generate_revision_change_log when the comment contains @@ -125,6 +129,10 @@ class PagureHooksPagureHooktests(tests.SimplePagureTest): "http://localhost.localdomain/", ) + @mock.patch.dict( + "pagure.hooks.pagure_hook.pagure_config", + {"APP_URL": "http://localhost.localdomain/"}, + ) @mock.patch("pagure.hooks.pagure_hook.fixes_relation") def test_generate_revision_change_log_full_url(self, fixes_relation): """Test generate_revision_change_log when the comment contains @@ -161,6 +169,10 @@ class PagureHooksPagureHooktests(tests.SimplePagureTest): "http://localhost.localdomain/", ) + @mock.patch.dict( + "pagure.hooks.pagure_hook.pagure_config", + {"APP_URL": "http://localhost.localdomain/"}, + ) @mock.patch("pagure.hooks.pagure_hook.fixes_relation") def test_generate_revision_change_log_full_url_fork(self, fixes_relation): """Test generate_revision_change_log when the comment contains From 2ad7ddd96d5f813ade888a17979d967bb230e390 Mon Sep 17 00:00:00 2001 From: Klaus Koder Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 44/55] Fix warning jinga2.escape DeprecationWarning For example ./pagure/ui/filters.py:580: DeprecationWarning: 'jinja2.escape' is deprecated and will be removed in Jinja 3.1. Import 'markupsafe.escape' instead. output = escape(author.name) Signed-off-by: Dominik Wombacher --- diff --git a/pagure/doc_utils.py b/pagure/doc_utils.py index 9f3b5b2..3fb9d81 100644 --- a/pagure/doc_utils.py +++ b/pagure/doc_utils.py @@ -19,9 +19,9 @@ import docutils.core import docutils.examples try: - from jinja2 import escape -except ImportError: from markupsafe import escape +except ImportError: + from jinja2 import escape import kitchen.text.converters as ktc from markupsafe import Markup diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index 40cc276..0618bd0 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -27,9 +27,9 @@ import pygit2 import six try: - from jinja2 import escape -except ImportError: from markupsafe import escape +except ImportError: + from jinja2 import escape from six.moves.urllib.parse import parse_qsl, urlparse import pagure.exceptions From 29baf9c9ea48ce017c2b3b0b9bde16a89d82fdfd Mon Sep 17 00:00:00 2001 From: Klaus Koder Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 45/55] Fix Celery related exception in get_key_for_task @wombelix : celery introduce strict error handling of an empty key with https://github.com/celery/celery/commit/726b664840b6a1fcea9225b254a393e665363ad0 and is part of celery releases since 5.3.0. This causes a large amount of errors like this one in the unit tests. Tracked down to the decorator function `@pagure_task` in `pagure/lib/task_utils.py`, this can be solved by passing `self.task_id` to `update_state`. At this point, there is no task running and therefore no id assigned to `self.request.id` yet. By passing `self`, the request id will be added right after the task was started as part of the `update_state` call. All `task_id` related errors are gone in the unit tests after this change. Signed-off-by: Dominik Wombacher --- diff --git a/pagure/lib/tasks_utils.py b/pagure/lib/tasks_utils.py index f65d7eb..9214b1e 100644 --- a/pagure/lib/tasks_utils.py +++ b/pagure/lib/tasks_utils.py @@ -28,7 +28,7 @@ def pagure_task(function): """Decorated function, actually does the work.""" if self is not None: try: - self.update_state(state="RUNNING") + self.update_state(self=self, state="RUNNING") except TypeError: pass session = pagure.lib.model_base.create_session(pagure_config["DB_URL"]) From 430e28caacf8ab80a0b82ade0f8d015d322322b4 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 46/55] chore: Remove Celery related MonkeyPatch after applying the Patch from Klaus Koder --- diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index bc230dc..f4c50bd 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -62,29 +62,6 @@ conn = Celery("tasks", broker=broker_url, backend=broker_url) conn.conf.update(pagure_config["CELERY_CONFIG"]) -# Monkey Patch - start -# Issues that celery task_id is None are related to -# https://github.com/celery/celery/commit/726b664840b6a1fcea9225b254a393e665363ad0 -# Monkey Path to introduce the previous logic -# till it's clear how the Pagure code has to be adjusted. -from celery.backends.base import BaseKeyValueStoreBackend # noqa: E402 - - -def get_key_for_task(self, task_id, key=""): - key_t = self.key_t - return key_t("").join( - [ - self.task_keyprefix, - key_t(task_id), - key_t(key), - ] - ) - - -BaseKeyValueStoreBackend.get_key_for_task = get_key_for_task -# Monkey Patch - end - - @after_setup_task_logger.connect def augment_celery_log(**kwargs): pagure.utils.set_up_logging(force=True) From 46c9a9888639fe6fb35cc6d072a35f554041a790 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 47/55] tests: CI pipeline - Drop tests on Fedora RPM temporary because of pytest xdist issues, tasks stuck till OOM Killer hits. Bump pip based tests to F39 and run against all available tox environments (py39, py310, py311, py312) --- diff --git a/dev/containers/fedora-pip-py3 b/dev/containers/fedora-pip-py3 index 9bc82a9..621af70 100644 --- a/dev/containers/fedora-pip-py3 +++ b/dev/containers/fedora-pip-py3 @@ -1,4 +1,4 @@ -FROM quay.io/fedora/fedora:36-x86_64 +FROM quay.io/fedora/fedora:39-x86_64 ARG repo=https://pagure.io/pagure.git ARG branch=master diff --git a/run_ci_tests_containers.sh b/run_ci_tests_containers.sh index bf5468e..34226bc 100644 --- a/run_ci_tests_containers.sh +++ b/run_ci_tests_containers.sh @@ -21,6 +21,7 @@ echo "Last commits:" git --no-pager log -2 fi + # F39 RPM SKIPPED - Bug with pytest+xdist, tests stuck and trigger OOM Killer - see https://pagure.io/pagure/pull-request/5463 #podman build --rm -t pagure-fedora-rpms-py3 \ # -f dev/containers/fedora-rpms-py3 \ From 66d0a1fdf4945bc7a036bc4cb9134eef2cb42b42 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 48/55] fix: black/flake8 formatting issue --- diff --git a/pagure/lib/plugins.py b/pagure/lib/plugins.py index 1eb98bb..df2e513 100644 --- a/pagure/lib/plugins.py +++ b/pagure/lib/plugins.py @@ -22,6 +22,7 @@ sys.modules["imp"] = mock.Mock() from straight.plugin import load # noqa: E402 + def get_plugin_names(blacklist=None, without_backref=False): """Return the list of plugins names. From cc54bd6f0c844ae33a282ad931f785ed4cc5ad29 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 49/55] tests: Revert 'CI pipeline - Drop tests on Fedora RPM temporary', bump rpm container Dockerfile to F39 --- diff --git a/dev/containers/fedora-rpms-py3 b/dev/containers/fedora-rpms-py3 index a1abe54..88cafa7 100644 --- a/dev/containers/fedora-rpms-py3 +++ b/dev/containers/fedora-rpms-py3 @@ -1,4 +1,4 @@ -FROM quay.io/fedora/fedora:36-x86_64 +FROM quay.io/fedora/fedora:39-x86_64 ARG repo=https://pagure.io/pagure.git ARG branch=master From 51a3fb7691880fbafb34650dc832b687cb29ca16 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 18 2024 23:23:59 +0000 Subject: [PATCH 50/55] build: python3-cchardet replaced by python3-chardet in F39 --- diff --git a/dev/containers/fedora-rpms-py3 b/dev/containers/fedora-rpms-py3 index 88cafa7..a7e8882 100644 --- a/dev/containers/fedora-rpms-py3 +++ b/dev/containers/fedora-rpms-py3 @@ -17,7 +17,7 @@ RUN dnf -y --enablerepo=updates-testing install \ python3-isort \ python3-pytest-xdist \ python3-flask-oidc \ - python3-cchardet \ + python3-chardet \ python3-fedora-messaging \ python3-pip \ redis \ From 2e1382b79b60e63d08d200cee3cda61949fd1634 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 19 2024 11:32:29 +0000 Subject: [PATCH 51/55] tests: Disable F39 RPM test because of pytest+xdist bug F39 pip isn't affected, but on rpm the tests stuck till the OOM Killer triggers. There are multiple similar reports upstream but no fix. To unblock the unit tests, F39 RPM will be skipped for now. F39 pip covers py39, py310, py311 and py312. CI jobs that run into the issue: https://jenkins-pagure.apps.ocp.cloud.ci.centos.org/job/pull-requests/276/ https://jenkins-pagure.apps.ocp.cloud.ci.centos.org/job/pull-requests/277/ GitHub issues that report similar issues: https://github.com/pytest-dev/pytest-xdist/issues/110 https://github.com/pytest-dev/pytest-xdist/issues/661 https://github.com/pytest-dev/pytest-xdist/issues/872 https://github.com/pytest-dev/pytest-xdist/issues/1005 --- diff --git a/dev/containers/fedora-rpms-py3 b/dev/containers/fedora-rpms-py3 index a7e8882..4a33252 100644 --- a/dev/containers/fedora-rpms-py3 +++ b/dev/containers/fedora-rpms-py3 @@ -41,5 +41,5 @@ RUN dnf install -y --enablerepo=updates-testing `grep "Requires:" /pagure/files/ python3 setup.py build WORKDIR /pagure -ENTRYPOINT ["/pagure/dev/containers/runtests_py3.sh"] +ENTRYPOINT ["/bin/sh", "-c", "echo F39 RPM SKIPPED - Bug with pytest+xdist, tests stuck and trigger OOM Killer - see https://pagure.io/pagure/pull-request/5463"] CMD [] From cfdc13474497af42a38c8d04d76199348886bef0 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 19 2024 11:32:29 +0000 Subject: [PATCH 52/55] tests: Set executable flag for 'runtests_py3.sh' and 'tox_py3.sh' to align with later usage in unit test containers. --- diff --git a/dev/containers/runtests_py3.sh b/dev/containers/runtests_py3.sh old mode 100644 new mode 100755 diff --git a/dev/containers/tox_py3.sh b/dev/containers/tox_py3.sh old mode 100644 new mode 100755 From 056b769972232d817df6b0678c24b9350eea2b16 Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 19 2024 11:32:29 +0000 Subject: [PATCH 53/55] tests: Install missing dependencies in pip container to fix 'Failed building wheel for Pillow' --- diff --git a/dev/containers/fedora-pip-py3 b/dev/containers/fedora-pip-py3 index 621af70..afd896d 100644 --- a/dev/containers/fedora-pip-py3 +++ b/dev/containers/fedora-pip-py3 @@ -18,7 +18,10 @@ RUN dnf -y install \ redis \ which \ git \ - glibc-langpack-en + glibc-langpack-en \ + libtiff-devel libjpeg-devel openjpeg2-devel zlib-devel \ + freetype-devel lcms2-devel libwebp-devel tcl-devel tk-devel \ + harfbuzz-devel fribidi-devel libraqm-devel libimagequant-devel libxcb-devel RUN cd / \ && GIT_TRACE=1 git clone -b $BRANCH $REPO \ From 47aee434036fa0ca92f128e5df28cc6036137ace Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 19 2024 11:33:48 +0000 Subject: [PATCH 54/55] tests: Make container entrypoint and run test scripts more reliable, fail on error, and always merge feature branch against upstream/master and not fork/master. --- diff --git a/dev/containers/entrypoint_pip.sh b/dev/containers/entrypoint_pip.sh index 4078369..deb51d6 100644 --- a/dev/containers/entrypoint_pip.sh +++ b/dev/containers/entrypoint_pip.sh @@ -1,9 +1,17 @@ #!/bin/bash -cd / \ -&& GIT_TRACE=1 git clone -b ${BRANCH} ${REPO} /pagure \ -&& chmod +x /pagure/dev/containers/tox_py3.sh \ -&& ln -s /tox /pagure/.tox \ -&& cd /pagure \ -&& ln -s /results /pagure/results \ -&& sed -i -e 's|"alembic-3"|"alembic"|' /pagure/tests/test_alembic.py \ -&& dev/containers/tox_py3.sh + +# Print all executed commands to the terminal +set -x + +# Fail script if any commands returns an error +set -e + +cd / +GIT_TRACE=1 git clone -b ${BRANCH} ${REPO} /pagure +chmod +x /pagure/dev/containers/tox_py3.sh +ln -s /tox /pagure/.tox +cd /pagure +ln -s /results /pagure/results +sed -i -e 's|"alembic-3"|"alembic"|' /pagure/tests/test_alembic.py + +dev/containers/tox_py3.sh diff --git a/dev/containers/entrypoint_rpms.sh b/dev/containers/entrypoint_rpms.sh index 0687326..9f81fbe 100644 --- a/dev/containers/entrypoint_rpms.sh +++ b/dev/containers/entrypoint_rpms.sh @@ -1,8 +1,13 @@ #!/bin/bash -cd / \ -&& GIT_TRACE=1 git clone -b ${BRANCH} ${REPO} /pagure \ -&& chmod +x /pagure/dev/containers/runtests_py3.sh \ -&& cd /pagure \ -&& ln -s /results /pagure/results \ -&& python3 setup.py build \ -&& dev/containers/runtests_py3.sh \ No newline at end of file + +# Fail script if any commands returns an error +set -e + +cd / +GIT_TRACE=1 git clone -b ${BRANCH} ${REPO} /pagure +chmod +x /pagure/dev/containers/runtests_py3.sh +cd /pagure +ln -s /results /pagure/results +python3 setup.py build + +dev/containers/runtests_py3.sh diff --git a/dev/containers/runtests_py3.sh b/dev/containers/runtests_py3.sh index 84598e7..725bab5 100755 --- a/dev/containers/runtests_py3.sh +++ b/dev/containers/runtests_py3.sh @@ -1,5 +1,10 @@ #!/bin/bash +# Print all executed commands to the terminal +set -x + +# Fail script if any commands returns an error +set -e ls -l / @@ -8,18 +13,23 @@ echo "============== ENVIRONMENT =============" echo "============== END ENVIRONMENT =============" if [ -n "$REPO" -a -n "$BRANCH" ]; then -git remote rm proposed || true -git gc --auto -git remote add proposed "$REPO" -GIT_TRACE=1 git fetch proposed -git checkout origin/master -git config --global user.email "you@example.com" -git config --global user.name "Your Name" -git merge --no-ff "proposed/$BRANCH" -m "Merge PR" - -echo "Running tests for branch $BRANCH of repo $REPO" -echo "Last commits:" -git --no-pager log -2 + git remote rm proposed || true + git gc --auto + + # Merge into upstream/master to identify if feature branch + # is out-of-sync or has merge conflicts early in testing + git remote add proposed "$REPO" + GIT_TRACE=1 git fetch proposed + git remote add upstream https://pagure.io/pagure.git + GIT_TRACE=1 git fetch upstream + git checkout upstream/master + git config --global user.email "you@example.com" + git config --global user.name "Your Name" + git merge --no-ff "proposed/$BRANCH" -m "Merge PR" + + echo "Running tests for branch $BRANCH of repo $REPO" + echo "Last commits:" + git --no-pager log -2 fi sed -i -e "s|#!/usr/bin/env python|#!/usr/bin/env python3|" pagure/hooks/files/hookrunner diff --git a/dev/containers/tox_py3.sh b/dev/containers/tox_py3.sh index 8f94860..735a606 100755 --- a/dev/containers/tox_py3.sh +++ b/dev/containers/tox_py3.sh @@ -1,5 +1,10 @@ #!/bin/bash +# Print all executed commands to the terminal +set -x + +# Fail script if any commands returns an error +set -e ls -l / @@ -8,18 +13,23 @@ echo "============== ENVIRONMENT =============" echo "============== END ENVIRONMENT =============" if [ -n "$REPO" -a -n "$BRANCH" ]; then -git remote rm proposed || true -git gc --auto -git remote add proposed "$REPO" -GIT_TRACE=1 git fetch proposed -git checkout origin/master -git config --global user.email "you@example.com" -git config --global user.name "Your Name" -git merge --no-ff "proposed/$BRANCH" -m "Merge PR" - -echo "Running tests for branch $BRANCH of repo $REPO" -echo "Last commits:" -git --no-pager log -2 + git remote rm proposed || true + git gc --auto + + # Merge into upstream/master to identify if feature branch + # is out-of-sync or has merge conflicts early in testing + git remote add proposed "$REPO" + GIT_TRACE=1 git fetch proposed + git remote add upstream https://pagure.io/pagure.git + GIT_TRACE=1 git fetch upstream + git checkout upstream/master + git config --global user.email "you@example.com" + git config --global user.name "Your Name" + git merge --no-ff "proposed/$BRANCH" -m "Merge PR" + + echo "Running tests for branch $BRANCH of repo $REPO" + echo "Last commits:" + git --no-pager log -2 fi export LANG="en_US.UTF-8" From 49be21253ea7ce2e48019a492c44d3f2ff804ccc Mon Sep 17 00:00:00 2001 From: Dominik Wombacher Date: Apr 19 2024 11:33:48 +0000 Subject: [PATCH 55/55] tests: Rename 'alembic-3' to 'alembic' in unit tests, sed commands to replace it during container start removed. Not required anymore after dropping Python 2 a while ago. --- diff --git a/dev/containers/entrypoint_pip.sh b/dev/containers/entrypoint_pip.sh index deb51d6..52f20b4 100644 --- a/dev/containers/entrypoint_pip.sh +++ b/dev/containers/entrypoint_pip.sh @@ -12,6 +12,5 @@ chmod +x /pagure/dev/containers/tox_py3.sh ln -s /tox /pagure/.tox cd /pagure ln -s /results /pagure/results -sed -i -e 's|"alembic-3"|"alembic"|' /pagure/tests/test_alembic.py dev/containers/tox_py3.sh diff --git a/dev/containers/fedora-pip-py3 b/dev/containers/fedora-pip-py3 index afd896d..1473aea 100644 --- a/dev/containers/fedora-pip-py3 +++ b/dev/containers/fedora-pip-py3 @@ -25,8 +25,7 @@ RUN dnf -y install \ RUN cd / \ && GIT_TRACE=1 git clone -b $BRANCH $REPO \ - && chmod +x /pagure/dev/containers/tox_py3.sh \ - && sed -i -e 's|"alembic-3"|"alembic"|' /pagure/tests/test_alembic.py + && chmod +x /pagure/dev/containers/tox_py3.sh WORKDIR /pagure ENTRYPOINT ["/pagure/dev/containers/tox_py3.sh"] diff --git a/tests/test_alembic.py b/tests/test_alembic.py index 9f79937..1198bdf 100644 --- a/tests/test_alembic.py +++ b/tests/test_alembic.py @@ -31,7 +31,7 @@ class TestAlembic(unittest.TestCase): """ proc1 = subprocess.Popen( - ["alembic-3", "history"], cwd=REPO_PATH, stdout=subprocess.PIPE + ["alembic", "history"], cwd=REPO_PATH, stdout=subprocess.PIPE ) proc2 = subprocess.Popen( ["grep", " (head), "], stdin=proc1.stdout, stdout=subprocess.PIPE