From 1e86e640b585f8fc3ebd98d80df7bc7766f85204 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 1/19] Fix issue when running the tests on jenkins with sqlite and log error when querying faitout --- diff --git a/tests/__init__.py b/tests/__init__.py index 383f21d..fcfec19 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -49,7 +49,8 @@ if os.environ.get('BUILD_ID'): if req.status_code == 200: DB_PATH = req.text print 'Using faitout at: %s' % DB_PATH - except: + except Exception as err: + print 'Error while querying faitout:', err pass # Remove the log handlers for the tests diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index a34c77f..6e62327 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -2465,13 +2465,14 @@ class PagureLibtests(tests.Modeltests): self.assertTrue(watch) # Entry into watchers table - pagure.lib.update_watch_status( + msg = pagure.lib.update_watch_status( session=self.session, project=project, user='pingou', - watch='0', + watch=0, ) self.session.commit() + self.assertEqual(msg, 'You are no longer watching this repo.') # From watchers table watch = pagure.lib.is_watching( From 59df6fa9176abcca860d0da4a0b99c950dd9ee4e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 2/19] Add some logging to know how the tests are being run --- diff --git a/tests/__init__.py b/tests/__init__.py index fcfec19..00c79a1 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -11,11 +11,13 @@ __requires__ = ['SQLAlchemy >= 0.7'] import pkg_resources +import logging import unittest import shutil import sys import tempfile import os +logging.basicConfig(stream=sys.stderr) from datetime import date from datetime import datetime @@ -40,17 +42,20 @@ from pagure.lib.repo import PagureRepo DB_PATH = 'sqlite:///:memory:' FAITOUT_URL = 'http://faitout.cloud.fedoraproject.org/faitout/' HERE = os.path.join(os.path.dirname(os.path.abspath(__file__))) +LOG = logging.getLogger("pagure") +LOG.setLevel(logging.DEBUG) +LOG.info('BUILD_ID: %s', os.environ.get('BUILD_ID')) if os.environ.get('BUILD_ID'): try: import requests req = requests.get('%s/new' % FAITOUT_URL) if req.status_code == 200: DB_PATH = req.text - print 'Using faitout at: %s' % DB_PATH + LOG.info('Using faitout at: %s', DB_PATH) except Exception as err: - print 'Error while querying faitout:', err + LOG.info('Error while querying faitout: %s', err) pass # Remove the log handlers for the tests From 5a49634bd60da10e71c0d6cc7c9ddc16fcf2b3f5 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 3/19] Log a little more info about what returned faitout --- diff --git a/tests/__init__.py b/tests/__init__.py index 00c79a1..e49bfb0 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -54,6 +54,8 @@ if os.environ.get('BUILD_ID'): if req.status_code == 200: DB_PATH = req.text LOG.info('Using faitout at: %s', DB_PATH) + else: + LOG.info('faitout returned: %s : %s', req.status_code, req.text) except Exception as err: LOG.info('Error while querying faitout: %s', err) pass From 7ba9a0aa8704d3c990dd0b1422174a553b0078b8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 4/19] Fix the faitout url --- diff --git a/tests/__init__.py b/tests/__init__.py index e49bfb0..64718a5 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -40,7 +40,7 @@ import pagure.lib.model from pagure.lib.repo import PagureRepo DB_PATH = 'sqlite:///:memory:' -FAITOUT_URL = 'http://faitout.cloud.fedoraproject.org/faitout/' +FAITOUT_URL = 'http://faitout.fedorainfracloud.org/' HERE = os.path.join(os.path.dirname(os.path.abspath(__file__))) LOG = logging.getLogger("pagure") LOG.setLevel(logging.DEBUG) From 07bd832c6706ba9f0897b6f7ad19a5b56cabd273 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 5/19] Add the close_status to the new_issue method in pagure.lib --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index a4e4bbc..b1056e1 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1173,7 +1173,7 @@ def new_project(session, user, name, blacklist, allowed_prefix, def new_issue(session, repo, title, content, user, ticketfolder, issue_id=None, issue_uid=None, private=False, status=None, - notify=True, date_created=None): + close_status=None, notify=True, date_created=None): ''' Create a new issue for the specified repo. ''' user_obj = get_user(session, user) @@ -1190,6 +1190,8 @@ def new_issue(session, repo, title, content, user, ticketfolder, if status is not None: issue.status = status + if close_status is not None: + issue.close_status = close_status session.add(issue) # Make sure we won't have SQLAlchemy error before we create the issue From fad6c15ba75a547052635fc0da1ec7455e1323d4 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 6/19] Adjust the unit-tests for the changes related to the close_status --- diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index f5c02fc..f60e430 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -269,7 +269,8 @@ class PagureFlaskIssuestests(tests.Modeltests): title='Test invalid issue', content='This really is not related', user='pingou', - status='Invalid', + status='Closed', + close_status='Invalid', ticketfolder=None ) self.session.commit() diff --git a/tests/test_pagure_flask_ui_roadmap.py b/tests/test_pagure_flask_ui_roadmap.py index 06bdd16..c01da82 100644 --- a/tests/test_pagure_flask_ui_roadmap.py +++ b/tests/test_pagure_flask_ui_roadmap.py @@ -474,7 +474,8 @@ class PagureFlaskRoadmaptests(tests.Modeltests): repo, issueid=iid ) - ticket.status = 'Fixed' + ticket.status = 'Closed' + ticket.close_status = 'Fixed' self.session.add(ticket) self.session.commit() diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 6e62327..27f5496 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -259,7 +259,8 @@ class PagureLibtests(tests.Modeltests): ticketfolder=None, title='Foo issue #2', content='We should work on this period', - status='Invalid', + status='Closed', + close_status='Invalid', private=True, ) self.session.commit() @@ -490,16 +491,18 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(issues[1].tags, []) self.assertEqual(issues[0].id, 2) self.assertEqual(issues[0].project_id, 1) - self.assertEqual(issues[0].status, 'Invalid') + self.assertEqual(issues[0].status, 'Closed') + self.assertEqual(issues[0].close_status, 'Invalid') self.assertEqual(issues[0].tags, []) # Issues by status issues = pagure.lib.search_issues( - self.session, repo, status='Invalid') + self.session, repo, status='Closed') self.assertEqual(len(issues), 1) self.assertEqual(issues[0].id, 2) self.assertEqual(issues[0].project_id, 1) - self.assertEqual(issues[0].status, 'Invalid') + self.assertEqual(issues[0].status, 'Closed') + self.assertEqual(issues[0].close_status, 'Invalid') self.assertEqual(issues[0].tags, []) # Issues closed @@ -508,7 +511,8 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(len(issues), 1) self.assertEqual(issues[0].id, 2) self.assertEqual(issues[0].project_id, 1) - self.assertEqual(issues[0].status, 'Invalid') + self.assertEqual(issues[0].status, 'Closed') + self.assertEqual(issues[0].close_status, 'Invalid') self.assertEqual(issues[0].tags, []) # Issues by tag @@ -528,7 +532,8 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(len(issues), 1) self.assertEqual(issues[0].id, 2) self.assertEqual(issues[0].project_id, 1) - self.assertEqual(issues[0].status, 'Invalid') + self.assertEqual(issues[0].status, 'Closed') + self.assertEqual(issues[0].close_status, 'Invalid') self.assertEqual(issues[0].tags, []) # Issues by assignee From 324de6ec4c1125c98e5cab4c91f98433fc85c3a0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 7/19] More unit-test fixes Ensure the order remains consistent Use boolean where postgresql expects them --- diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 27f5496..936b653 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -124,8 +124,8 @@ class PagureLibtests(tests.Modeltests): self.assertEqual('pingou', items[0].username) self.assertEqual([], items[0].groups) self.assertEqual( - ['bar@pingou.com', 'foo@pingou.com'], - [email.email for email in items[0].emails]) + sorted(['bar@pingou.com', 'foo@pingou.com']), + sorted([email.email for email in items[0].emails])) self.assertEqual(3, items[1].id) self.assertEqual('pingou2', items[1].user) self.assertEqual('pingou2', items[1].username) @@ -1109,8 +1109,8 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(3, len(items)) self.assertEqual('skvidal', items[2].user) self.assertEqual( - ['skvidal@fp.o', 'svidal@fp.o'], - [email.email for email in items[2].emails]) + sorted(['skvidal@fp.o', 'svidal@fp.o']), + sorted([email.email for email in items[2].emails])) def test_update_user_ssh(self): """ Test the update_user_ssh of pagure.lib. """ @@ -2367,7 +2367,7 @@ class PagureLibtests(tests.Modeltests): session=self.session, project=project, user='aavrug', - watch='1', + watch=True, ) # All good and when user seleted watch option. @@ -2375,7 +2375,7 @@ class PagureLibtests(tests.Modeltests): session=self.session, project=project, user='pingou', - watch='1', + watch=True, ) self.session.commit() self.assertEqual(msg, 'You are now watching this repo.') @@ -2385,7 +2385,7 @@ class PagureLibtests(tests.Modeltests): session=self.session, project=project, user='pingou', - watch='0', + watch=False, ) self.session.commit() self.assertEqual(msg, 'You are no longer watching this repo.') @@ -2457,7 +2457,7 @@ class PagureLibtests(tests.Modeltests): session=self.session, project=project, user='pingou', - watch='1', + watch=True, ) self.session.commit() @@ -2474,7 +2474,7 @@ class PagureLibtests(tests.Modeltests): session=self.session, project=project, user='pingou', - watch=0, + watch=False, ) self.session.commit() self.assertEqual(msg, 'You are no longer watching this repo.') From 1e10d6f5634d14d3d9a98affacf163456196cc09 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 8/19] Fix bug when changing the status of a ticket via the API --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index bdf6295..157f9cc 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -571,7 +571,7 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None): new_status = form.status.data.strip() if new_status in repo.close_status and not close_status: close_status = new_status - form.status.data = 'Closed' + new_status = 'Closed' if form.validate_on_submit(): try: From cfd95d8cdf139fe1341cc5e7bf241e004fa72f3b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 9/19] Fix search_issues to be backward compatible and search the close_status where desired --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index b1056e1..1fbf29f 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1757,9 +1757,14 @@ def search_issues( ) if status is not None and not closed: - query = query.filter( - model.Issue.status == status - ) + if status == 'Open': + query = query.filter( + model.Issue.status == status + ) + else: + query = query.filter( + model.Issue.close_status == status + ) if closed: query = query.filter( model.Issue.status != 'Open' From 68ccb023d74630157af7838c838ec75e9f97c639 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 10/19] We're searching closed issues everytime we're not searching open ones --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 3f9acb8..4a25511 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -464,7 +464,7 @@ def view_issues(repo, username=None, namespace=None): issues = pagure.lib.search_issues( SESSION, repo, - closed=True if status.lower() == 'closed' else False, + closed=True if status.lower() != 'open' else False, status=status.capitalize() if status.lower() != 'closed' else None, tags=tags, assignee=assignee, @@ -478,7 +478,7 @@ def view_issues(repo, username=None, namespace=None): issues_cnt = pagure.lib.search_issues( SESSION, repo, - closed=True if status.lower() == 'closed' else False, + closed=True if status.lower() != 'open' else False, status=status.capitalize() if status.lower() != 'closed' else None, tags=tags, assignee=assignee, @@ -490,7 +490,7 @@ def view_issues(repo, username=None, namespace=None): oth_issues = pagure.lib.search_issues( SESSION, repo, - closed=False if status.lower() == 'closed' else True, + closed=True if status.lower() != 'open' else False, tags=tags, assignee=assignee, author=author, From 8558c84fc09f91c017b2da785bb838ade2080b33 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 11/19] Adjust unit-tests to be more flexible with the ordering --- diff --git a/tests/test_pagure_flask_ui_app.py b/tests/test_pagure_flask_ui_app.py index 75d7423..279f0f8 100644 --- a/tests/test_pagure_flask_ui_app.py +++ b/tests/test_pagure_flask_ui_app.py @@ -670,9 +670,13 @@ class PagureFlaskApptests(tests.Modeltests): '/settings/email/add', data=data, follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertTrue("Add new email" in output.data) - self.assertIn( + self.assertTrue( 'Invalid value, can't be any of: bar@pingou.com, ' - 'foo@pingou.com. ', output.data) + 'foo@pingou.com. ' in output.data + or + 'Invalid value, can't be any of: foo@pingou.com, ' + 'bar@pingou.com. ' in output.data + ) self.assertEqual(output.data.count('foo@pingou.com'), 6) self.assertEqual(output.data.count('bar@pingou.com'), 5) self.assertEqual(output.data.count('foobar@pingou.com'), 0) From 7252593224e7f1dd08e24bfd5b9b05248dcfe7e9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 12/19] Fix search issues by status --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 1fbf29f..593eeb7 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1757,7 +1757,7 @@ def search_issues( ) if status is not None and not closed: - if status == 'Open': + if status in ['Open', 'Closed']: query = query.filter( model.Issue.status == status ) From 4661b9f5b8a49917751c7bae3719e889d0254dd4 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 13/19] Fix the ordering in the tests --- diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 936b653..2b49320 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -78,8 +78,8 @@ class PagureLibtests(tests.Modeltests): item = pagure.lib.search_user(self.session, email='foo@pingou.com') self.assertEqual('pingou', item.user) self.assertEqual( - ['bar@pingou.com', 'foo@pingou.com'], - [email.email for email in item.emails]) + sorted(['bar@pingou.com', 'foo@pingou.com']), + sorted([email.email for email in item.emails])) def test_search_user_token(self): """ Test the search_user of pagure.lib. """ From ee58a119a5bf03c63d6049796903a564cb25bfff Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:13 +0000 Subject: [PATCH 14/19] Sort the emails when generating the JSON view of an user --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index f639e14..cfdeb1e 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -223,7 +223,7 @@ class User(BASE): } if not public: output['default_email'] = self.default_email - output['emails'] = [email.email for email in self.emails] + output['emails'] = sorted([email.email for email in self.emails]) return output From a61250c4776970387d6ba7c4b36d69e4c3366518 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:14 +0000 Subject: [PATCH 15/19] Fix searching for closed issues --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 593eeb7..b3da502 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1756,7 +1756,7 @@ def search_issues( model.Issue.uid == issueuid ) - if status is not None and not closed: + if status is not None: if status in ['Open', 'Closed']: query = query.filter( model.Issue.status == status From ecec59765d2bdb1707e1d87062409d1af291ea51 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:14 +0000 Subject: [PATCH 16/19] Set the session in pagure.ui.app as well to run the tests --- diff --git a/tests/test_pagure_flask_ui_groups.py b/tests/test_pagure_flask_ui_groups.py index 8efd9b2..b505f16 100644 --- a/tests/test_pagure_flask_ui_groups.py +++ b/tests/test_pagure_flask_ui_groups.py @@ -36,6 +36,7 @@ class PagureFlaskGroupstests(tests.Modeltests): pagure.APP.config['TESTING'] = True pagure.SESSION = self.session pagure.ui.SESSION = self.session + pagure.ui.app.SESSION = self.session pagure.ui.groups.SESSION = self.session pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session From 3f2e4b52a6da3529c4e4ebcfe0c538691862ce0f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:14 +0000 Subject: [PATCH 17/19] Add some debugging code --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 201375a..6e4ad73 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -774,6 +774,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): # Valid request output = self.app.post( '/api/0/test/issue/1/status', data=data, headers=headers) + print output.data self.assertEqual(output.status_code, 200) data = json.loads(output.data) self.assertDictEqual( From 51c459489c92bb18353754d31c7ffb21d66e8a35 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:21:14 +0000 Subject: [PATCH 18/19] Set the status in the form so that it validates --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 157f9cc..4f0c64a 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -572,6 +572,7 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None): if new_status in repo.close_status and not close_status: close_status = new_status new_status = 'Closed' + form.status.data = new_status if form.validate_on_submit(): try: From 621b9538c91d5d01abcaf509e8bcc1dc4e6c159a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 07 2016 21:35:50 +0000 Subject: [PATCH 19/19] Remove debugging code --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 6e4ad73..201375a 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -774,7 +774,6 @@ class PagureFlaskApiIssuetests(tests.Modeltests): # Valid request output = self.app.post( '/api/0/test/issue/1/status', data=data, headers=headers) - print output.data self.assertEqual(output.status_code, 200) data = json.loads(output.data) self.assertDictEqual(