From 0dd880ae93094323e157df592bdcf163dbdcc278 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Jan 31 2022 11:33:58 +0000 Subject: [PATCH 1/16] Add tox to README Signed-off-by: Michal Konečný --- diff --git a/README.rst b/README.rst index 2a2a7b5..533ee0b 100644 --- a/README.rst +++ b/README.rst @@ -122,8 +122,8 @@ The Vagrant pagure doesn't have its own log file, use ``journalctl -f`` to show the pagure output. The verbosity can be configured in the pagure config file with the ``LOGGING`` parameter. -Running the unit-tests -********************** +Running the unit-tests in container +*********************************** To run the unit-tests, there is container available with all the dependencies needed. @@ -160,6 +160,43 @@ You can also get `run-tests-container` help :: $ ./dev/run-tests-container.py --help +Run the tests on your own development branch in your fork :: + + $ ./dev/run-tests-container.py --repo https://pagure.io/forks//pagure.git --branch + + .. note:: This run could take pretty long to finish and there isn't any useful summary. + So it's better to redirect the output to some file. You can use `tee` for this. + + +Running the unit-tests in tox +***************************** + +You can run the tests using tox. This allows you to run the tests on local version of the code. + + .. note:: This way of running tests could help you test your local changes, + but the output could be different then from the containerized tests. + Always check your branch after push with containerized tests as well. + +* Install the needed system libraries:: + + sudo dnf install libgit2-devel redis gcc tox python-alembic + + + .. note:: You can also install any missing python interpreter. + For example `sudo dnf install python35` + +* Run the whole test suite:: + + tox + +* Or just single environment:: + + tox -e py39 + +* Or single module:: + + tox tests/test_style.py + Manually ^^^^^^^^ From 9c50979eb4f92ac311737b7688585933180cfb64 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:16:57 +0000 Subject: [PATCH 2/16] Update fedora version in CI containers Signed-off-by: Michal Konečný --- diff --git a/dev/containers/fedora-pip-py3 b/dev/containers/fedora-pip-py3 index 731e5b8..1c4c534 100644 --- a/dev/containers/fedora-pip-py3 +++ b/dev/containers/fedora-pip-py3 @@ -1,4 +1,4 @@ -FROM quay.io/fedora/fedora:33-x86_64 +FROM quay.io/fedora/fedora:35-x86_64 ARG repo=https://pagure.io/pagure.git ARG branch=master diff --git a/dev/containers/fedora-rpms-py3 b/dev/containers/fedora-rpms-py3 index c1d24da..90591a2 100644 --- a/dev/containers/fedora-rpms-py3 +++ b/dev/containers/fedora-rpms-py3 @@ -1,4 +1,4 @@ -FROM quay.io/fedora/fedora:32-x86_64 +FROM quay.io/fedora/fedora:35-x86_64 ARG repo=https://pagure.io/pagure.git ARG branch=master From dc1e9c39e81593466cfc1994c2e4241fdc6b1c78 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 3/16] Fix wtforms issue The Required validator is no longer available. This will use DataRequired instead Required. Signed-off-by: Michal Konečný --- diff --git a/pagure/forms.py b/pagure/forms.py index 458a74a..280ba4d 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -335,7 +335,7 @@ class RequestPullEditForm(RequestPullForm): branch_to = wtforms.SelectField( "Target branch", - [wtforms.validators.Required()], + [wtforms.validators.DataRequired()], choices=[], coerce=convert_value, ) @@ -964,7 +964,7 @@ class TriggerCIPRForm(PagureForm): self.comment.choices = choices comment = wtforms.SelectField( - "comment", [wtforms.validators.Required()], choices=[] + "comment", [wtforms.validators.DataRequired()], choices=[] ) From dbb90e2ca39a53718f7f050dc083aaabf294d536 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 4/16] Fix error with fork when namespace is missing Check if the namespace is filled before forking the repo. This value is optional on ForkRepoForm, so we can't strip it before checking if there is any. Signed-off-by: Michal Konečný --- diff --git a/pagure/api/project.py b/pagure/api/project.py index bc88523..1afdadd 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -1745,7 +1745,9 @@ def api_fork_project(): if form.validate_on_submit(): repo = form.repo.data username = form.username.data or None - namespace = form.namespace.data.strip() or None + namespace = None + if form.namespace.data: + namespace = form.namespace.data.strip() repo = get_authorized_api_project( flask.g.session, repo, user=username, namespace=namespace From 83ad40b66bb9921847ceb5f56b2ab0fe236ce3ea Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 5/16] Check the optional values first for pagure_api_fork Signed-off-by: Michal Konečný --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 95e7de8..547e93b 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -512,7 +512,10 @@ def api_pull_request_update(repo, requestid, username=None, namespace=None): ) else: request.title = form.title.data.strip() - request.initial_comment = form.initial_comment.data.strip() + request.initial_comment = "" + # This value is optional, check first if it's filled + if form.initial_comment.data: + request.initial_comment = form.initial_comment.data.strip() flask.g.session.add(request) if not request.private and not request.project.private: pagure.lib.notify.log( @@ -1535,7 +1538,10 @@ def api_pull_request_create(repo, username=None, namespace=None): if orig_commit: orig_commit = orig_commit.oid.hex - initial_comment = form.initial_comment.data.strip() or None + initial_comment = None + # This value is optional, check first if it's filled + if form.initial_comment.data: + initial_comment = form.initial_comment.data.strip() commit_start = commit_stop = None if diff_commits: From dcb9b6a9b5ceef1cede91daa25677fe64b2bd703 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 6/16] Check the optional values first for pagure_ui_issues Signed-off-by: Michal Konečný --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index cc7ece9..2722e5b 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -185,12 +185,14 @@ def update_issue(repo, issueid, username=None, namespace=None): comment = form.comment.data depends = [] - for depend in form.depending.data.split(","): - if depend.strip(): - try: - depends.append(int(depend.strip())) - except ValueError: - pass + # This field is optional, check if it's filled first + if form.depending.data: + for depend in form.depending.data.split(","): + if depend.strip(): + try: + depends.append(int(depend.strip())) + except ValueError: + pass blocks = [] for block in form.blocking.data.split(","): From 2889d1e9a996f5766b6e6c63dfdd956ca5c67e2f Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 7/16] Check optional fields first in pagure_ui_repo Signed-off-by: Michal Konečný --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 38f52dd..1997fce 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1406,18 +1406,24 @@ def update_project(repo, username=None, namespace=None): try: repo.description = form.description.data - repo.avatar_email = form.avatar_email.data.strip() - repo.url = form.url.data.strip() + # Check if the optional value is filled + if form.avatar_email.data is not None: + repo.avatar_email = form.avatar_email.data.strip() + # Check if the optional value is filled + if form.url.data: + repo.url = form.url.data.strip() if repo.private: repo.private = form.private.data if repo.mirrored_from: repo.mirrored_from = form.mirrored_from.data - pagure.lib.query.update_tags( - flask.g.session, - repo, - tags=[t.strip() for t in form.tags.data.split(",")], - username=flask.g.fas_user.username, - ) + # Check if the optional value is filled + if form.tags.data: + pagure.lib.query.update_tags( + flask.g.session, + repo, + tags=[t.strip() for t in form.tags.data.split(",")], + username=flask.g.fas_user.username, + ) flask.g.session.add(repo) flask.g.session.commit() flask.flash("Project updated") From 77a3fbf58b2164520a4598c50f15e2855e309999 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 8/16] Check optional fields first in pagure_ui_app Signed-off-by: Michal Konečný --- diff --git a/pagure/ui/app.py b/pagure/ui/app.py index 8cd2b08..90fe33b 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -1543,10 +1543,14 @@ def add_api_user_token(): if form.validate_on_submit(): try: + description = None + # Check if the optional value is filled + if form.description.data: + description = form.description.data.strip() pagure.lib.query.add_token_to_user( flask.g.session, project=None, - description=form.description.data.strip() or None, + description=description, acls=form.acls.data, username=user.username, expiration_date=form.expiration_date.data, From 7832b71a0f2ddbdaf1f18867707c8831fe32a6ea Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 9/16] Check optional fields first in pagure_ui_issues Signed-off-by: Michal Konečný --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 2722e5b..101252c 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -195,14 +195,19 @@ def update_issue(repo, issueid, username=None, namespace=None): pass blocks = [] - for block in form.blocking.data.split(","): - if block.strip(): - try: - blocks.append(int(block.strip())) - except ValueError: - pass - - assignee = form.assignee.data.strip() or None + # Check if the optional field is filled + if form.blocking.data: + for block in form.blocking.data.split(","): + if block.strip(): + try: + blocks.append(int(block.strip())) + except ValueError: + pass + + assignee = None + # Check if the optional field is filled + if form.assignee.data: + assignee = form.assignee.data.strip() new_status = form.status.data.strip() or None close_status = form.close_status.data or None if close_status not in repo.close_status: @@ -213,7 +218,10 @@ def update_issue(repo, issueid, username=None, namespace=None): new_priority = int(form.priority.data) except (ValueError, TypeError): pass - tags = [tag.strip() for tag in form.tag.data.split(",") if tag.strip()] + tags = [] + # Check if the optional field is filled + if form.tag.data: + tags = [tag.strip() for tag in form.tag.data.split(",")] new_milestone = None try: From dbda4085a2ce879747f001d79993838a44beacbc Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 10/16] Fix assert for test_pagure_flask_api_user Signed-off-by: Michal Konečný --- diff --git a/tests/test_pagure_flask_api_user.py b/tests/test_pagure_flask_api_user.py index 27bd122..3609294 100644 --- a/tests/test_pagure_flask_api_user.py +++ b/tests/test_pagure_flask_api_user.py @@ -378,7 +378,7 @@ class PagureFlaskApiUSertests(tests.Modeltests): "error": "Could not match input '2016asd' to any of the following formats: " "YYYY-MM-DD, YYYY-M-DD, YYYY-M-D, YYYY/MM/DD, YYYY/M/DD, YYYY/M/D, " "YYYY.MM.DD, YYYY.M.DD, YYYY.M.D, YYYYMMDD, YYYY-DDDD, YYYYDDDD, " - "YYYY-MM, YYYY/MM, YYYY.MM, YYYY, W", + "YYYY-MM, YYYY/MM, YYYY.MM, YYYY, W.", "error_code": "ENOCODE", } self.assertEqual(json.loads(output.get_data(as_text=True)), exp) From 5a73e49e7fafc8841ef84118f1239a3109b152f0 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 11/16] Fix issues with timestamp json dump Arrow.timestamp() is a method not property in newer arrow releases. We will use float_timestamp instead, which is available even in older versions. See https://arrow.readthedocs.io/en/latest/#arrow.arrow.Arrow.timestamp Signed-off-by: Michal Konečný --- diff --git a/pagure/api/user.py b/pagure/api/user.py index 7cfc7fa..4e31ecc 100644 --- a/pagure/api/user.py +++ b/pagure/api/user.py @@ -634,10 +634,10 @@ def api_view_user_activity_stats(username): # aim for noon on the desired date. try: - return arrow.get(d, tz).replace(hour=12).timestamp + return int(arrow.get(d, tz).replace(hour=12).float_timestamp) except (arrow.parser.ParserError, ValueError): # if tz is invalid for some reason, just go with UTC - return arrow.get(d).replace(hour=12).timestamp + return int(arrow.get(d).replace(hour=12).float_timestamp) else: d = d.isoformat() return d From 31455d7f2f3c2700e7b8d7863957ce2a1efe35ef Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 12/16] Fix the output in test_pagure_flask_ui_fork The output has some HTML strings that are not decoded. This fix will add replace to convert the value to string. Signed-off-by: Michal Konečný --- diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index e717419..bf03850 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -6382,7 +6382,7 @@ More information self.assertIn( "

