From a7f281f9c19b619dd9b27e5a506f7394360b8b55 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mar 23 2018 16:46:14 +0000 Subject: [PATCH 1/9] Use task objects instead of task IDs when possible --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index eedd6f3..fa4c74b 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -362,14 +362,14 @@ def api_pull_request_merge(repo, requestid, username=None, namespace=None): raise pagure.exceptions.APIError(403, error_code=APIERROR.EPRSCORE) try: - taskid = pagure.lib.tasks.merge_pull_request.delay( + task = pagure.lib.tasks.merge_pull_request.delay( repo.name, namespace, username, requestid, - flask.g.fas_user.username).id + flask.g.fas_user.username) output = {'message': 'Merging queued', - 'taskid': taskid} + 'taskid': task.id} if flask.request.form.get('wait', True): - pagure.lib.tasks.get_result(taskid).get() + task.get() output = {'message': 'Changes merged!'} except pagure.exceptions.PagureException as err: raise pagure.exceptions.APIError( diff --git a/pagure/api/project.py b/pagure/api/project.py index b5bba89..70d2dcf 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -841,7 +841,7 @@ def api_new_project(): private = form.private.data try: - taskid = pagure.lib.new_project( + task = pagure.lib.new_project( flask.g.session, name=name, namespace=namespace, @@ -864,10 +864,10 @@ def api_new_project(): ) flask.g.session.commit() output = {'message': 'Project creation queued', - 'taskid': taskid} + 'taskid': task.id} if flask.request.form.get('wait', True): - result = pagure.lib.tasks.get_result(taskid).get() + result = task.get() project = pagure.lib._get_project( flask.g.session, name=result['repo'], namespace=result['namespace'], @@ -1103,7 +1103,7 @@ def api_fork_project(): 404, error_code=APIERROR.ENOPROJECT) try: - taskid = pagure.lib.fork_project( + task = pagure.lib.fork_project( flask.g.session, user=flask.g.fas_user.username, repo=repo, @@ -1114,10 +1114,10 @@ def api_fork_project(): ) flask.g.session.commit() output = {'message': 'Project forking queued', - 'taskid': taskid} + 'taskid': task.id} if flask.request.form.get('wait', True): - pagure.lib.tasks.get_result(taskid).get() + task.get() output = {'message': 'Repo "%s" cloned to "%s/%s"' % (repo.fullname, flask.g.fas_user.username, repo.fullname)} @@ -1203,16 +1203,16 @@ def api_generate_acls(repo, username=None, namespace=None): wait = str(flask.request.form.get('wait')).lower() in ['true', '1'] try: - taskid = pagure.lib.git.generate_gitolite_acls( + task = pagure.lib.git.generate_gitolite_acls( project=project, - ).id + ) if wait: - pagure.lib.tasks.get_result(taskid).get() + task.get() output = {'message': 'Project ACLs generated'} else: output = {'message': 'Project ACL generation queued', - 'taskid': taskid} + 'taskid': task.id} except pagure.exceptions.PagureException as err: raise pagure.exceptions.APIError( 400, error_code=APIERROR.ENOCODE, error=str(err)) diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 4ccc5b5..f4e0877 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1578,7 +1578,7 @@ def new_project(session, user, name, blacklist, allowed_prefix, ) return tasks.create_project.delay(user_obj.username, namespace, name, - add_readme, ignore_existing_repo).id + add_readme, ignore_existing_repo) def new_issue(session, repo, title, content, user, ticketfolder, issue_id=None, @@ -2063,7 +2063,7 @@ def fork_project(session, user, repo, gitfolder, user, editbranch, editfile) - return task.id + return task def search_projects( diff --git a/pagure/ui/app.py b/pagure/ui/app.py index f8d3476..786b6e7 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -28,6 +28,7 @@ from pagure.utils import ( authenticated, is_safe_url, login_required, + get_task_redirect_url, ) @@ -513,7 +514,7 @@ def new_project(): namespace = namespace.strip() try: - taskid = pagure.lib.new_project( + task = pagure.lib.new_project( flask.g.session, name=name, private=private, @@ -535,7 +536,7 @@ def new_project(): user_ns=pagure_config.get('USER_NAMESPACE', False), ) flask.g.session.commit() - return pagure.utils.wait_for_task(taskid) + return pagure.utils.wait_for_task(task) except pagure.exceptions.PagureException as err: flask.flash(str(err), 'error') except SQLAlchemyError as err: # pragma: no cover @@ -574,16 +575,7 @@ def wait_task(taskid): if task.ready(): if is_js: flask.abort(417) - - result = task.get(timeout=0, propagate=False) - if task.failed(): - flask.flash('Your task failed: %s' % str(result)) - task.forget() - return flask.redirect(prev) - endpoint = result.pop('endpoint') - task.forget() - return flask.redirect( - flask.url_for(endpoint, **result)) + return flask.redirect(get_task_redirect_url(task, prev)) else: if is_js: return flask.jsonify({ diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 63a112f..1424130 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -778,11 +778,11 @@ def merge_request_pull(repo, requestid, username=None, namespace=None): _log.info('All checks in the controller passed') try: - taskid = pagure.lib.tasks.merge_pull_request.delay( + task = pagure.lib.tasks.merge_pull_request.delay( repo.name, namespace, username, requestid, flask.g.fas_user.username) return pagure.utils.wait_for_task( - taskid, + task, prev=flask.url_for('ui_ns.request_pull', repo=repo.name, namespace=namespace, @@ -896,10 +896,10 @@ def refresh_request_pull(repo, requestid, username=None, namespace=None): 403, 'You are not allowed to refresh this pull request') - taskid = pagure.lib.tasks.refresh_remote_pr.delay( + task = pagure.lib.tasks.refresh_remote_pr.delay( flask.g.repo.name, namespace, username, requestid) return pagure.utils.wait_for_task( - taskid, + task, prev=flask.url_for('ui_ns.request_pull', repo=flask.g.repo.name, namespace=namespace, @@ -1035,7 +1035,7 @@ def fork_project(repo, username=None, namespace=None): namespace=namespace)) try: - taskid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=flask.g.session, repo=repo, gitfolder=pagure_config['GIT_FOLDER'], @@ -1046,7 +1046,7 @@ def fork_project(repo, username=None, namespace=None): flask.g.session.commit() return pagure.utils.wait_for_task( - taskid, + task, prev=flask.url_for( 'ui_ns.view_repo', repo=repo.name, username=username, namespace=namespace, @@ -1477,7 +1477,7 @@ def fork_edit_file( )) try: - taskid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=flask.g.session, repo=repo, gitfolder=pagure_config['GIT_FOLDER'], @@ -1489,7 +1489,7 @@ def fork_edit_file( editfile=filename) flask.g.session.commit() - return pagure.utils.wait_for_task(taskid) + return pagure.utils.wait_for_task(task) except pagure.exceptions.PagureException as err: flask.flash(str(err), 'error') except SQLAlchemyError as err: # pragma: no cover diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index fa8e83f..ff89933 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1429,7 +1429,7 @@ def delete_repo(repo, username=None, namespace=None): name=repo.name, user=repo.user.user if repo.is_fork else None, action_user=flask.g.fas_user.username) - return pagure.utils.wait_for_task(task.id) + return pagure.utils.wait_for_task(task) @UI_NS.route('//hook_token', methods=['POST']) @@ -2048,7 +2048,7 @@ def edit_file(repo, branchname, filename, username=None, namespace=None): if form.validate_on_submit(): try: - taskid = pagure.lib.tasks.update_file_in_git.delay( + task = pagure.lib.tasks.update_file_in_git.delay( repo.name, repo.namespace, repo.user.username if repo.is_fork else None, @@ -2063,9 +2063,8 @@ def edit_file(repo, branchname, filename, username=None, namespace=None): username=user.username, email=form.email.data, runhook=True, - ).id - return flask.redirect(flask.url_for( - 'ui_ns.wait_task', taskid=taskid)) + ) + return pagure.utils.wait_for_task(task) except pagure.exceptions.PagureException as err: # pragma: no cover _log.exception(err) flask.flash('Commit could not be done', 'error') @@ -2132,9 +2131,9 @@ def delete_branch(repo, branchname, username=None, namespace=None): if branchname not in repo_obj.listall_branches(): flask.abort(404, 'Branch not found') - taskid = pagure.lib.tasks.delete_branch.delay(repo, namespace, username, - branchname).id - return pagure.utils.wait_for_task(taskid) + task = pagure.lib.tasks.delete_branch.delay(repo, namespace, username, + branchname) + return pagure.utils.wait_for_task(task) @UI_NS.route('/docs//') @@ -2568,10 +2567,10 @@ def project_dowait(repo, username=None, namespace=None): if not pagure_config.get('ALLOW_PROJECT_DOWAIT', False): flask.abort(401, 'No') - taskid = pagure.lib.tasks.project_dowait.delay( - name=repo, namespace=namespace, user=username).id + task = pagure.lib.tasks.project_dowait.delay( + name=repo, namespace=namespace, user=username) - return pagure.utils.wait_for_task(taskid) + return pagure.utils.wait_for_task(task) @UI_NS.route('//stats/') diff --git a/pagure/utils.py b/pagure/utils.py index 0a82058..48360ab 100644 --- a/pagure/utils.py +++ b/pagure/utils.py @@ -320,13 +320,28 @@ def get_remote_repo_path(remote_git, branch_from, ignore_non_exist=False): return repopath -def wait_for_task(taskid, prev=None): +def get_task_redirect_url(task, prev): + if not task.ready(): + return flask.url_for( + 'ui_ns.wait_task', + taskid=task.id, + prev=prev) + result = task.get(timeout=0, propagate=False) + if task.failed(): + flask.flash('Your task failed: %s' % str(result)) + task.forget() + return prev + endpoint = result.pop('endpoint') + task.forget() + return flask.url_for(endpoint, **result) + + +def wait_for_task(task, prev=None): if prev is None: prev = flask.request.full_path - return flask.redirect(flask.url_for( - 'ui_ns.wait_task', - taskid=taskid, - prev=prev)) + elif not is_safe_url(prev): + prev = flask.url_for('index') + return flask.redirect(get_task_redirect_url(task, prev)) def wait_for_task_post(taskid, form, endpoint, initial=False, **kwargs): diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index a950152..ca357a1 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -2616,9 +2616,8 @@ class PagureFlaskApiProjecttests(tests.Modeltests): mock_gen_acls.assert_called_once_with( name='test', namespace=None, user=None, group=None) - @patch('pagure.lib.tasks.get_result') @patch('pagure.lib.tasks.generate_gitolite_acls.delay') - def test_api_generate_acls_wait_true(self, mock_gen_acls, mock_get_result): + def test_api_generate_acls_wait_true(self, mock_gen_acls): """ Test the api_generate_acls method of the flask api when wait is set to True """ tests.create_projects(self.session) @@ -2631,9 +2630,6 @@ class PagureFlaskApiProjecttests(tests.Modeltests): mock_gen_acls_rv.id = 'abc-1234' mock_gen_acls.return_value = mock_gen_acls_rv - mock_get_result_rv = Mock() - mock_get_result.return_value = mock_get_result_rv - user = pagure.lib.get_user(self.session, 'pingou') with tests.user_set(self.app.application, user): output = self.app.post( @@ -2647,7 +2643,6 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.assertEqual(data, expected_output) mock_gen_acls.assert_called_once_with( name='test', namespace=None, user=None, group=None) - mock_get_result.assert_called_once_with('abc-1234') def test_api_generate_acls_no_project(self): """ Test the api_generate_acls method of the flask api when the project diff --git a/tests/test_pagure_flask_ui_plugins_default_hook.py b/tests/test_pagure_flask_ui_plugins_default_hook.py index 15863f9..462afed 100644 --- a/tests/test_pagure_flask_ui_plugins_default_hook.py +++ b/tests/test_pagure_flask_ui_plugins_default_hook.py @@ -46,7 +46,7 @@ class PagureFlaskPluginDefaultHooktests(tests.Modeltests): project is created. """ - taskid = pagure.lib.new_project( + task = pagure.lib.new_project( self.session, user='pingou', name='test', @@ -64,8 +64,7 @@ class PagureFlaskPluginDefaultHooktests(tests.Modeltests): prevent_40_chars=False, namespace=None ) - result = pagure.lib.tasks.get_result(taskid).get() - self.assertEqual(result, + self.assertEqual(task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'test', 'namespace': None}) diff --git a/tests/test_pagure_flask_ui_repo_delete_project.py b/tests/test_pagure_flask_ui_repo_delete_project.py index 7eaecf4..7641293 100644 --- a/tests/test_pagure_flask_ui_repo_delete_project.py +++ b/tests/test_pagure_flask_ui_repo_delete_project.py @@ -53,7 +53,7 @@ class PagureFlaskDeleteRepotests(tests.Modeltests): self.session.commit() # Create a fork - task_id = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='pingou', repo=project, @@ -62,7 +62,7 @@ class PagureFlaskDeleteRepotests(tests.Modeltests): ticketfolder=os.path.join(self.path, 'repos', 'tickets'), requestfolder=os.path.join(self.path, 'repos', 'requests'), ) - pagure.lib.tasks.get_result(task_id).get() + task.get() # Ensure everything was correctly created projects = pagure.lib.search_projects(self.session) diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index e4675fa..6f5e53b 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -1600,7 +1600,7 @@ class PagureLibtests(tests.Modeltests): # Create a new project pagure.config.config['GIT_FOLDER'] = gitfolder - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -1614,9 +1614,8 @@ class PagureLibtests(tests.Modeltests): parent_id=None, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': None}) @@ -1700,7 +1699,7 @@ class PagureLibtests(tests.Modeltests): self.session.commit() self.session = pagure.lib.create_session(self.dbpath) - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -1715,9 +1714,8 @@ class PagureLibtests(tests.Modeltests): ignore_existing_repo=True ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': None}) @@ -1734,7 +1732,7 @@ class PagureLibtests(tests.Modeltests): # Drop the main git repo and try again shutil.rmtree(gitrepo) - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -1748,7 +1746,7 @@ class PagureLibtests(tests.Modeltests): parent_id=None) self.assertIn( 'already exists', - str(pagure.lib.tasks.get_result(tid).get(propagate=False))) + str(task.get(propagate=False))) self.session.rollback() self.assertFalse(os.path.exists(gitrepo)) @@ -1803,7 +1801,7 @@ class PagureLibtests(tests.Modeltests): self.assertTrue(os.path.exists(requestrepo)) # Re-Try creating a 40 chars project this time allowing it - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='pingou/' + 's' * 40, @@ -1817,9 +1815,8 @@ class PagureLibtests(tests.Modeltests): parent_id=None, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'pingou/ssssssssssssssssssssssssssssssssssssssss', 'namespace': None}) @@ -1833,7 +1830,7 @@ class PagureLibtests(tests.Modeltests): # Create a new project with user_ns as True pagure.config.config['GIT_FOLDER'] = gitfolder - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -1848,9 +1845,8 @@ class PagureLibtests(tests.Modeltests): user_ns=True, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': 'pingou'}) @@ -1870,7 +1866,7 @@ class PagureLibtests(tests.Modeltests): # Create a new project with a namespace and user_ns as True pagure.config.config['GIT_FOLDER'] = gitfolder - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject2', @@ -1886,9 +1882,8 @@ class PagureLibtests(tests.Modeltests): user_ns=True, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject2', 'namespace': 'testns'}) @@ -2227,7 +2222,7 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(len(projects), 0) # Create a new project - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -2241,9 +2236,8 @@ class PagureLibtests(tests.Modeltests): parent_id=None, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': None}) @@ -2270,7 +2264,7 @@ class PagureLibtests(tests.Modeltests): # Fork - tid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=project, @@ -2280,9 +2274,8 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': None, @@ -2310,7 +2303,7 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(len(projects), 0) # Create a new project - taskid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -2325,8 +2318,7 @@ class PagureLibtests(tests.Modeltests): parent_id=None, ) self.session.commit() - result = pagure.lib.tasks.get_result(taskid).get() - self.assertEqual(result, + self.assertEqual(task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': 'foonamespace'}) @@ -2367,7 +2359,7 @@ class PagureLibtests(tests.Modeltests): grepo = '%s.git' % os.path.join( docfolder, 'forks', 'foo', 'foonamespace', 'testproject') os.makedirs(grepo) - tid = pagure.lib.fork_project(session=self.session, + task = pagure.lib.fork_project(session=self.session, user='foo', repo=repo, gitfolder=gitfolder, @@ -2376,7 +2368,7 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder) self.assertIn( 'already exists', - str(pagure.lib.tasks.get_result(tid).get(propagate=False))) + str(task.get(propagate=False))) self.session.rollback() shutil.rmtree(grepo) @@ -2384,7 +2376,7 @@ class PagureLibtests(tests.Modeltests): grepo = '%s.git' % os.path.join( ticketfolder, 'forks', 'foo', 'foonamespace', 'testproject') os.makedirs(grepo) - tid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=repo, @@ -2394,7 +2386,7 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder) self.assertIn( 'already exists', - str(pagure.lib.tasks.get_result(tid).get(propagate=False))) + str(task.get(propagate=False))) self.session.rollback() shutil.rmtree(grepo) @@ -2402,7 +2394,7 @@ class PagureLibtests(tests.Modeltests): grepo = '%s.git' % os.path.join( requestfolder, 'forks', 'foo', 'foonamespace', 'testproject') os.makedirs(grepo) - tid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=repo, @@ -2412,13 +2404,13 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder) self.assertIn( 'already exists', - str(pagure.lib.tasks.get_result(tid).get(propagate=False))) + str(task.get(propagate=False))) self.session.rollback() shutil.rmtree(grepo) # Fork worked - tid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=repo, @@ -2428,9 +2420,8 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': 'foonamespace', @@ -2440,7 +2431,7 @@ class PagureLibtests(tests.Modeltests): repo = pagure.lib._get_project(self.session, 'testproject', user='foo', namespace='foonamespace') - tid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='pingou', repo=repo, @@ -2450,9 +2441,8 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': 'foonamespace', diff --git a/tests/test_pagure_lib_git.py b/tests/test_pagure_lib_git.py index a041b4d..721aebf 100644 --- a/tests/test_pagure_lib_git.py +++ b/tests/test_pagure_lib_git.py @@ -3025,7 +3025,7 @@ index 0000000..60f7480 repo_obj = pygit2.init_repository(self.gitrepo, bare=True) # Fork the project - taskid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=repo, @@ -3035,8 +3035,7 @@ index 0000000..60f7480 requestfolder=requestfolder, ) self.session.commit() - result = pagure.lib.tasks.get_result(taskid).get() - self.assertEqual(result, + self.assertEqual(task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'test', 'username': 'foo', @@ -3113,7 +3112,7 @@ index 0000000..60f7480 tests.add_content_git_repo(self.gitrepo, branch='master') # Fork the project - taskid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=repo, @@ -3123,8 +3122,7 @@ index 0000000..60f7480 requestfolder=requestfolder, ) self.session.commit() - result = pagure.lib.tasks.get_result(taskid).get() - self.assertEqual(result, + self.assertEqual(task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'test', 'username': 'foo', From 57d7943abd4b5bb03e8712fe319d8175f2438f6b Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mar 23 2018 16:46:14 +0000 Subject: [PATCH 2/9] Drop Faitout support --- diff --git a/tests/__init__.py b/tests/__init__.py index 0646b80..ae78f71 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -53,10 +53,6 @@ import pagure.perfrepo as perfrepo from pagure.config import config as pagure_config from pagure.lib.repo import PagureRepo -DB_PATH = None -FAITOUT_URL = 'http://faitout.fedorainfracloud.org/' -if os.environ.get('FAITOUT_URL'): - FAITOUT_URL = os.environ.get('FAITOUT_URL') HERE = os.path.join(os.path.dirname(os.path.abspath(__file__))) LOG = logging.getLogger(__name__) LOG.setLevel(logging.INFO) @@ -79,18 +75,6 @@ MAX_NOFILE = 4096 LOG.info('BUILD_ID: %s', os.environ.get('BUILD_ID')) -if os.environ.get('BUILD_ID')or os.environ.get('FAITOUT_URL'): - try: - import requests - req = requests.get('%s/new' % FAITOUT_URL) - 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 WAIT_REGEX = re.compile("""var _url = '(\/wait\/[a-z0-9-]+\??.*)'""") @@ -189,8 +173,6 @@ def _create_db_entities(dbpath): def setUp(): - if DB_PATH: - return dbpath = 'sqlite:///%s' % dbfile _create_db_entities(dbpath) @@ -254,13 +236,9 @@ class SimplePagureTest(unittest.TestCase): perfrepo.REQUESTS = [] def _set_db_path(self): - if DB_PATH: - self.dbpath = DB_PATH - _create_db_entities(self.dbpath) - else: - self.dbpath = 'sqlite:///%s' % os.path.join( - self.path, 'db.sqlite') - shutil.copyfile(dbfile, self.dbpath[len('sqlite://'):]) + self.dbpath = 'sqlite:///%s' % os.path.join( + self.path, 'db.sqlite') + shutil.copyfile(dbfile, self.dbpath[len('sqlite://'):]) def setUp(self): self.perfReset() @@ -310,12 +288,6 @@ class SimplePagureTest(unittest.TestCase): def tearDown(self): self.session.close() - # Clear DB - if self.dbpath.startswith('postgres'): - if 'localhost' not in self.dbpath: - db_name = self.dbpath.rsplit('/', 1)[1] - requests.get('%s/clean/%s' % (FAITOUT_URL, db_name)) - # Remove testdir try: shutil.rmtree(self.path) From 362c30939951a4fa89c4bf5d2857f78eefde72b9 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mar 23 2018 16:46:14 +0000 Subject: [PATCH 3/9] Fix symlink --- diff --git a/utils/perfrepo.py b/utils/perfrepo.py index bb0e407..2ee7718 120000 --- a/utils/perfrepo.py +++ b/utils/perfrepo.py @@ -1 +1 @@ -pagure/perfrepo.py \ No newline at end of file +../pagure/perfrepo.py \ No newline at end of file From 13bcde9ec875491173bf3aa4d7ecd9922750d528 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mar 23 2018 16:46:14 +0000 Subject: [PATCH 4/9] Tests: commit instead of recreating a session --- diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index b793a61..65f1b82 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -795,7 +795,7 @@ class PagureFlaskApiForktests(tests.Modeltests): self.assertEqual(req.title, 'test pull-request') # Check comments before - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 0) @@ -819,7 +819,7 @@ class PagureFlaskApiForktests(tests.Modeltests): ) # No change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 0) @@ -839,7 +839,7 @@ class PagureFlaskApiForktests(tests.Modeltests): ) # One comment added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 1) @@ -912,7 +912,7 @@ class PagureFlaskApiForktests(tests.Modeltests): self.assertEqual(req.title, 'test pull-request') # Check comments before - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 0) @@ -936,7 +936,7 @@ class PagureFlaskApiForktests(tests.Modeltests): ) # No change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 0) @@ -956,7 +956,7 @@ class PagureFlaskApiForktests(tests.Modeltests): ) # One comment added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 1) diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index b192a97..f5a8f97 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -2504,7 +2504,7 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): ) # One comment added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(issue.assignee.user, 'pingou') @@ -3016,7 +3016,7 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): } ) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.other_fields), 1) @@ -3039,7 +3039,7 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): } ) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.other_fields), 0) diff --git a/tests/test_pagure_flask_api_issue_custom_fields.py b/tests/test_pagure_flask_api_issue_custom_fields.py index 8b13fea..d15ccc5 100644 --- a/tests/test_pagure_flask_api_issue_custom_fields.py +++ b/tests/test_pagure_flask_api_issue_custom_fields.py @@ -122,7 +122,7 @@ class PagureFlaskApiCustomFieldIssuetests(tests.Modeltests): } ) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.other_fields), 1) @@ -149,7 +149,7 @@ class PagureFlaskApiCustomFieldIssuetests(tests.Modeltests): } ) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.other_fields), 3) diff --git a/tests/test_pagure_flask_api_pr_flag.py b/tests/test_pagure_flask_api_pr_flag.py index 9f1b47e..3f044b4 100644 --- a/tests/test_pagure_flask_api_pr_flag.py +++ b/tests/test_pagure_flask_api_pr_flag.py @@ -58,7 +58,7 @@ class PagureFlaskApiPRFlagtests(tests.Modeltests): self.assertEqual(req.title, 'test pull-request') # Check flags before - # self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 0) @@ -159,7 +159,7 @@ class PagureFlaskApiPRFlagtests(tests.Modeltests): ) # No change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 0) @@ -215,7 +215,7 @@ class PagureFlaskApiPRFlagtests(tests.Modeltests): ) # One flag added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 1) @@ -276,7 +276,7 @@ class PagureFlaskApiPRFlagtests(tests.Modeltests): ) # One flag added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 1) @@ -321,7 +321,7 @@ class PagureFlaskApiPRFlagtests(tests.Modeltests): ) # One flag added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 1) @@ -371,7 +371,7 @@ class PagureFlaskApiPRFlagtests(tests.Modeltests): ) # One flag added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 1) @@ -417,7 +417,7 @@ class PagureFlaskApiPRFlagtests(tests.Modeltests): ) # Two flag added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 2) @@ -533,7 +533,7 @@ class PagureFlaskApiPRFlagUserTokentests(tests.Modeltests): self.assertEqual(req.title, 'test pull-request') # Check flags before - # self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 0) @@ -620,7 +620,7 @@ class PagureFlaskApiPRFlagUserTokentests(tests.Modeltests): ) # No change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 0) @@ -653,7 +653,7 @@ class PagureFlaskApiPRFlagUserTokentests(tests.Modeltests): ) # No change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 0) @@ -704,7 +704,7 @@ class PagureFlaskApiPRFlagUserTokentests(tests.Modeltests): ) # One flag added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 1) @@ -757,7 +757,7 @@ class PagureFlaskApiPRFlagUserTokentests(tests.Modeltests): ) # One flag added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 1) @@ -803,7 +803,7 @@ class PagureFlaskApiPRFlagUserTokentests(tests.Modeltests): ) # Still only one flag - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 1) diff --git a/tests/test_pagure_flask_api_ui_private_repo.py b/tests/test_pagure_flask_api_ui_private_repo.py index 1530504..2d4f200 100644 --- a/tests/test_pagure_flask_api_ui_private_repo.py +++ b/tests/test_pagure_flask_api_ui_private_repo.py @@ -619,7 +619,7 @@ class PagurePrivateRepotest(tests.Modeltests): '', output.get_data(as_text=True)) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test4') self.assertTrue(repo.private) @@ -640,7 +640,7 @@ class PagurePrivateRepotest(tests.Modeltests): '', output.get_data(as_text=True)) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test4') self.assertFalse(repo.private) @@ -675,7 +675,7 @@ class PagurePrivateRepotest(tests.Modeltests): '', output.get_data(as_text=True)) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test4') self.assertFalse(repo.private) @@ -697,7 +697,7 @@ class PagurePrivateRepotest(tests.Modeltests): output.get_data(as_text=True)) # No change since we can't do public -> private - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test4') self.assertFalse(repo.private) @@ -1530,7 +1530,7 @@ class PagurePrivateRepotest(tests.Modeltests): self.assertEqual(req.title, 'test pull-request') # Check comments before - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 0) @@ -1554,7 +1554,7 @@ class PagurePrivateRepotest(tests.Modeltests): ) # No change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 0) @@ -1574,7 +1574,7 @@ class PagurePrivateRepotest(tests.Modeltests): ) # One comment added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 1) @@ -1667,7 +1667,7 @@ class PagurePrivateRepotest(tests.Modeltests): self.assertEqual(req.title, 'test pull-request') # Check comments before - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 0) @@ -1694,7 +1694,7 @@ class PagurePrivateRepotest(tests.Modeltests): ) # No change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 0) @@ -1736,7 +1736,7 @@ class PagurePrivateRepotest(tests.Modeltests): ) # One flag added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 1) @@ -1780,7 +1780,7 @@ class PagurePrivateRepotest(tests.Modeltests): ) # One flag added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.flags), 1) @@ -2893,7 +2893,7 @@ class PagurePrivateRepotest(tests.Modeltests): self.assertEqual(msg.title, 'Test issue #1') # Check comments before - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test4') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.comments), 0) @@ -2917,7 +2917,7 @@ class PagurePrivateRepotest(tests.Modeltests): ) # No change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test4') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(issue.status, 'Open') @@ -2937,7 +2937,7 @@ class PagurePrivateRepotest(tests.Modeltests): ) # One comment added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test4') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.comments), 1) diff --git a/tests/test_pagure_flask_api_user.py b/tests/test_pagure_flask_api_user.py index 3208523..d7b99fd 100644 --- a/tests/test_pagure_flask_api_user.py +++ b/tests/test_pagure_flask_api_user.py @@ -288,7 +288,7 @@ class PagureFlaskApiUSertests(tests.Modeltests): self.assertEqual(req.title, 'test pull-request') # Check comments before - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 0) @@ -308,7 +308,7 @@ class PagureFlaskApiUSertests(tests.Modeltests): ) # One comment added - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(len(request.comments), 1) @@ -324,7 +324,7 @@ class PagureFlaskApiUSertests(tests.Modeltests): ) # PR closed - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() request = pagure.lib.search_pull_requests( self.session, project_id=1, requestid=1) self.assertEqual(request.status, 'Closed') diff --git a/tests/test_pagure_flask_internal.py b/tests/test_pagure_flask_internal.py index b13b250..c654989 100644 --- a/tests/test_pagure_flask_internal.py +++ b/tests/test_pagure_flask_internal.py @@ -116,7 +116,7 @@ class PagureFlaskInternaltests(tests.Modeltests): js_data = json.loads(output.data) self.assertDictEqual(js_data, {'message': 'Comment added'}) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') request = repo.requests[0] self.assertEqual(len(request.comments), 1) @@ -196,7 +196,7 @@ class PagureFlaskInternaltests(tests.Modeltests): js_data = json.loads(output.data) self.assertDictEqual(js_data, {'message': 'Comment added'}) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') issue = repo.issues[0] self.assertEqual(len(issue.comments), 1) @@ -286,7 +286,7 @@ class PagureFlaskInternaltests(tests.Modeltests): js_data = json.loads(output.data) self.assertDictEqual(js_data, {'message': 'Comment added'}) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') issue = repo.issues[0] self.assertEqual(len(issue.comments), 1) diff --git a/tests/test_pagure_flask_ui_app_give_project.py b/tests/test_pagure_flask_ui_app_give_project.py index 0fbe8dd..3e71b22 100644 --- a/tests/test_pagure_flask_ui_app_give_project.py +++ b/tests/test_pagure_flask_ui_app_give_project.py @@ -43,7 +43,7 @@ class PagureFlaskGiveRepotests(tests.SimplePagureTest): tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) def _check_user(self, user='pingou'): - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() project = pagure.lib.get_authorized_project( self.session, project_name='test') self.assertEqual(project.user.user, user) diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index 40665b3..5a02e97 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -337,7 +337,7 @@ class PagureFlaskForktests(tests.Modeltests): self.assertEqual(output.status_code, 404) # Project w/o pull-request - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') settings = repo.settings settings['pull_requests'] = False @@ -351,7 +351,7 @@ class PagureFlaskForktests(tests.Modeltests): self.assertEqual(output.status_code, 404) # Project w pull-request but only assignee can merge - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') settings['pull_requests'] = True settings['Only_assignee_can_merge_pull-request'] = True @@ -374,7 +374,7 @@ class PagureFlaskForktests(tests.Modeltests): 'assigned to be merged', output.data) # PR assigned but not to this user - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') req = repo.requests[0] req.assignee_id = 2 @@ -393,7 +393,7 @@ class PagureFlaskForktests(tests.Modeltests): 'merge this review', output.data) # Project w/ minimal PR score - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') settings['Only_assignee_can_merge_pull-request'] = False settings['Minimum_score_to_merge_pull-request'] = 2 @@ -414,7 +414,7 @@ class PagureFlaskForktests(tests.Modeltests): output.data) # Merge - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') settings['Minimum_score_to_merge_pull-request'] = -1 repo.settings = settings @@ -675,7 +675,7 @@ class PagureFlaskForktests(tests.Modeltests): ' PR from the feature branch\n', output.data) self.assertTrue( output.data.count('\n Comment updated', output.data) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.comments), 1) @@ -3073,7 +3073,7 @@ class PagureFlaskIssuestests(tests.Modeltests): '\n Comment updated', output.data) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.comments), 1) @@ -3158,7 +3158,7 @@ class PagureFlaskIssuestests(tests.Modeltests): ) # Ticket #1 has one more comment and is still open - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.comments), 2) self.assertEqual(issue.status, 'Open') diff --git a/tests/test_pagure_flask_ui_login.py b/tests/test_pagure_flask_ui_login.py index f43d01d..50bfe55 100644 --- a/tests/test_pagure_flask_ui_login.py +++ b/tests/test_pagure_flask_ui_login.py @@ -200,7 +200,7 @@ class PagureFlaskLogintests(tests.SimplePagureTest): 'provided by email?', output.data) # Confirm the user so that we can log in - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() item = pagure.lib.search_user(self.session, username='foouser') self.assertEqual(item.user, 'foouser') self.assertNotEqual(item.token, None) @@ -228,7 +228,7 @@ class PagureFlaskLogintests(tests.SimplePagureTest): # partly fail if hasattr(flask, '__version__'): flask_v = tuple(int(el) for el in flask.__version__.split('.')) - if flask_v <= (0, 12, 0): + if flask_v < (0, 12, 0): self.assertIn( '', output.data) @@ -241,7 +241,7 @@ class PagureFlaskLogintests(tests.SimplePagureTest): 'href="/logout/?next=http://localhost/">', output.data) # Make the password invalid - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() item = pagure.lib.search_user(self.session, username='foouser') self.assertEqual(item.user, 'foouser') self.assertTrue(item.password.startswith('$2$')) @@ -252,7 +252,7 @@ class PagureFlaskLogintests(tests.SimplePagureTest): self.session.commit() # Check the password - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() item = pagure.lib.search_user(self.session, username='foouser') self.assertEqual(item.user, 'foouser') self.assertFalse(item.password.startswith('$2$')) @@ -268,7 +268,7 @@ class PagureFlaskLogintests(tests.SimplePagureTest): self.assertIn('Username or password of invalid format.', output.data) # Check the password is still not of a known version - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() item = pagure.lib.search_user(self.session, username='foouser') self.assertEqual(item.user, 'foouser') self.assertFalse(item.password.startswith('$1$')) @@ -283,7 +283,7 @@ class PagureFlaskLogintests(tests.SimplePagureTest): self.session.commit() # Check the password - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() item = pagure.lib.search_user(self.session, username='foouser') self.assertEqual(item.user, 'foouser') self.assertTrue(item.password.startswith('$1$')) @@ -297,7 +297,7 @@ class PagureFlaskLogintests(tests.SimplePagureTest): self.assertIn('Activity', output.data) # Check the password got upgraded to version 2 - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() item = pagure.lib.search_user(self.session, username='foouser') self.assertEqual(item.user, 'foouser') self.assertTrue(item.password.startswith('$2$')) diff --git a/tests/test_pagure_flask_ui_priorities.py b/tests/test_pagure_flask_ui_priorities.py index d8caff0..8483ec5 100644 --- a/tests/test_pagure_flask_ui_priorities.py +++ b/tests/test_pagure_flask_ui_priorities.py @@ -181,7 +181,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): 'Settings - test - Pagure', output.data) self.assertIn('

Settings for test

', output.data) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(repo.priorities, {u'': u'', u'1': u'High'}) @@ -203,7 +203,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertTrue( output.data.find('Normal') < output.data.find('Low')) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -228,7 +228,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): ' Priorities weights and titles are ' 'not of the same length', output.data) # Check the result of the action -- Priorities un-changed - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -253,7 +253,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): ' Priorities weights must be numbers', output.data) # Check the result of the action -- Priorities un-changed - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -278,7 +278,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): ' Priority weight 2 is present 2 times', output.data) # Check the result of the action -- Priorities un-changed - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -303,7 +303,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): ' Priority Normal is present 2 times', output.data) # Check the result of the action -- Priorities un-changed - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -381,7 +381,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertIn('

Settings for test

', output.data) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -436,7 +436,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertNotIn('', output.data) # Check the result of the action -- Priority reset - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test') self.assertEqual(repo.priorities, {}) @@ -482,7 +482,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertIn('

Settings for test

', output.data) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -538,7 +538,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertNotIn('', output.data) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test') self.assertEqual(repo.priorities, {}) @@ -583,7 +583,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertIn('

Settings for test

', output.data) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -707,7 +707,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertIn('

Settings for test

', output.data) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -810,7 +810,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertIn('

Settings for test

', output.data) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -908,7 +908,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertTrue( output.data.find('Normal') < output.data.find('Low')) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -926,7 +926,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): 'Settings - test - Pagure', output.data) self.assertIn('

Settings for test

', output.data) # Check the result of the action -- default_priority no change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(repo.default_priority, None) @@ -944,7 +944,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): '\n Default priority set ' 'to High', output.data) # Check the result of the action -- default_priority no change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(repo.default_priority, 'High') @@ -959,7 +959,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): 'Settings - test - Pagure', output.data) self.assertIn('

Settings for test

', output.data) # Check the result of the action -- default_priority no change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(repo.default_priority, 'High') @@ -977,7 +977,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): '\n Default priority reset', output.data) # Check the result of the action -- default_priority no change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(repo.default_priority, None) @@ -1049,7 +1049,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertTrue( output.data.find('Normal') < output.data.find('Low')) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.priorities, @@ -1070,7 +1070,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): '\n Default priority set ' 'to High', output.data) # Check the result of the action -- default_priority no change - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(repo.default_priority, 'High') @@ -1098,7 +1098,7 @@ class PagureFlaskPrioritiestests(tests.Modeltests): self.assertTrue( output.data.find('Normal') < output.data.find('Low')) # Check the result of the action -- Priority recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.priorities, diff --git a/tests/test_pagure_flask_ui_quick_reply.py b/tests/test_pagure_flask_ui_quick_reply.py index 8260fd3..7cc1e1a 100644 --- a/tests/test_pagure_flask_ui_quick_reply.py +++ b/tests/test_pagure_flask_ui_quick_reply.py @@ -77,7 +77,7 @@ class PagureFlaskQuickReplytest(tests.Modeltests): self.assertIn(notice, output.data) def assertQuickReplies(self, quick_replies, project='test'): - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, project) self.assertEqual(repo.quick_replies, quick_replies) diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 0cf7737..efaac6d 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -716,7 +716,7 @@ class PagureFlaskRepotests(tests.Modeltests): '\n User removed', output.data) self.assertNotIn('action="/test/dropuser/2">', output.data) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(len(repo.users), 0) @@ -761,7 +761,7 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertIn( u'\n User removed', output.data) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(len(repo.users), 0) @@ -937,7 +937,7 @@ class PagureFlaskRepotests(tests.Modeltests): output.data) self.assertNotIn('action="/test/dropgroup/1">', output.data) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(len(repo.groups), 0) @@ -3720,7 +3720,7 @@ index 0000000..fb7093d output.data) pagure.config.config['WEBHOOK'] = False - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertNotEqual(repo.hook_token, 'aaabbbccc') @@ -4423,7 +4423,7 @@ index 0000000..fb7093d output.data) # Existing token has been expired - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.tokens[0].expiration.date(), @@ -4824,7 +4824,7 @@ index 0000000..fb7093d self.assertIn( '\n List of reports updated', output.data) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() project = pagure.lib.get_authorized_project(self.session, project_name='test') self.assertEqual(project.reports, {}) @@ -4884,7 +4884,7 @@ index 0000000..fb7093d output.data) # Create a report - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() project = pagure.lib.get_authorized_project( self.session, project_name='test', namespace='foo') self.assertEqual(project.reports, {}) @@ -4935,7 +4935,7 @@ index 0000000..fb7093d '\n List of reports updated', output.data) - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() project = pagure.lib.get_authorized_project( self.session, project_name='test', namespace='foo') self.assertEqual(project.reports, {}) diff --git a/tests/test_pagure_flask_ui_roadmap.py b/tests/test_pagure_flask_ui_roadmap.py index d7e9cb1..798503b 100644 --- a/tests/test_pagure_flask_ui_roadmap.py +++ b/tests/test_pagure_flask_ui_roadmap.py @@ -194,7 +194,7 @@ class PagureFlaskRoadmaptests(tests.Modeltests): self.assertIn(u'

Settings for test

', output.data) self.assertIn(u'Milestones updated', output.data) # Check the result of the action -- Milestones recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(repo.milestones, {u'1': {u'active': False, u'date': None}}) @@ -213,7 +213,7 @@ class PagureFlaskRoadmaptests(tests.Modeltests): self.assertIn(u'

Settings for test

', output.data) self.assertIn(u'Milestones updated', output.data) # Check the result of the action -- Milestones recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.milestones, @@ -239,7 +239,7 @@ class PagureFlaskRoadmaptests(tests.Modeltests): u'Settings - test - Pagure', output.data) self.assertIn(u'

Settings for test

', output.data) # Check the result of the action -- Milestones un-changed - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.milestones, @@ -269,7 +269,7 @@ class PagureFlaskRoadmaptests(tests.Modeltests): ' Milestone v2.0 is present 2 times', output.data) # Check the result of the action -- Milestones un-changed - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.milestones, @@ -299,7 +299,7 @@ class PagureFlaskRoadmaptests(tests.Modeltests): ' Milestones updated', output.data) # Check the result of the action -- Milestones updated - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.milestones, @@ -368,7 +368,7 @@ class PagureFlaskRoadmaptests(tests.Modeltests): self.assertIn(u'

Settings for test

', output.data) self.assertIn(u'Milestones updated', output.data) # Check the result of the action -- Milestones recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual( repo.milestones, @@ -421,7 +421,7 @@ class PagureFlaskRoadmaptests(tests.Modeltests): self.assertIn(u'

Settings for test

', output.data) self.assertIn(u'Milestones updated', output.data) # Check the result of the action -- Milestones recorded - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib._get_project(self.session, 'test') self.assertEqual( repo.milestones, diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 6f5e53b..a396c06 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -1638,7 +1638,7 @@ class PagureLibtests(tests.Modeltests): ) # Now test that creation fails if ignore_existing_repo is False - self.session = pagure.lib.create_session(self.dbpath) + self.session.commit() repo = pagure.lib.get_authorized_project(self.session, 'testproject') self.assertEqual(repo.path, 'testproject.git') @@ -1697,7 +1697,6 @@ class PagureLibtests(tests.Modeltests): repo = pagure.lib._get_project(self.session, 'testproject') self.session.delete(repo) self.session.commit() - self.session = pagure.lib.create_session(self.dbpath) task = pagure.lib.new_project( session=self.session, From 24a83796ef1938172b1c52c7fada9d4350c09aef Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mar 23 2018 16:46:14 +0000 Subject: [PATCH 5/9] Tests: factor a patch call --- diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index ca357a1..76b8ef9 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -33,6 +33,19 @@ from pagure.lib.repo import PagureRepo class PagureFlaskApiProjecttests(tests.Modeltests): """ Tests for the flask API of pagure for issue """ + def setUp(self): + super(PagureFlaskApiProjecttests, self).setUp() + self.gga_patcher = patch( + 'pagure.lib.tasks.generate_gitolite_acls.delay') + self.mock_gen_acls = self.gga_patcher.start() + task_result = Mock() + task_result.id = 'abc-1234' + self.mock_gen_acls.return_value = task_result + + def tearDown(self): + self.gga_patcher.stop() + super(PagureFlaskApiProjecttests, self).tearDown() + def test_api_git_tags(self): """ Test the api_git_tags method of the flask api. """ tests.create_projects(self.session) @@ -2004,10 +2017,8 @@ class PagureFlaskApiProjecttests(tests.Modeltests): } self.assertDictEqual(json.loads(output.data), expected_data) - @patch('pagure.lib.git.generate_gitolite_acls') - def test_api_new_project(self, p_gga): + def test_api_new_project(self): """ Test the api_new_project method of the flask api. """ - p_gga.return_value = True tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'tickets')) @@ -2096,11 +2107,9 @@ class PagureFlaskApiProjecttests(tests.Modeltests): ) @patch.dict('pagure.config.config', {'PRIVATE_PROJECTS': True}) - @patch('pagure.lib.git.generate_gitolite_acls') - def test_api_new_project_private(self, p_gga): + def test_api_new_project_private(self): """ Test the api_new_project method of the flask api to create a private project. """ - p_gga.return_value = True tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'tickets')) @@ -2125,11 +2134,8 @@ class PagureFlaskApiProjecttests(tests.Modeltests): {'message': 'Project "pingou/test" created'} ) - @patch('pagure.lib.git.generate_gitolite_acls') - def test_api_new_project_user_token(self, p_gga): + def test_api_new_project_user_token(self): """ Test the api_new_project method of the flask api. """ - p_gga.return_value = True - tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'tickets')) tests.create_tokens(self.session, project_id=None) @@ -2258,12 +2264,9 @@ class PagureFlaskApiProjecttests(tests.Modeltests): {'message': 'Project "rpms/test_42" created'} ) - @patch('pagure.lib.git.generate_gitolite_acls') - def test_api_new_project_user_ns(self, p_gga): + @patch.dict('pagure.config.config', {'USER_NAMESPACE': True}) + def test_api_new_project_user_ns(self): """ Test the api_new_project method of the flask api. """ - pagure.config.config['USER_NAMESPACE'] = True - p_gga.return_value = True - tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'tickets')) tests.create_tokens(self.session) @@ -2305,13 +2308,8 @@ class PagureFlaskApiProjecttests(tests.Modeltests): {'message': 'Project "testns/testproject2" created'} ) - pagure.config.config['USER_NAMESPACE'] = False - - @patch('pagure.lib.git.generate_gitolite_acls') - def test_api_fork_project(self, p_gga): + def test_api_fork_project(self): """ Test the api_fork_project method of the flask api. """ - p_gga.return_value = True - tests.create_projects(self.session) for folder in ['docs', 'tickets', 'requests', 'repos']: tests.create_projects_git( @@ -2433,11 +2431,8 @@ class PagureFlaskApiProjecttests(tests.Modeltests): } ) - @patch('pagure.lib.git.generate_gitolite_acls') - def test_api_fork_project_user_token(self, p_gga): + def test_api_fork_project_user_token(self): """ Test the api_fork_project method of the flask api. """ - p_gga.return_value = True - tests.create_projects(self.session) for folder in ['docs', 'tickets', 'requests', 'repos']: tests.create_projects_git( @@ -2559,8 +2554,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): } ) - @patch('pagure.lib.tasks.generate_gitolite_acls.delay') - def test_api_generate_acls(self, mock_gen_acls): + def test_api_generate_acls(self): """ Test the api_generate_acls method of the flask api """ tests.create_projects(self.session) tests.create_tokens(self.session, project_id=None) @@ -2568,10 +2562,6 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.session, 'aaabbbcccddd', 'generate_acls_project') headers = {'Authorization': 'token aaabbbcccddd'} - mock_gen_acls_rv = Mock() - mock_gen_acls_rv.id = 'abc-1234' - mock_gen_acls.return_value = mock_gen_acls_rv - user = pagure.lib.get_user(self.session, 'pingou') with tests.user_set(self.app.application, user): output = self.app.post( @@ -2584,11 +2574,10 @@ class PagureFlaskApiProjecttests(tests.Modeltests): 'taskid': 'abc-1234' } self.assertEqual(data, expected_output) - mock_gen_acls.assert_called_once_with( + self.mock_gen_acls.assert_called_once_with( name='test', namespace=None, user=None, group=None) - @patch('pagure.lib.tasks.generate_gitolite_acls.delay') - def test_api_generate_acls_json(self, mock_gen_acls): + def test_api_generate_acls_json(self): """ Test the api_generate_acls method of the flask api using JSON """ tests.create_projects(self.session) tests.create_tokens(self.session, project_id=None) @@ -2597,10 +2586,6 @@ class PagureFlaskApiProjecttests(tests.Modeltests): headers = {'Authorization': 'token aaabbbcccddd', 'Content-Type': 'application/json'} - mock_gen_acls_rv = Mock() - mock_gen_acls_rv.id = 'abc-1234' - mock_gen_acls.return_value = mock_gen_acls_rv - user = pagure.lib.get_user(self.session, 'pingou') with tests.user_set(self.app.application, user): output = self.app.post( @@ -2613,11 +2598,10 @@ class PagureFlaskApiProjecttests(tests.Modeltests): 'taskid': 'abc-1234' } self.assertEqual(data, expected_output) - mock_gen_acls.assert_called_once_with( + self.mock_gen_acls.assert_called_once_with( name='test', namespace=None, user=None, group=None) - @patch('pagure.lib.tasks.generate_gitolite_acls.delay') - def test_api_generate_acls_wait_true(self, mock_gen_acls): + def test_api_generate_acls_wait_true(self): """ Test the api_generate_acls method of the flask api when wait is set to True """ tests.create_projects(self.session) @@ -2626,9 +2610,9 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.session, 'aaabbbcccddd', 'generate_acls_project') headers = {'Authorization': 'token aaabbbcccddd'} - mock_gen_acls_rv = Mock() - mock_gen_acls_rv.id = 'abc-1234' - mock_gen_acls.return_value = mock_gen_acls_rv + task_result = Mock() + task_result.id = 'abc-1234' + self.mock_gen_acls.return_value = task_result user = pagure.lib.get_user(self.session, 'pingou') with tests.user_set(self.app.application, user): @@ -2641,7 +2625,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): 'message': 'Project ACLs generated', } self.assertEqual(data, expected_output) - mock_gen_acls.assert_called_once_with( + self.mock_gen_acls.assert_called_once_with( name='test', namespace=None, user=None, group=None) def test_api_generate_acls_no_project(self): From e78d798b08574259566b56ceace5268caa27e0a3 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mar 23 2018 17:14:14 +0000 Subject: [PATCH 6/9] Tests: empty the DB after each tests And let tests choose if they want a populated DB or not. --- diff --git a/tests/__init__.py b/tests/__init__.py index ae78f71..2c10f7a 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -50,7 +50,7 @@ import pagure.flask_app import pagure.lib import pagure.lib.model import pagure.perfrepo as perfrepo -from pagure.config import config as pagure_config +from pagure.config import config as pagure_config, reload_config from pagure.lib.repo import PagureRepo HERE = os.path.join(os.path.dirname(os.path.abspath(__file__))) @@ -63,15 +63,22 @@ PAGLOG.handlers = [] CONFIG_TEMPLATE = """ GIT_FOLDER = '%(path)s/repos' -ENABLE_DOCS = True -ENABLE_TICKETS = True +ENABLE_DOCS = %(enable_docs)s +ENABLE_TICKETS = %(enable_tickets)s REMOTE_GIT_FOLDER = '%(path)s/remotes' -ATTACHMENTS_FOLDER = '%(path)s/attachments' DB_URL = '%(dburl)s' ALLOW_PROJECT_DOWAIT = True DEBUG = True +PAGURE_CI_SERVICES = ['jenkins'] +EMAIL_SEND = False +TESTING = True +GIT_FOLDER = '%(path)s/repos' +REQUESTS_FOLDER = '%(path)s/repos/requests' +TICKETS_FOLDER = %(tickets_folder)r +DOCS_FOLDER = %(docs_folder)r +ATTACHMENTS_FOLDER = '%(path)s/attachments' +BROKER_URL = 'redis+socket://%(global_path)s/broker' """ -MAX_NOFILE = 4096 LOG.info('BUILD_ID: %s', os.environ.get('BUILD_ID')) @@ -129,17 +136,14 @@ def user_set(APP, user): with appcontext_pushed.connected_to(handler, APP): yield -# In order to save time during local test execution, we create sqlite DB file -# only once and then we use a fresh copy of it for every test case (as opposed -# to creating DB file for every test case). -_, dbfile = tempfile.mkstemp() -broker = None +tests_state = { + "path": tempfile.mkdtemp(prefix='pagure-tests-'), + "broker": None, +} -def _create_db_entities(dbpath): - session = pagure.lib.model.create_tables( - dbpath, acls=pagure_config.get('ACLS', {})) +def _populate_db(session): # Create a couple of users item = pagure.lib.model.User( user='pingou', @@ -172,15 +176,22 @@ def _create_db_entities(dbpath): session.commit() -def setUp(): - dbpath = 'sqlite:///%s' % dbfile - _create_db_entities(dbpath) + +def setUp(): + # In order to save time during local test execution, we create sqlite DB + # file only once and then we populate it and empty it for every test case + # (as opposed to creating DB file for every test case). + session = pagure.lib.model.create_tables( + 'sqlite:///%s/db.sqlite' % tests_state["path"], + acls=pagure_config.get('ACLS', {}), + ) + tests_state["db_session"] = session # Create a broker - global broker - _, broker_url = tempfile.mkstemp() - broker = subprocess.Popen( + broker_url = os.path.join(tests_state["path"], 'broker') + + tests_state["broker"] = broker = subprocess.Popen( ['/usr/bin/redis-server', '--unixsocket', broker_url, '--port', '0', '--loglevel', 'warning', '--logfile', '/dev/null'], stdout=None, stderr=None) @@ -188,17 +199,13 @@ def setUp(): if broker.returncode is not None: raise Exception('Broker failed to start') - celery_broker_url = 'redis+socket://' + broker_url - pagure_config['BROKER_URL'] = celery_broker_url - reload(pagure.lib.tasks) - reload(pagure.lib.tasks_services) - def tearDown(): - os.unlink(dbfile) - + tests_state["db_session"].close() + broker = tests_state["broker"] broker.kill() broker.wait() + shutil.rmtree(tests_state["path"]) class SimplePagureTest(unittest.TestCase): @@ -206,6 +213,9 @@ class SimplePagureTest(unittest.TestCase): Simple Test class that does not set a broker/worker """ + populate_db = True + config_values = {} + @mock.patch('pagure.lib.notify.fedmsg_publish', mock.MagicMock()) def __init__(self, method_name='runTest'): """ Constructor. """ @@ -235,11 +245,6 @@ class SimplePagureTest(unittest.TestCase): perfrepo.reset_stats() perfrepo.REQUESTS = [] - def _set_db_path(self): - self.dbpath = 'sqlite:///%s' % os.path.join( - self.path, 'db.sqlite') - shutil.copyfile(dbfile, self.dbpath[len('sqlite://'):]) - def setUp(self): self.perfReset() @@ -257,36 +262,38 @@ class SimplePagureTest(unittest.TestCase): pagure.lib.REDIS.connection_pool.disconnect() pagure.lib.REDIS = None - self._set_db_path() + # Database + self._prepare_db() # Write a config file - config_values = {'path': self.path, 'dburl': self.dbpath} + config_values = { + 'path': self.path, 'dburl': self.dbpath, + 'enable_docs': True, + 'docs_folder': '%s/repos/docs' % self.path, + 'enable_tickets': True, + 'tickets_folder': '%s/repos/tickets' % self.path, + 'global_path': tests_state["path"], + } + config_values.update(self.config_values) config_path = os.path.join(self.path, 'config') if not os.path.exists(config_path): with open(config_path, 'w') as f: f.write(CONFIG_TEMPLATE % config_values) + os.environ["PAGURE_CONFIG"] = config_path + pagure_config.update(reload_config()) - # Prevent unit-tests to send email, globally - pagure_config['EMAIL_SEND'] = False - pagure_config['TESTING'] = True - pagure_config['GIT_FOLDER'] = gf = os.path.join( - self.path, 'repos') - pagure_config['REQUESTS_FOLDER'] = os.path.join( - gf, 'requests') - pagure.config.config['TICKETS_FOLDER'] = os.path.join( - gf, 'tickets') - pagure_config['ATTACHMENTS_FOLDER'] = os.path.join( - self.path, 'attachments') + reload(pagure.lib.tasks) + reload(pagure.lib.tasks_services) self._app = pagure.flask_app.create_app({'DB_URL': self.dbpath}) # Remove the log handlers for the tests self._app.logger.handlers = [] self.app = self._app.test_client() - self.session = pagure.lib.create_session(self.dbpath) def tearDown(self): - self.session.close() + self.session.rollback() + self._clear_database() # Remove testdir try: @@ -305,6 +312,30 @@ class SimplePagureTest(unittest.TestCase): doc = self.__str__() +": "+self._testMethodDoc return doc or None + def _prepare_db(self): + self.dbpath = 'sqlite:///%s' % os.path.join( + tests_state["path"], 'db.sqlite') + self.session = tests_state["db_session"] + pagure.lib.model.create_default_status( + self.session, acls=pagure_config.get('ACLS', {})) + if self.populate_db: + _populate_db(self.session) + + def _clear_database(self): + tables = reversed(pagure.lib.model.BASE.metadata.sorted_tables) + if self.dbpath.startswith('postgresql'): + self.session.execute("TRUNCATE %s CASCADE" % ", ".join( + [t.name for t in tables])) + elif self.dbpath.startswith('sqlite'): + for table in tables: + self.session.execute("DELETE FROM %s" % table.name) + elif self.dbpath.startswith('mysql'): + self.session.execute("SET FOREIGN_KEY_CHECKS = 0") + for table in tables: + self.session.execute("TRUNCATE %s" % table.name) + self.session.execute("SET FOREIGN_KEY_CHECKS = 1") + self.session.commit() + def get_csrf(self, url='/new', output=None): """Retrieve a CSRF token from given URL.""" if output is None: @@ -345,8 +376,7 @@ class Modeltests(SimplePagureTest): 'PYTHONPATH': '.' }) celery_cwd = os.path.normpath( - os.path.join(os.path.dirname(__file__), - '..') + os.path.join(os.path.dirname(__file__), '..') ) self.worker = subprocess.Popen( [celery_exec, '-A', 'pagure.lib.tasks', 'worker', @@ -386,8 +416,7 @@ class Modeltests(SimplePagureTest): def tearDown(self): # pylint: disable=invalid-name """ Remove the test.db database if there is one. """ - super(Modeltests, self).tearDown() - # Terminate worker and broker + # Terminate worker # We just send a SIGKILL (kill -9), since when the test finishes, we # don't really care about the output of either worker or broker # anymore @@ -400,6 +429,7 @@ class Modeltests(SimplePagureTest): ['/usr/bin/redis-cli', '-s', pagure_config['BROKER_URL'][len('redis+socket://'):], 'flushall'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + super(Modeltests, self).tearDown() class FakeGroup(object): # pylint: disable=too-few-public-methods diff --git a/tests/test_pagure_admin.py b/tests/test_pagure_admin.py index 8fbb283..7e1461b 100644 --- a/tests/test_pagure_admin.py +++ b/tests/test_pagure_admin.py @@ -334,26 +334,13 @@ optional arguments: class PagureAdminAdminTokenEmptytests(tests.Modeltests): """ Tests for pagure-admin admin-token when there is nothing in the DB """ + + populate_db = False + def setUp(self): """ Set up the environnment, ran before every tests. """ super(PagureAdminAdminTokenEmptytests, self).setUp() - - self.configfile = os.path.join(self.path, 'config') - self.dbpath = "sqlite:///%s/pagure_dev.sqlite" % self.path - with open(self.configfile, 'w') as stream: - stream.write('DB_URL="%s"\n' % self.dbpath) - - os.environ['PAGURE_CONFIG'] = self.configfile - - createdb = os.path.abspath( - os.path.join(tests.HERE, '..', 'createdb.py')) - cmd = ['python', createdb] - _get_ouput(cmd) - - def tearDown(self): - """ Tear down the environnment after running the tests. """ - super(PagureAdminAdminTokenEmptytests, self).tearDown() - del(os.environ['PAGURE_CONFIG']) + pagure.cli.admin.session = self.session def test_do_create_admin_token_no_user(self): """ Test the do_create_admin_token function of pagure-admin without @@ -373,24 +360,12 @@ class PagureAdminAdminTokenEmptytests(tests.Modeltests): class PagureAdminAdminRefreshGitolitetests(tests.Modeltests): """ Tests for pagure-admin refresh-gitolite """ + populate_db = False + def setUp(self): """ Set up the environnment, ran before every tests. """ super(PagureAdminAdminRefreshGitolitetests, self).setUp() - - self.configfile = os.path.join(self.path, 'config') - self.dbpath = "sqlite:///%s/pagure_dev.sqlite" % self.path - with open(self.configfile, 'w') as stream: - stream.write('DB_URL="%s"\n' % self.dbpath) - - os.environ['PAGURE_CONFIG'] = self.configfile - - createdb = os.path.abspath( - os.path.join(tests.HERE, '..', 'createdb.py')) - cmd = ['python', createdb] - _get_ouput(cmd) - - self.session = pagure.lib.model.create_tables( - self.dbpath, acls=pagure.config.config.get('ACLS', {})) + pagure.cli.admin.session = self.session # Create the user pingou item = pagure.lib.model.User( @@ -426,11 +401,6 @@ class PagureAdminAdminRefreshGitolitetests(tests.Modeltests): # Make the imported pagure use the correct db session pagure.cli.admin.session = self.session - def tearDown(self): - """ Tear down the environnment after running the tests. """ - super(PagureAdminAdminRefreshGitolitetests, self).tearDown() - del(os.environ['PAGURE_CONFIG']) - @patch('pagure.cli.admin._ask_confirmation') @patch('pagure.lib.git_auth.get_git_auth_helper') def test_do_refresh_gitolite_no_args(self, get_helper, conf): @@ -521,24 +491,12 @@ class PagureAdminAdminRefreshGitolitetests(tests.Modeltests): class PagureAdminAdminTokentests(tests.Modeltests): """ Tests for pagure-admin admin-token """ + populate_db = False + def setUp(self): """ Set up the environnment, ran before every tests. """ super(PagureAdminAdminTokentests, self).setUp() - - self.configfile = os.path.join(self.path, 'config') - self.dbpath = "sqlite:///%s/pagure_dev.sqlite" % self.path - with open(self.configfile, 'w') as stream: - stream.write('DB_URL="%s"\n' % self.dbpath) - - os.environ['PAGURE_CONFIG'] = self.configfile - - createdb = os.path.abspath( - os.path.join(tests.HERE, '..', 'createdb.py')) - cmd = ['python', createdb] - _get_ouput(cmd) - - self.session = pagure.lib.model.create_tables( - self.dbpath, acls=pagure.config.config.get('ACLS', {})) + pagure.cli.admin.session = self.session # Create the user pingou item = pagure.lib.model.User( @@ -557,11 +515,6 @@ class PagureAdminAdminTokentests(tests.Modeltests): # Make the imported pagure use the correct db session pagure.cli.admin.session = self.session - def tearDown(self): - """ Tear down the environnment after running the tests. """ - super(PagureAdminAdminTokentests, self).tearDown() - del(os.environ['PAGURE_CONFIG']) - @patch('pagure.cli.admin._get_input') @patch('pagure.cli.admin._ask_confirmation') def test_do_create_admin_token(self, conf, rinp): @@ -827,24 +780,12 @@ class PagureAdminAdminTokentests(tests.Modeltests): class PagureAdminGetWatchTests(tests.Modeltests): """ Tests for pagure-admin get-watch """ + populate_db = False + def setUp(self): """ Set up the environnment, ran before every tests. """ super(PagureAdminGetWatchTests, self).setUp() - - self.configfile = os.path.join(self.path, 'config') - self.dbpath = "sqlite:///%s/pagure_dev.sqlite" % self.path - with open(self.configfile, 'w') as stream: - stream.write('DB_URL="%s"\n' % self.dbpath) - - os.environ['PAGURE_CONFIG'] = self.configfile - - createdb = os.path.abspath( - os.path.join(tests.HERE, '..', 'createdb.py')) - cmd = ['python', createdb] - _get_ouput(cmd) - - self.session = pagure.lib.model.create_tables( - self.dbpath, acls=pagure.config.config.get('ACLS', {})) + pagure.cli.admin.session = self.session # Create the user pingou item = pagure.lib.model.User( @@ -892,11 +833,6 @@ class PagureAdminGetWatchTests(tests.Modeltests): # Make the imported pagure use the correct db session pagure.cli.admin.session = self.session - def tearDown(self): - """ Tear down the environnment after running the tests. """ - super(PagureAdminGetWatchTests, self).tearDown() - del(os.environ['PAGURE_CONFIG']) - def test_get_watch_get_project_unknown_project(self): """ Test the get-watch function of pagure-admin with an unknown project. @@ -974,24 +910,12 @@ class PagureAdminGetWatchTests(tests.Modeltests): class PagureAdminUpdateWatchTests(tests.Modeltests): """ Tests for pagure-admin update-watch """ + populate_db = False + def setUp(self): """ Set up the environnment, ran before every tests. """ super(PagureAdminUpdateWatchTests, self).setUp() - - self.configfile = os.path.join(self.path, 'config') - self.dbpath = "sqlite:///%s/pagure_dev.sqlite" % self.path - with open(self.configfile, 'w') as stream: - stream.write('DB_URL="%s"\n' % self.dbpath) - - os.environ['PAGURE_CONFIG'] = self.configfile - - createdb = os.path.abspath( - os.path.join(tests.HERE, '..', 'createdb.py')) - cmd = ['python', createdb] - _get_ouput(cmd) - - self.session = pagure.lib.model.create_tables( - self.dbpath, acls=pagure.config.config.get('ACLS', {})) + pagure.cli.admin.session = self.session # Create the user pingou item = pagure.lib.model.User( @@ -1039,11 +963,6 @@ class PagureAdminUpdateWatchTests(tests.Modeltests): # Make the imported pagure use the correct db session pagure.cli.admin.session = self.session - def tearDown(self): - """ Tear down the environnment after running the tests. """ - super(PagureAdminUpdateWatchTests, self).tearDown() - del(os.environ['PAGURE_CONFIG']) - def test_get_watch_update_project_unknown_project(self): """ Test the update-watch function of pagure-admin on an unknown project. @@ -1109,24 +1028,12 @@ class PagureAdminUpdateWatchTests(tests.Modeltests): class PagureAdminReadOnlyTests(tests.Modeltests): """ Tests for pagure-admin read-only """ + populate_db = False + def setUp(self): """ Set up the environnment, ran before every tests. """ super(PagureAdminReadOnlyTests, self).setUp() - - self.configfile = os.path.join(self.path, 'config') - self.dbpath = "sqlite:///%s/pagure_dev.sqlite" % self.path - with open(self.configfile, 'w') as stream: - stream.write('DB_URL="%s"\n' % self.dbpath) - - os.environ['PAGURE_CONFIG'] = self.configfile - - createdb = os.path.abspath( - os.path.join(tests.HERE, '..', 'createdb.py')) - cmd = ['python', createdb] - _get_ouput(cmd) - - self.session = pagure.lib.model.create_tables( - self.dbpath, acls=pagure.config.config.get('ACLS', {})) + pagure.cli.admin.session = self.session # Create the user pingou item = pagure.lib.model.User( @@ -1165,11 +1072,6 @@ class PagureAdminReadOnlyTests(tests.Modeltests): # Make the imported pagure use the correct db session pagure.cli.admin.session = self.session - def tearDown(self): - """ Tear down the environnment after running the tests. """ - super(PagureAdminReadOnlyTests, self).tearDown() - del(os.environ['PAGURE_CONFIG']) - def test_read_only_unknown_project(self): """ Test the read-only function of pagure-admin on an unknown project. diff --git a/tests/test_pagure_flask_ui_app.py b/tests/test_pagure_flask_ui_app.py index d825f3c..7bf65db 100644 --- a/tests/test_pagure_flask_ui_app.py +++ b/tests/test_pagure_flask_ui_app.py @@ -1673,29 +1673,11 @@ class PagureFlaskApptests(tests.Modeltests): class PagureFlaskAppNoDocstests(tests.Modeltests): """ Tests for flask app controller of pagure """ - def setUp(self): - pagure.config.config['DOCS_FOLDER'] = None - pagure.config.config['ENABLE_DOCS'] = False + config_values = { + "enable_docs": False, + "docs_folder": None, + } - self.path = tempfile.mkdtemp(prefix='pagure-tests-path-') - - self._set_db_path() - - config_path = os.path.join(self.path, 'config') - config_values = {'path': self.path, 'dburl': self.dbpath} - config_content = tests.CONFIG_TEMPLATE % config_values - config_content += 'DOCS_FOLDER = None\nENABLE_DOCS = False' - - with open(config_path, 'w') as f: - f.write(config_content) - - super(PagureFlaskAppNoDocstests, self).setUp() - - def tearDown(self): - pagure.config.config['ENABLE_DOCS'] = True - super(PagureFlaskAppNoDocstests, self).tearDown() - - @patch.dict('pagure.config.config', {'DOCS_FOLDER': None}) def test_new_project_no_docs_folder(self): """ Test the new_project endpoint with DOCS_FOLDER is None. """ # Before @@ -1746,35 +1728,11 @@ class PagureFlaskAppNoDocstests(tests.Modeltests): class PagureFlaskAppNoTicketstests(tests.Modeltests): """ Tests for flask app controller of pagure """ - def setUp(self): - # Stop the current running workers - tests.tearDown() - - pagure.config.config['TICKETS_FOLDER'] = None - pagure.config.config['ENABLE_TICKETS'] = False - - self.path = tempfile.mkdtemp(prefix='pagure-tests-path-') - tests.setUp() - - self.dbpath = 'sqlite:///%s' % os.path.join(self.path, 'db.sqlite') - - config_path = os.path.join(self.path, 'config') - config_values = {'path': self.path, 'dburl': self.dbpath} - config_content = tests.CONFIG_TEMPLATE % config_values - config_content += 'TICKETS_FOLDER = None\nENABLE_TICKETS = False' - - with open(config_path, 'w') as f: - f.write(config_content) - - super(PagureFlaskAppNoTicketstests, self).setUp() - pagure.config.config['TICKETS_FOLDER'] = None - pagure.config.config['ENABLE_TICKETS'] = False - - def tearDown(self): - pagure.config.config['ENABLE_TICKETS'] = True - super(PagureFlaskAppNoTicketstests, self).tearDown() + config_values = { + "enable_tickets": False, + "tickets_folder": None, + } - @patch.dict('pagure.config.config', {'TICKETS_FOLDER': None}) def test_new_project_no_tickets_folder(self): """ Test the new_project endpoint with TICKETS_FOLDER is None. """ # Before diff --git a/tests/test_pagure_flask_ui_plugins_pagure_ci.py b/tests/test_pagure_flask_ui_plugins_pagure_ci.py index 302c34d..c628be1 100644 --- a/tests/test_pagure_flask_ui_plugins_pagure_ci.py +++ b/tests/test_pagure_flask_ui_plugins_pagure_ci.py @@ -6,14 +6,6 @@ import unittest import sys import os -# Insert the PAGURE_CONFIG env variable before we do the imports -HERE = os.path.join(os.path.dirname(os.path.abspath(__file__))) -CONFIG = os.path.join(HERE, 'test_config') -os.environ['PAGURE_CONFIG'] = CONFIG - -sys.path.insert(0, os.path.join(os.path.dirname( - os.path.abspath(__file__)), '..')) - import pagure.lib import tests From a297422610a0c93906329fcbd770858277304024 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mar 23 2018 17:14:14 +0000 Subject: [PATCH 7/9] Tests: run the admin commands as functions, not as subprocesses --- diff --git a/pagure/cli/admin.py b/pagure/cli/admin.py index c4eea5c..7ba5419 100644 --- a/pagure/cli/admin.py +++ b/pagure/cli/admin.py @@ -262,7 +262,7 @@ def _parser_read_only(subparser): local_parser.set_defaults(func=do_read_only) -def parse_arguments(): +def parse_arguments(args=None): """ Set-up the argument parsing. """ parser = argparse.ArgumentParser( description='The admin CLI for this pagure instance') @@ -298,7 +298,7 @@ def parse_arguments(): # read-only _parser_read_only(subparser) - return parser.parse_args() + return parser.parse_args(args) def _ask_confirmation(): diff --git a/tests/__init__.py b/tests/__init__.py index 2c10f7a..d6da102 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -893,6 +893,24 @@ def add_binary_git_repo(folder, filename): shutil.rmtree(newfolder) +@contextmanager +def capture_output(merge_stderr=True): + import sys + from cStringIO import StringIO + oldout, olderr = sys.stdout, sys.stderr + try: + out = StringIO() + err = StringIO() + if merge_stderr: + sys.stdout = sys.stderr = out + yield out + else: + sys.stdout, sys.stderr = out, err + yield out, err + finally: + sys.stdout, sys.stderr = oldout, olderr + + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase(Modeltests) unittest.TextTestRunner(verbosity=2).run(SUITE) diff --git a/tests/test_pagure_admin.py b/tests/test_pagure_admin.py index 7e1461b..a3479c0 100644 --- a/tests/test_pagure_admin.py +++ b/tests/test_pagure_admin.py @@ -26,310 +26,11 @@ sys.path.insert(0, os.path.join(os.path.dirname( os.path.abspath(__file__)), '..')) import pagure.config # noqa +import pagure.exceptions # noqa: E402 import pagure.cli.admin # noqa import pagure.lib.model # noqa import tests # noqa -PAGURE_ADMIN = os.path.abspath( - os.path.join(tests.HERE, '..', 'pagure', 'cli', 'admin.py')) - - -def _get_ouput(cmd): - """ Returns the std-out of the command specified. - - :arg cmd: the command to run provided as a list - :type cmd: list - - """ - my_env = os.environ.copy() - my_env["PYTHONPATH"] = os.path.abspath(os.path.join(tests.HERE, '..')) - output = subprocess.Popen( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - env=my_env, - ).communicate() - - return output - - -class PagureAdminHelptests(tests.Modeltests): - """ Tests for pagure-admin --help """ - - maxDiff = None - - def test_parse_arguments(self): - """ Test the parse_arguments function of pagure-admin, empty. """ - if 'BUILD_ID' in os.environ: - raise unittest.case.SkipTest('Skipping on jenkins/el7') - - header = 'usage: admin.py [-h] [-c CONFIG] [--debug]\n' + \ - ' {refresh-gitolite,refresh-ssh,' + \ - 'clear-hook-token,admin-token,get-watch,update-watch,' + \ - 'read-only}\n' - - py_version = tuple(int(el) for el in platform.python_version_tuple()) - if py_version < (2, 7, 7): - header = 'usage: admin.py [-h] [-c CONFIG] [--debug]\n' + \ - ' \n' + \ - ' {refresh-gitolite,refresh-ssh,' + \ - 'clear-hook-token,admin-token,get-watch,update-watch,' + \ - 'read-only}\n' - - cmd = ['python', PAGURE_ADMIN] - output = _get_ouput(cmd) - self.assertEqual(output[0], '') - self.assertEqual(output[1], header + ''' ... -admin.py: error: too few arguments -''') # noqa - - def test_parse_arguments_help(self): - """ Test the parse_arguments function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, '--help'] - header = 'usage: admin.py [-h] [-c CONFIG] [--debug]\n' + \ - ' {refresh-gitolite,refresh-ssh,' + \ - 'clear-hook-token,admin-token,get-watch,update-watch,' + \ - 'read-only}\n' - - py_version = tuple(int(el) for el in platform.python_version_tuple()) - if py_version < (2, 7, 7): - header = 'usage: admin.py [-h] [-c CONFIG] [--debug]\n' + \ - ' \n' + \ - ' {refresh-gitolite,refresh-ssh,' + \ - 'clear-hook-token,admin-token,get-watch,update-watch,' + \ - 'read-only}\n' - - self.assertEqual( - _get_ouput(cmd)[0], - header + ''' ... - -The admin CLI for this pagure instance - -optional arguments: - -h, --help show this help message and exit - -c CONFIG, --config CONFIG - Specify a configuration to use - --debug Increase the verbosity of the information displayed - -actions: - {refresh-gitolite,refresh-ssh,clear-hook-token,admin-token,get-watch,update-watch,read-only} - refresh-gitolite Re-generate the gitolite config file - refresh-ssh Re-write to disk every user's ssh key stored in the - database - clear-hook-token Generate a new hook token for every project in this - instance - admin-token Manages the admin tokens for this instance - get-watch Get someone's watch status on a project - update-watch Update someone's watch status on a project - read-only Get or set the read-only flag on a project -''') - - def test_parser_refresh_gitolite_help(self): - """ Test the parser_refresh_gitolite function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'refresh-gitolite', '--help'] - header = 'usage: admin.py refresh-gitolite [-h] [--user USER] ' + \ - '[--project PROJECT]\n' + \ - ' [--group GROUP] [--all]' - - py_version = tuple(int(el) for el in platform.python_version_tuple()) - if py_version < (2, 7, 7): - header = 'usage: admin.py refresh-gitolite [-h] [--user USER] '+ \ - '[--project PROJECT]\n' + \ - ' [--group GROUP] [--all]' - - print(_get_ouput(cmd)[0]) - - self.assertEqual( - _get_ouput(cmd)[0], - header + ''' - -optional arguments: - -h, --help show this help message and exit - --user USER User of the project (to use only on forks) - --project PROJECT Project to update (as namespace/project if there is a - namespace) - --group GROUP Group to refresh - --all Refresh all the projects -''') - - def test_parser_refresh_ssh_help(self): - """ Test the parser_refresh_ssh function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'refresh-ssh', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py refresh-ssh [-h] - -optional arguments: - -h, --help show this help message and exit -''') - - def test_parser_clear_hook_token_help(self): - """ Test the parser_clear_hook_token function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'clear-hook-token', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py clear-hook-token [-h] - -optional arguments: - -h, --help show this help message and exit -''') - - def test_parser_admin_token_help(self): - """ Test the parser_admin_token function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'admin-token', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py admin-token [-h] {list,info,expire,create,update} ... - -optional arguments: - -h, --help show this help message and exit - -actions: - {list,info,expire,create,update} - list List the API admin token - info Provide some information about a specific API token - expire Expire a specific API token - create Create a new API token - update Update the expiration date of an API token -''') - - def test_parser_admin_token_create_help(self): - """ Test the parser_admin_token_create function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'create', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py admin-token create [-h] user - -positional arguments: - user User to associate with the token - -optional arguments: - -h, --help show this help message and exit -''') - - def test_parser_admin_token_update_help(self): - """ Test the parser_admin_token_create function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'update', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py admin-token update [-h] token date - -positional arguments: - token API token - date New expiration date - -optional arguments: - -h, --help show this help message and exit -''') - - def test_parser_admin_token_list_help(self): - """ Test the _parser_admin_token_list function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py admin-token list [-h] [--user USER] [--token TOKEN] [--active] - [--expired] - -optional arguments: - -h, --help show this help message and exit - --user USER User to associate or associated with the token - --token TOKEN API token - --active Only list active API token - --expired Only list expired API token -''') # noqa - - def test_parser_admin_token_info_help(self): - """ Test the _parser_admin_token_info function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'info', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py admin-token info [-h] token - -positional arguments: - token API token - -optional arguments: - -h, --help show this help message and exit -''') - - def test_parser_admin_token_expire_help(self): - """ Test the _parser_admin_token_expire function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'expire', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py admin-token expire [-h] token - -positional arguments: - token API token - -optional arguments: - -h, --help show this help message and exit -''') - - def test_parser_admin_token_invalid_help(self): - """ Test the _parser_admin_token_expire function of pagure-admin. """ - if 'BUILD_ID' in os.environ: - raise unittest.case.SkipTest('Skipping on jenkins/el7') - - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'foo', '--help'] - self.assertEqual( - _get_ouput(cmd)[1], - '''usage: admin.py admin-token [-h] {list,info,expire,create,update} ... -admin.py admin-token: error: invalid choice: 'foo' (choose from 'list', 'info', 'expire', 'create', 'update') -''') # noqa - - def test_parser_get_watch(self): - """ Test the _parser_get_watch function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'get-watch', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py get-watch [-h] project user - -positional arguments: - project Project (as namespace/project if there is a namespace) -- Fork - not supported - user User to get the watch status of - -optional arguments: - -h, --help show this help message and exit -''') - - def test_parser_update_watch(self): - """ Test the _parser_update_watch function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'update-watch', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py update-watch [-h] [-s STATUS] project user - -positional arguments: - project Project to update (as namespace/project if there is a - namespace) -- Fork not supported - user User to update the watch status of - -optional arguments: - -h, --help show this help message and exit - -s STATUS, --status STATUS - Watch status to update to -''') - - def test_parser_read_only(self): - """ Test the _parser_update_watch function of pagure-admin. """ - cmd = ['python', PAGURE_ADMIN, 'read-only', '--help'] - self.assertEqual( - _get_ouput(cmd)[0], - '''usage: admin.py read-only [-h] [--user USER] [--ro RO] project - -positional arguments: - project Project to update (as namespace/project if there is a - namespace) - -optional arguments: - -h, --help show this help message and exit - --user USER User of the project (to use only on forks) - --ro RO Read-Only status to set (has to be: true or false), do not - specify to get the current status -''') - class PagureAdminAdminTokenEmptytests(tests.Modeltests): """ Tests for pagure-admin admin-token when there is nothing in the DB @@ -346,15 +47,28 @@ class PagureAdminAdminTokenEmptytests(tests.Modeltests): """ Test the do_create_admin_token function of pagure-admin without user. """ - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'create', 'pingou'] - self.assertEqual(_get_ouput(cmd)[0], 'No user "pingou" found\n') + args = munch.Munch({'user': "pingou"}) + with self.assertRaises(pagure.exceptions.PagureException) as cm: + pagure.cli.admin.do_create_admin_token(args) + self.assertEqual( + cm.exception.args[0], + 'No user "pingou" found' + ) def test_do_list_admin_token_empty(self): """ Test the do_list_admin_token function of pagure-admin when there are not tokens in the db. """ - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list'] - self.assertEqual(_get_ouput(cmd)[0], 'No admin tokens found\n') + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': False, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() + self.assertEqual(output, 'No admin tokens found\n') class PagureAdminAdminRefreshGitolitetests(tests.Modeltests): @@ -526,8 +240,15 @@ class PagureAdminAdminTokentests(tests.Modeltests): pagure.cli.admin.do_create_admin_token(args) # Check the outcome - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': False, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertNotEqual(output, 'No user "pingou" found\n') self.assertEqual(len(output.split('\n')), 2) self.assertIn(' -- pingou -- ', output) @@ -544,17 +265,29 @@ class PagureAdminAdminTokentests(tests.Modeltests): pagure.cli.admin.do_create_admin_token(args) # Retrieve all tokens - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': False, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertNotEqual(output, 'No user "pingou" found\n') self.assertEqual(len(output.split('\n')), 2) self.assertIn(' -- pingou -- ', output) # Retrieve pfrields's tokens - cmd = [ - 'python', PAGURE_ADMIN, - 'admin-token', 'list', '--user', 'pfrields'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': 'pfrields', + 'token': None, + 'active': False, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertEqual(output, 'No admin tokens found\n') @patch('pagure.cli.admin._get_input') @@ -569,16 +302,25 @@ class PagureAdminAdminTokentests(tests.Modeltests): pagure.cli.admin.do_create_admin_token(args) # Retrieve the token - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': False, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertNotEqual(output, 'No user "pingou" found\n') self.assertEqual(len(output.split('\n')), 2) self.assertIn(' -- pingou -- ', output) token = output.split(' ', 1)[0] - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'info', token] - output = _get_ouput(cmd)[0] + args = munch.Munch({'token': token}) + with tests.capture_output() as output: + pagure.cli.admin.do_info_admin_token(args) + output = output.getvalue() self.assertIn(' -- pingou -- ', output.split('\n', 1)[0]) self.assertEqual( output.split('\n', 1)[1], '''ACLs: @@ -602,8 +344,15 @@ class PagureAdminAdminTokentests(tests.Modeltests): pagure.cli.admin.do_create_admin_token(args) # Retrieve the token - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': False, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertNotEqual(output, 'No user "pingou" found\n') self.assertEqual(len(output.split('\n')), 2) self.assertIn(' -- pingou -- ', output) @@ -611,8 +360,15 @@ class PagureAdminAdminTokentests(tests.Modeltests): token = output.split(' ', 1)[0] # Before - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list', '--active'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': True, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertNotEqual(output, 'No admin tokens found\n') self.assertEqual(len(output.split('\n')), 2) self.assertIn(' -- pingou -- ', output) @@ -622,8 +378,15 @@ class PagureAdminAdminTokentests(tests.Modeltests): pagure.cli.admin.do_expire_admin_token(args) # After - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list', '--active'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': True, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertEqual(output, 'No admin tokens found\n') @patch('pagure.cli.admin._get_input') @@ -642,8 +405,15 @@ class PagureAdminAdminTokentests(tests.Modeltests): pagure.cli.admin.do_create_admin_token(args) # Retrieve the token - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': False, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertNotEqual(output, 'No user "pingou" found\n') self.assertEqual(len(output.split('\n')), 2) self.assertIn(' -- pingou -- ', output) @@ -675,8 +445,15 @@ class PagureAdminAdminTokentests(tests.Modeltests): pagure.cli.admin.do_create_admin_token(args) # Retrieve the token - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': False, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertNotEqual(output, 'No user "pingou" found\n') self.assertEqual(len(output.split('\n')), 2) self.assertIn(' -- pingou -- ', output) @@ -708,8 +485,15 @@ class PagureAdminAdminTokentests(tests.Modeltests): pagure.cli.admin.do_create_admin_token(args) # Retrieve the token - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': False, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertNotEqual(output, 'No user "pingou" found\n') self.assertEqual(len(output.split('\n')), 2) self.assertIn(' -- pingou -- ', output) @@ -742,8 +526,15 @@ class PagureAdminAdminTokentests(tests.Modeltests): pagure.cli.admin.do_create_admin_token(args) # Retrieve the token - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': False, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertNotEqual(output, 'No user "pingou" found\n') self.assertEqual(len(output.split('\n')), 2) self.assertIn(' -- pingou -- ', output) @@ -752,8 +543,15 @@ class PagureAdminAdminTokentests(tests.Modeltests): current_expiration = output.strip().split(' -- ', 2)[-1] # Before - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list', '--active'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': True, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertNotEqual(output, 'No admin tokens found\n') self.assertEqual(len(output.split('\n')), 2) self.assertIn(' -- pingou -- ', output) @@ -769,8 +567,15 @@ class PagureAdminAdminTokentests(tests.Modeltests): pagure.cli.admin.do_update_admin_token(args) # After - cmd = ['python', PAGURE_ADMIN, 'admin-token', 'list', '--active'] - output = _get_ouput(cmd)[0] + list_args = munch.Munch({ + 'user': None, + 'token': None, + 'active': True, + 'expired': False, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_list_admin_token(list_args) + output = output.getvalue() self.assertEqual(output.split(' ', 1)[0], token) self.assertNotEqual( output.strip().split(' -- ', 2)[-1], @@ -837,36 +642,56 @@ class PagureAdminGetWatchTests(tests.Modeltests): """ Test the get-watch function of pagure-admin with an unknown project. """ - - cmd = ['python', PAGURE_ADMIN, 'get-watch', 'foobar', 'pingou'] - output = _get_ouput(cmd)[0] - self.assertEqual('No project found with: foobar\n', output) + args = munch.Munch({ + 'project': 'foobar', + 'user': 'pingou', + }) + with self.assertRaises(pagure.exceptions.PagureException) as cm: + pagure.cli.admin.do_get_watch_status(args) + self.assertEqual( + cm.exception.args[0], + 'No project found with: foobar' + ) def test_get_watch_get_project_invalid_project(self): """ Test the get-watch function of pagure-admin with an invalid project. """ - - cmd = ['python', PAGURE_ADMIN, 'get-watch', 'fo/o/bar', 'pingou'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'fo/o/bar', + 'user': 'pingou', + }) + with self.assertRaises(pagure.exceptions.PagureException) as cm: + pagure.cli.admin.do_get_watch_status(args) self.assertEqual( - 'Invalid project name, has more than one "/": fo/o/bar\n', - output) + cm.exception.args[0], + 'Invalid project name, has more than one "/": fo/o/bar', + ) def test_get_watch_get_project_invalid_user(self): """ Test the get-watch function of pagure-admin on a invalid user. """ - - cmd = ['python', PAGURE_ADMIN, 'get-watch', 'test', 'beebop'] - output = _get_ouput(cmd)[0] - self.assertEqual('No user "beebop" found\n', output) + args = munch.Munch({ + 'project': 'test', + 'user': 'beebop', + }) + with self.assertRaises(pagure.exceptions.PagureException) as cm: + pagure.cli.admin.do_get_watch_status(args) + self.assertEqual( + cm.exception.args[0], + 'No user "beebop" found' + ) def test_get_watch_get_project(self): """ Test the get-watch function of pagure-admin on a regular project. """ - - cmd = ['python', PAGURE_ADMIN, 'get-watch', 'test', 'pingou'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'test', + 'user': 'pingou', + }) + with tests.capture_output() as output: + pagure.cli.admin.do_get_watch_status(args) + output = output.getvalue() self.assertEqual( 'On test user: pingou is watching the following items: ' 'issues, pull-requests\n', output) @@ -875,8 +700,13 @@ class PagureAdminGetWatchTests(tests.Modeltests): """ Test the get-watch function of pagure-admin on a regular project. """ - cmd = ['python', PAGURE_ADMIN, 'get-watch', 'test', 'foo'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'test', + 'user': 'foo', + }) + with tests.capture_output() as output: + pagure.cli.admin.do_get_watch_status(args) + output = output.getvalue() self.assertEqual( 'On test user: foo is watching the following items: None\n', output) @@ -885,10 +715,13 @@ class PagureAdminGetWatchTests(tests.Modeltests): """ Test the get-watch function of pagure-admin on a namespaced project. """ - cmd = [ - 'python', PAGURE_ADMIN, 'get-watch', - 'somenamespace/test', 'pingou'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'somenamespace/test', + 'user': 'pingou', + }) + with tests.capture_output() as output: + pagure.cli.admin.do_get_watch_status(args) + output = output.getvalue() self.assertEqual( 'On somenamespace/test user: pingou is watching the following ' 'items: issues, pull-requests\n', output) @@ -897,11 +730,15 @@ class PagureAdminGetWatchTests(tests.Modeltests): """ Test the get-watch function of pagure-admin on a namespaced project. """ - cmd = [ - 'python', PAGURE_ADMIN, 'get-watch', - 'somenamespace/test', 'foo'] - output = _get_ouput(cmd)[0] - _get_ouput(cmd) + args = munch.Munch({ + 'project': 'somenamespace/test', + 'user': 'foo', + }) + with tests.capture_output() as output: + pagure.cli.admin.do_get_watch_status(args) + output = output.getvalue() + with tests.capture_output() as _discarded: + pagure.cli.admin.do_get_watch_status(args) self.assertEqual( 'On somenamespace/test user: foo is watching the following ' 'items: None\n', output) @@ -967,59 +804,100 @@ class PagureAdminUpdateWatchTests(tests.Modeltests): """ Test the update-watch function of pagure-admin on an unknown project. """ - - cmd = ['python', PAGURE_ADMIN, 'update-watch', 'foob', 'pingou', '-s=1'] - output = _get_ouput(cmd)[0] - self.assertEqual('No project found with: foob\n', output) + args = munch.Munch({ + 'project': 'foob', + 'user': 'pingou', + 'status': '1' + }) + with self.assertRaises(pagure.exceptions.PagureException) as cm: + pagure.cli.admin.do_update_watch_status(args) + self.assertEqual( + cm.exception.args[0], + 'No project found with: foob' + ) def test_get_watch_update_project_invalid_project(self): """ Test the update-watch function of pagure-admin on an invalid project. """ - - cmd = ['python', PAGURE_ADMIN, 'update-watch', 'fo/o/b', 'pingou', '-s=1'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'fo/o/b', + 'user': 'pingou', + 'status': '1' + }) + with self.assertRaises(pagure.exceptions.PagureException) as cm: + pagure.cli.admin.do_update_watch_status(args) self.assertEqual( - 'Invalid project name, has more than one "/": fo/o/b\n', - output) + cm.exception.args[0], + 'Invalid project name, has more than one "/": fo/o/b', + ) def test_get_watch_update_project_invalid_user(self): """ Test the update-watch function of pagure-admin on an invalid user. """ - - cmd = ['python', PAGURE_ADMIN, 'update-watch', 'test', 'foob', '-s=1'] - output = _get_ouput(cmd)[0] - self.assertEqual('No user "foob" found\n', output) + args = munch.Munch({ + 'project': 'test', + 'user': 'foob', + 'status': '1' + }) + with self.assertRaises(pagure.exceptions.PagureException) as cm: + pagure.cli.admin.do_update_watch_status(args) + self.assertEqual( + cm.exception.args[0], + 'No user "foob" found' + ) def test_get_watch_update_project_invalid_status(self): """ Test the update-watch function of pagure-admin with an invalid status. """ - - cmd = ['python', PAGURE_ADMIN, 'update-watch', 'test', 'pingou', '-s=10'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'test', + 'user': 'pingou', + 'status': '10' + }) + with self.assertRaises(pagure.exceptions.PagureException) as cm: + pagure.cli.admin.do_update_watch_status(args) self.assertEqual( - 'Invalid status provided: 10 not in -1, 0, 1, 2, 3\n', output) + cm.exception.args[0], + 'Invalid status provided: 10 not in -1, 0, 1, 2, 3' + ) def test_get_watch_update_project_no_effect(self): """ Test the update-watch function of pagure-admin with a regular project - nothing changed. """ - cmd = ['python', PAGURE_ADMIN, 'get-watch', 'test', 'pingou'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'test', + 'user': 'pingou', + }) + with tests.capture_output() as output: + pagure.cli.admin.do_get_watch_status(args) + output = output.getvalue() self.assertEqual( 'On test user: pingou is watching the following items: ' 'issues, pull-requests\n', output) - cmd = ['python', PAGURE_ADMIN, 'update-watch', 'test', 'pingou', '-s=1'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'test', + 'user': 'pingou', + 'status': '1' + }) + with tests.capture_output() as output: + pagure.cli.admin.do_update_watch_status(args) + output = output.getvalue() self.assertEqual( 'Updating watch status of pingou to 1 (watch issues and PRs) ' 'on test\n', output) - cmd = ['python', PAGURE_ADMIN, 'get-watch', 'test', 'pingou'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'test', + 'user': 'pingou', + }) + with tests.capture_output() as output: + pagure.cli.admin.do_get_watch_status(args) + output = output.getvalue() self.assertEqual( 'On test user: pingou is watching the following items: ' 'issues, pull-requests\n', output) @@ -1077,28 +955,48 @@ class PagureAdminReadOnlyTests(tests.Modeltests): project. """ - cmd = ['python', PAGURE_ADMIN, 'read-only', 'foob'] - output = _get_ouput(cmd)[0] - self.assertEqual('No project found with: foob\n', output) + args = munch.Munch({ + 'project': 'foob', + 'user': None, + 'ro': None, + }) + with self.assertRaises(pagure.exceptions.PagureException) as cm: + pagure.cli.admin.do_read_only(args) + self.assertEqual( + cm.exception.args[0], + 'No project found with: foob' + ) def test_read_only_invalid_project(self): """ Test the read-only function of pagure-admin on an invalid project. """ - cmd = ['python', PAGURE_ADMIN, 'read-only', 'fo/o/b'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'fo/o/b', + 'user': None, + 'ro': None, + }) + with self.assertRaises(pagure.exceptions.PagureException) as cm: + pagure.cli.admin.do_read_only(args) self.assertEqual( - 'Invalid project name, has more than one "/": fo/o/b\n', - output) + cm.exception.args[0], + 'Invalid project name, has more than one "/": fo/o/b' + ) def test_read_only(self): """ Test the read-only function of pagure-admin to get status of a non-namespaced project. """ - cmd = ['python', PAGURE_ADMIN, 'read-only', 'test'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'test', + 'user': None, + 'ro': None, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_read_only(args) + output = output.getvalue() self.assertEqual( 'The current read-only flag of the project test is set to True\n', output) @@ -1108,8 +1006,14 @@ class PagureAdminReadOnlyTests(tests.Modeltests): a namespaced project. """ - cmd = ['python', PAGURE_ADMIN, 'read-only', 'somenamespace/test'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'somenamespace/test', + 'user': None, + 'ro': None, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_read_only(args) + output = output.getvalue() self.assertEqual( 'The current read-only flag of the project somenamespace/test '\ 'is set to True\n', output) @@ -1120,23 +1024,39 @@ class PagureAdminReadOnlyTests(tests.Modeltests): """ # Before - cmd = ['python', PAGURE_ADMIN, 'read-only', 'somenamespace/test'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'somenamespace/test', + 'user': None, + 'ro': None, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_read_only(args) + output = output.getvalue() self.assertEqual( 'The current read-only flag of the project somenamespace/test '\ 'is set to True\n', output) - cmd = [ - 'python', PAGURE_ADMIN, 'read-only', - 'somenamespace/test', '--ro', 'false'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'somenamespace/test', + 'user': None, + 'ro': 'false', + }) + with tests.capture_output() as output: + pagure.cli.admin.do_read_only(args) + output = output.getvalue() self.assertEqual( 'The read-only flag of the project somenamespace/test has been ' 'set to False\n', output) # After - cmd = ['python', PAGURE_ADMIN, 'read-only', 'somenamespace/test'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'somenamespace/test', + 'user': None, + 'ro': None, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_read_only(args) + output = output.getvalue() self.assertEqual( 'The current read-only flag of the project somenamespace/test '\ 'is set to False\n', output) @@ -1147,22 +1067,39 @@ class PagureAdminReadOnlyTests(tests.Modeltests): """ # Before - cmd = ['python', PAGURE_ADMIN, 'read-only', 'test'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'test', + 'user': None, + 'ro': None, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_read_only(args) + output = output.getvalue() self.assertEqual( 'The current read-only flag of the project test '\ 'is set to True\n', output) - cmd = [ - 'python', PAGURE_ADMIN, 'read-only', 'test', '--ro', 'true'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'test', + 'user': None, + 'ro': 'true', + }) + with tests.capture_output() as output: + pagure.cli.admin.do_read_only(args) + output = output.getvalue() self.assertEqual( 'The read-only flag of the project test has been ' 'set to True\n', output) # After - cmd = ['python', PAGURE_ADMIN, 'read-only', 'test'] - output = _get_ouput(cmd)[0] + args = munch.Munch({ + 'project': 'test', + 'user': None, + 'ro': None, + }) + with tests.capture_output() as output: + pagure.cli.admin.do_read_only(args) + output = output.getvalue() self.assertEqual( 'The current read-only flag of the project test '\ 'is set to True\n', output) From 8e7c53d438a87d0c215115d2735f457afd77bf98 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mar 23 2018 17:14:14 +0000 Subject: [PATCH 8/9] Fix a test --- diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 76b8ef9..4a1cde3 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -2291,7 +2291,6 @@ class PagureFlaskApiProjecttests(tests.Modeltests): ) # Create a project with a namespace and the user namespace feature on - pagure.config.config['ALLOWED_PREFIX'] = ['testns'] data = { 'name': 'testproject2', 'namespace': 'testns', @@ -2299,8 +2298,9 @@ class PagureFlaskApiProjecttests(tests.Modeltests): } # Valid request - output = self.app.post( - '/api/0/new/', data=data, headers=headers) + with patch.dict('pagure.config.config', {'ALLOWED_PREFIX': ['testns']}): + output = self.app.post( + '/api/0/new/', data=data, headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.data) self.assertDictEqual( From 00115db9434e0f12df949a94a671ae2f41dcbb4b Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mar 23 2018 17:14:15 +0000 Subject: [PATCH 9/9] Tests: run the task synchronously --- diff --git a/tests/__init__.py b/tests/__init__.py index d6da102..c7d23db 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -36,7 +36,9 @@ from urlparse import urlparse, parse_qs import mock import pygit2 +import redis +from celery.app.task import EagerResult from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker from sqlalchemy.orm import scoped_session @@ -78,7 +80,14 @@ TICKETS_FOLDER = %(tickets_folder)r DOCS_FOLDER = %(docs_folder)r ATTACHMENTS_FOLDER = '%(path)s/attachments' BROKER_URL = 'redis+socket://%(global_path)s/broker' +CELERY_CONFIG = { + "task_always_eager": True, +} """ +# The Celery docs warn against using task_always_eager: +# http://docs.celeryproject.org/en/latest/userguide/testing.html +# but that warning is only valid when testing the async nature of the task, not +# what the task actually does. LOG.info('BUILD_ID: %s', os.environ.get('BUILD_ID')) @@ -140,6 +149,8 @@ def user_set(APP, user): tests_state = { "path": tempfile.mkdtemp(prefix='pagure-tests-'), "broker": None, + "broker_client": None, + "results": {}, } @@ -176,6 +187,11 @@ def _populate_db(session): session.commit() +def store_eager_results(*args, **kwargs): + """A wrapper for EagerResult that stores the instance.""" + result = EagerResult(*args, **kwargs) + tests_state["results"][result.id] = result + return result def setUp(): @@ -198,10 +214,17 @@ def setUp(): broker.poll() if broker.returncode is not None: raise Exception('Broker failed to start') + tests_state["broker_client"] = redis.Redis(unix_socket_path=broker_url) + + # Store the EagerResults to be able to retrieve them later + tests_state["eg_patcher"] = mock.patch('celery.app.task.EagerResult') + eg_mock = tests_state["eg_patcher"].start() + eg_mock.side_effect = store_eager_results def tearDown(): tests_state["db_session"].close() + tests_state["eg_patcher"].stop() broker = tests_state["broker"] broker.kill() broker.wait() @@ -290,8 +313,12 @@ class SimplePagureTest(unittest.TestCase): self._app.logger.handlers = [] self.app = self._app.test_client() + self.gr_patcher = mock.patch('pagure.lib.tasks.get_result') + gr_mock = self.gr_patcher.start() + gr_mock.side_effect = lambda tid: tests_state["results"][tid] def tearDown(self): + self.gr_patcher.stop() self.session.rollback() self._clear_database() @@ -309,8 +336,8 @@ class SimplePagureTest(unittest.TestCase): del self._app def shortDescription(self): - doc = self.__str__() +": "+self._testMethodDoc - return doc or None + doc = self.__str__() + ": " + self._testMethodDoc + return doc or None def _prepare_db(self): self.dbpath = 'sqlite:///%s' % os.path.join( @@ -363,72 +390,12 @@ class Modeltests(SimplePagureTest): """ Set up the environnment, ran before every tests. """ # Clean up test performance info super(Modeltests, self).setUp() - - # Start a worker - # Using cocurrency 2 to test with some concurrency, but not be heavy - # Using eventlet so that worker.terminate kills everything - self.workerlog = open(os.path.join('.', 'worker.log'), 'w') - celery_exec = 'celery' - celery_env = os.environ.copy() - celery_env.update({ - 'PAGURE_BROKER_URL': pagure_config['BROKER_URL'], - 'PAGURE_CONFIG': os.path.join(self.path, 'config'), - 'PYTHONPATH': '.' - }) - celery_cwd = os.path.normpath( - os.path.join(os.path.dirname(__file__), '..') - ) - self.worker = subprocess.Popen( - [celery_exec, '-A', 'pagure.lib.tasks', 'worker', - '--loglevel=info', '--concurrency=2', '--pool=eventlet', - '--without-gossip', '--without-mingle', '--quiet'], - env=celery_env, - cwd=celery_cwd, - stdout=self.workerlog, - stderr=self.workerlog) - self.worker.poll() - if self.worker.returncode is not None: - raise Exception('Worker failed to start') - - # We could do the ping below in-process: - # pagure.lib.tasks.conn.control.ping(timeout=0.1) - # but if we try it, Python starts raising OSError - # with too many open files. This is probably related - # to https://github.com/celery/celery/issues/4465 - wait_start = time.time() - while True: - time.sleep(0.1) - res = subprocess.call( - [celery_exec, '-A', 'pagure.lib.tasks', - 'inspect', '-t=0.1', 'ping'], - env=celery_env, - cwd=celery_cwd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE - ) - if res == 0: - break - if time.time() - wait_start > 5: - raise Exception('Worker failed to initialize in 5 seconds') - self.app.get = create_maybe_waiter(self.app.get, self.app.get) self.app.post = create_maybe_waiter(self.app.post, self.app.get) def tearDown(self): # pylint: disable=invalid-name """ Remove the test.db database if there is one. """ - # Terminate worker - # We just send a SIGKILL (kill -9), since when the test finishes, we - # don't really care about the output of either worker or broker - # anymore - self.worker.kill() - self.worker.wait() - self.worker = None - self.workerlog.close() - self.workerlog = None - subprocess.check_call( - ['/usr/bin/redis-cli', '-s', - pagure_config['BROKER_URL'][len('redis+socket://'):], 'flushall'], - stdout=subprocess.PIPE, stderr=subprocess.PIPE) + tests_state["broker_client"].flushall() super(Modeltests, self).tearDown() diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 4a1cde3..52ffa84 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -19,6 +19,7 @@ import tempfile import os import pygit2 +from celery.result import EagerResult from mock import patch, Mock sys.path.insert(0, os.path.join(os.path.dirname( @@ -38,8 +39,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.gga_patcher = patch( 'pagure.lib.tasks.generate_gitolite_acls.delay') self.mock_gen_acls = self.gga_patcher.start() - task_result = Mock() - task_result.id = 'abc-1234' + task_result = EagerResult('abc-1234', True, "SUCCESS") self.mock_gen_acls.return_value = task_result def tearDown(self): @@ -2627,6 +2627,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.assertEqual(data, expected_output) self.mock_gen_acls.assert_called_once_with( name='test', namespace=None, user=None, group=None) + self.assertTrue(task_result.get.called) def test_api_generate_acls_no_project(self): """ Test the api_generate_acls method of the flask api when the project