From 4fe245b63b6161734f2cca24e65aff44e591cea9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 31 2017 10:43:01 +0000 Subject: [PATCH 1/2] Fix setting up the git hooks when there is no DOCS_FOLDER set This happens on pagure instance where the documentation hosting is disabled but that shouldn't prevent the git hooks from working most of the time. So we are testing that we do not fail to set up the hooks. Fixes https://pagure.io/pagure/issue/2462 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/hooks/__init__.py b/pagure/hooks/__init__.py index 62f214e..540441a 100644 --- a/pagure/hooks/__init__.py +++ b/pagure/hooks/__init__.py @@ -53,9 +53,10 @@ class BaseHook(object): for folder in [ APP.config.get('DOCS_FOLDER'), APP.config.get('REQUESTS_FOLDER')]: - repopaths.append( - os.path.join(folder, project.path) - ) + if folder: + repopaths.append( + os.path.join(folder, project.path) + ) hook_files = os.path.join( os.path.dirname(os.path.realpath(__file__)), 'files') diff --git a/tests/test_pagure_flask_ui_plugins_fedmsg.py b/tests/test_pagure_flask_ui_plugins_fedmsg.py index d057793..59b7e29 100644 --- a/tests/test_pagure_flask_ui_plugins_fedmsg.py +++ b/tests/test_pagure_flask_ui_plugins_fedmsg.py @@ -42,12 +42,12 @@ class PagureFlaskPluginFedmsgtests(tests.SimplePagureTest): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - - def test_plugin_fedmsg(self): - """ Test the fedmsg plugin on/off endpoint. """ - tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'repos')) + tests.create_projects_git(os.path.join(self.path, 'docs')) + + def test_plugin_fedmsg_defaul_page(self): + """ Test the fedmsg plugin endpoint's default page. """ user = tests.FakeUser(username='pingou') with tests.user_set(pagure.APP, user): @@ -61,8 +61,7 @@ class PagureFlaskPluginFedmsgtests(tests.SimplePagureTest): '' in output.data) - csrf_token = output.data.split( - 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + csrf_token = self.get_csrf(output=output) data = {} @@ -79,8 +78,20 @@ class PagureFlaskPluginFedmsgtests(tests.SimplePagureTest): self.assertFalse(os.path.exists(os.path.join( self.path, 'repos', 'test.git', 'hooks', 'post-receive.fedmsg'))) + self.assertFalse(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) + + def test_plugin_fedmsg_no_data(self): + """ Test the setting up the fedmsg plugin when there are no Docs + folder. + """ - data['csrf_token'] = csrf_token + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + + data = {'csrf_token': csrf_token} # With the git repo output = self.app.post( @@ -104,6 +115,18 @@ class PagureFlaskPluginFedmsgtests(tests.SimplePagureTest): self.assertFalse(os.path.exists(os.path.join( self.path, 'repos', 'test.git', 'hooks', 'post-receive.fedmsg'))) + self.assertFalse(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) + + def test_plugin_fedmsg_activate(self): + """ Test the setting up the fedmsg plugin when there are no Docs + folder. + """ + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() # Activate hook data = { @@ -133,6 +156,19 @@ class PagureFlaskPluginFedmsgtests(tests.SimplePagureTest): self.assertTrue(os.path.exists(os.path.join( self.path, 'repos', 'test.git', 'hooks', 'post-receive.fedmsg'))) + self.assertTrue(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) + + def test_plugin_fedmsg_deactivate(self): + """ Test the setting up the fedmsg plugin when there are no Docs + folder. + """ + self.test_plugin_fedmsg_activate() + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() # De-Activate hook data = {'csrf_token': csrf_token} @@ -158,6 +194,36 @@ class PagureFlaskPluginFedmsgtests(tests.SimplePagureTest): self.assertFalse(os.path.exists(os.path.join( self.path, 'repos', 'test.git', 'hooks', 'post-receive.fedmsg'))) + self.assertTrue(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) + + @patch.dict('pagure.APP.config', {'DOCS_FOLDER': None}) + def test_plugin_fedmsg_no_docs(self): + """ Test the setting up the fedmsg plugin when there are no Docs + folder. + """ + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + + # Activate hook + data = { + 'csrf_token': csrf_token, + 'active': 'y', + } + + output = self.app.post( + '/test/settings/Fedmsg', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + + self.assertTrue(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive.fedmsg'))) + self.assertFalse(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) if __name__ == '__main__': From 8c7787f96c748bb58266e11c7a37b1f8ce24486f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 31 2017 12:46:24 +0000 Subject: [PATCH 2/2] Fix setting up the pagure hooks when there is no DOCS_FOLDER set On a pagure instance where the documentation hosting is disabled, the DOCS_FOLDER variable is set to ``None`` and this makes installing the pagure hook crash if we're not careful. This commit fixes this situation and test it so it doesn't happen again. Relates to https://pagure.io/pagure/issue/2462 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/hooks/pagure_hook.py b/pagure/hooks/pagure_hook.py index 16880d5..2d655cb 100644 --- a/pagure/hooks/pagure_hook.py +++ b/pagure/hooks/pagure_hook.py @@ -116,9 +116,10 @@ class PagureHook(BaseHook): for folder in [ APP.config.get('DOCS_FOLDER'), APP.config.get('REQUESTS_FOLDER')]: - repopaths.append( - os.path.join(folder, project.path) - ) + if folder: + repopaths.append( + os.path.join(folder, project.path) + ) cls.base_install(repopaths, dbobj, 'pagure', 'pagure_hook.py') @@ -134,8 +135,9 @@ class PagureHook(BaseHook): for folder in [ APP.config.get('DOCS_FOLDER'), APP.config.get('REQUESTS_FOLDER')]: - repopaths.append( - os.path.join(folder, project.path) - ) + if folder: + repopaths.append( + os.path.join(folder, project.path) + ) cls.base_remove(repopaths, 'pagure') diff --git a/tests/test_pagure_flask_ui_plugins_pagure_hook.py b/tests/test_pagure_flask_ui_plugins_pagure_hook.py index 5d22a77..321dd29 100644 --- a/tests/test_pagure_flask_ui_plugins_pagure_hook.py +++ b/tests/test_pagure_flask_ui_plugins_pagure_hook.py @@ -42,12 +42,12 @@ class PagureFlaskPluginPagureHooktests(tests.SimplePagureTest): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - - def test_plugin_mail(self): - """ Test the pagure hook plugin on/off endpoint. """ - tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'repos')) + tests.create_projects_git(os.path.join(self.path, 'docs')) + + def test_plugin_mail_page(self): + """ Test the default page of the pagure hook plugin. """ user = tests.FakeUser(username='pingou') with tests.user_set(pagure.APP, user): @@ -61,8 +61,11 @@ class PagureFlaskPluginPagureHooktests(tests.SimplePagureTest): '' in output.data) - csrf_token = output.data.split( - 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + def test_plugin_mail_no_data(self): + """ Test the pagure hook plugin endpoint when no data is sent. """ + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): data = {} @@ -76,7 +79,23 @@ class PagureFlaskPluginPagureHooktests(tests.SimplePagureTest): '' in output.data) - data['csrf_token'] = csrf_token + self.assertFalse(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive'))) + self.assertFalse(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) + + def test_plugin_mail_no_data_csrf(self): + """ Test the pagure hook plugin endpoint when no data is sent but + the csrf token. + """ + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + + data = {'csrf_token': csrf_token} tests.create_projects_git(os.path.join(self.path, 'docs')) tests.create_projects_git(os.path.join(self.path, 'requests')) @@ -105,6 +124,20 @@ class PagureFlaskPluginPagureHooktests(tests.SimplePagureTest): self.assertFalse(os.path.exists(os.path.join( self.path, 'repos', 'test.git', 'hooks', 'post-receive.pagure'))) + self.assertFalse(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive'))) + self.assertFalse(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) + + def test_plugin_mail_activate_hook(self): + """ Test the pagure hook plugin endpoint when activating the hook. + """ + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() # Activate hook data = { @@ -135,6 +168,21 @@ class PagureFlaskPluginPagureHooktests(tests.SimplePagureTest): self.assertTrue(os.path.exists(os.path.join( self.path, 'repos', 'test.git', 'hooks', 'post-receive.pagure'))) + self.assertTrue(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive'))) + self.assertTrue(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) + + def test_plugin_mail_deactivate_hook(self): + """ Test the pagure hook plugin endpoint when activating the hook. + """ + self.test_plugin_mail_activate_hook() + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() # De-Activate hook data = {'csrf_token': csrf_token} @@ -161,6 +209,102 @@ class PagureFlaskPluginPagureHooktests(tests.SimplePagureTest): self.assertFalse(os.path.exists(os.path.join( self.path, 'repos', 'test.git', 'hooks', 'post-receive.pagure'))) + self.assertTrue(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive'))) + self.assertTrue(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) + + @patch.dict('pagure.APP.config', {'DOCS_FOLDER': None}) + def test_plugin_mail_activate_hook_no_doc(self): + """ Test the pagure hook plugin endpoint when activating the hook + on a pagure instance that de-activated the doc repos. + """ + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + + # Activate hook + data = { + 'csrf_token': csrf_token, + 'active': 'y', + } + + output = self.app.post( + '/test/settings/Pagure', data=data, follow_redirects=True) + self.assertTrue( + '\n Hook Pagure activated' + in output.data) + + self.assertTrue(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive.pagure'))) + self.assertTrue(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive'))) + self.assertFalse(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) + + @patch.dict('pagure.APP.config', {'DOCS_FOLDER': None}) + def test_plugin_mail_deactivate_hook_no_doc(self): + """ Test the pagure hook plugin endpoint when activating then + deactivating the hook on a pagure instance that de-activated the + doc repos. + """ + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + + # Activate hook + data = { + 'csrf_token': csrf_token, + 'active': 'y', + } + + output = self.app.post( + '/test/settings/Pagure', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '
\n

Settings for test

', + output.data) + self.assertTrue( + '\n Hook Pagure activated' + in output.data) + + self.assertTrue(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive.pagure'))) + self.assertTrue(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive'))) + self.assertFalse(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) + + # Deactivate hook + data = { + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/test/settings/Pagure', data=data, follow_redirects=True) + self.assertTrue( + '\n Hook Pagure deactivated' + in output.data) + + self.assertFalse(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive.pagure'))) + self.assertTrue(os.path.exists(os.path.join( + self.path, 'repos', 'test.git', 'hooks', + 'post-receive'))) + self.assertFalse(os.path.exists(os.path.join( + self.path, 'docs', 'test.git', 'hooks', + 'post-receive'))) if __name__ == '__main__':