fork/foo/test is not part of fork/ralph/test2's " "family

", - output.get_data(as_text=True), + output.get_data(as_text=True).replace("'", "'"), ) @patch("pagure.lib.notify.send_email") From 256ec618c6b1bfb97ffa0b4635de4ad7850f9237 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 13/16] Fix error when timestamp is None Signed-off-by: Michal Konečný --- diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index 9011bf7..9743e13 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -74,6 +74,11 @@ def format_ts(string): # always use UTC timezone, and we don't use localized forms like # %b or %d because they will be 'localized' for the *server*. # This format should be pretty 'locale-neutral'. + + # Return empty string if timestamp is None + # This will prevent any formatting error in arrow + if string is None: + return "" arr = arrow.get(string) return arr.strftime("%Y-%m-%d %H:%M:%S %Z") From ea6216a3d2658db8e94a7b40db8fa7d055ca1057 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:05 +0000 Subject: [PATCH 14/16] Check the referrer url is not None When the referrer url is None the flask redirect will fail. Signed-off-by: Michal Konečný --- diff --git a/pagure/utils.py b/pagure/utils.py index 17721a9..a99eaf8 100644 --- a/pagure/utils.py +++ b/pagure/utils.py @@ -160,7 +160,8 @@ def is_safe_url(target): # pragma: no cover ref_url = urlparse(flask.request.host_url) test_url = urlparse(urljoin(flask.request.host_url, target)) return ( - test_url.scheme in ("http", "https") + target is not None + and test_url.scheme in ("http", "https") and ref_url.netloc == test_url.netloc ) From afe40174e06342132de59f6c0882a316e9e1590e Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:17:06 +0000 Subject: [PATCH 15/16] Use ui_ns.index instead index After adding check for the None in `is_safe_url` function the index didn't work anymore. Werkzeug library error suggested to use `ui_ns.index` instead and this worked. Signed-off-by: Michal Konečný --- diff --git a/pagure/ui/app.py b/pagure/ui/app.py index 90fe33b..63467f1 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -1135,7 +1135,7 @@ def wait_task(taskid): prev = flask.request.args.get("prev") if not is_safe_url(prev): - prev = flask.url_for("index") + prev = flask.url_for("ui_ns.index") count = flask.request.args.get("count", 0) try: From ce28af0f4612a09a63f74681e501a7636149fd10 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Feb 04 2022 12:47:34 +0000 Subject: [PATCH 16/16] Fix the timestamp issue The newer version of arrow library doesn't have timestamp property. Let's use float_timestamp instead. Signed-off-by: Michal Konečný --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 74d02eb..76d1df2 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -2460,7 +2460,7 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): repo = pagure.lib.query.get_authorized_project(self.session, "test") # Create 1st tickets - start = arrow.utcnow().timestamp + start = int(arrow.utcnow().float_timestamp) issue = pagure.lib.model.Issue( id=pagure.lib.query.get_next_id(self.session, repo.id), project_id=repo.id, @@ -2474,7 +2474,7 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): self.session.commit() time.sleep(1) - middle = arrow.utcnow().timestamp + middle = int(arrow.utcnow().float_timestamp) # Create 2nd tickets issue = pagure.lib.model.Issue( @@ -2490,7 +2490,7 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): self.session.commit() time.sleep(1) - final = arrow.utcnow().timestamp + final = int(arrow.utcnow().float_timestamp) # Create private issue issue = pagure.lib.model.Issue( @@ -2553,7 +2553,6 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): ) time.sleep(1) - late = arrow.utcnow().timestamp # List all opened issues from the start output = self.app.get("/api/0/test/issues?since=%s" % start)