From 9dd651f769aa0eaee4cad7ca68ca32cdb58ffa13 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jul 25 2018 09:15:21 +0000 Subject: [PATCH 1/3] Add tests for utils.py Signed-off-by: Chenxiong Qi --- diff --git a/test/test_utils.py b/test/test_utils.py index c4657c1..3fbb904 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -10,11 +10,18 @@ # option) any later version. See http://www.gnu.org/copyleft/gpl.html for # the full text of the license. +import json +import six + +from freezegun import freeze_time from mock import patch, Mock from pyrpkg.errors import rpkgError +from requests.exceptions import ConnectionError +from six.moves.configparser import NoOptionError +from six.moves.configparser import NoSectionError -from utils import unittest from fedpkg import utils +from utils import unittest class TestUtils(unittest.TestCase): @@ -141,3 +148,254 @@ class TestUtils(unittest.TestCase): expected = set(['el6', 'epel7', 'f25', 'f26', 'f27', 'f28']) actual = utils.get_release_branches('http://pdc.local') self.assertEqual(expected, actual) + + +@patch('requests.get') +class TestAssertNewTestsRepo(unittest.TestCase): + """Test assert_new_tests_repo""" + + def test_should_raise_error_if_connection_error_to_distgit(self, get): + get.side_effect = ConnectionError + + six.assertRaisesRegex( + self, rpkgError, 'The error was', + utils.assert_new_tests_repo, 'testrepo', 'http://distgit/') + + def test_test_repo_exists(self, get): + get.return_value = Mock(ok=True) + + six.assertRaisesRegex( + self, rpkgError, 'Repository .+ already exists', + utils.assert_new_tests_repo, 'testrepo', 'http://distgit/') + + def test_keep_quiet_if_repo_not_exist(self, get): + get.return_value = Mock(ok=False) + utils.assert_new_tests_repo('testrepo', 'http://distgit/') + + +class TestGetPagureToken(unittest.TestCase): + """Test get_pagure_token""" + + def test_return_token(self): + config = Mock() + config.get.return_value = '123456' + + token = utils.get_pagure_token(config, 'fedpkg') + + self.assertEqual('123456', token) + config.get.assert_called_once_with('fedpkg.pagure', 'token') + + def test_config_does_not_have_token(self): + config = Mock() + + config.get.side_effect = NoOptionError('token', 'fedpkg.pagure') + six.assertRaisesRegex(self, rpkgError, 'Missing a Pagure token', + utils.get_pagure_token, config, 'fedpkg') + + config.get.side_effect = NoSectionError('fedpkg.pagure') + six.assertRaisesRegex(self, rpkgError, 'Missing a Pagure token', + utils.get_pagure_token, config, 'fedpkg') + + +@patch('requests.get') +class TestGetServiceLevelType(unittest.TestCase): + """Test get_sl_type""" + + def test_raise_error_if_connection_error_to_pdc(self, get): + get.side_effect = ConnectionError + + six.assertRaisesRegex( + self, rpkgError, 'The connection to PDC failed', + utils.get_sl_type, 'http://localhost/', 'bug_fixes:2020-12-01') + + def test_sl_type_not_exist(self, get): + rv = Mock(ok=True) + rv.json.return_value = {'count': 0} + get.return_value = rv + + sl_type = utils.get_sl_type('http://localhost/', + 'bug_fixes:2020-12-01') + self.assertIsNone(sl_type) + + def test_raise_error_if_response_not_ok(self, get): + get.return_value = Mock(ok=False) + + six.assertRaisesRegex( + self, rpkgError, 'The following error occurred', + utils.get_sl_type, 'http://localhost/', 'bug_fixes:2020-12-01') + + +class TestVerifySLS(unittest.TestCase): + """Test verify_sls""" + + def test_sl_date_format_is_invalid(self): + six.assertRaisesRegex( + self, rpkgError, 'The EOL date .+ is in an invalid format', + utils.verify_sls, 'http://localhost/', {'bug_fixes': '2018/7/21'}) + + @freeze_time('2018-01-01') + @patch('requests.get') + def test_sl_not_exist(self, get): + rv = Mock(ok=True) + rv.json.return_value = {'count': 0} + + six.assertRaisesRegex( + self, rpkgError, 'The SL .+ is not in PDC', + utils.verify_sls, 'http://localhost/', {'some_sl': '2018-06-01'}) + + @freeze_time('2018-01-01') + @patch('requests.get') + def test_keep_quiet_if_service_levels_are_ok(self, get): + rv = Mock(ok=True) + rv.json.side_effect = [ + { + 'count': 1, + 'results': [{ + 'id': 1, + 'name': 'bug_fixes', + 'description': 'Bug fixes' + }], + }, + { + 'count': 1, + 'results': [{ + 'id': 2, + 'name': 'security_fixes', + 'description': 'Security fixes' + }], + } + ] + get.return_value = rv + + utils.verify_sls('http://localhost/', + { + 'bug_fixes': '2018-06-01', + 'security_fixes': '2018-12-01' + }) + + +@patch('requests.get') +class TestAssertValidEPELPackage(unittest.TestCase): + """Test assert_valid_epel_package""" + + def test_raise_error_if_connection_error(self, get): + get.side_effect = ConnectionError + + six.assertRaisesRegex( + self, rpkgError, 'The error was:', + utils.assert_valid_epel_package, 'pkg', 'epel7') + + def test_raise_error_if_GET_response_not_ok(self, get): + get.return_value = Mock(ok=False, status_code=404) + + six.assertRaisesRegex( + self, rpkgError, 'The status code was: 404', + utils.assert_valid_epel_package, 'pkg', 'epel7') + + def test_should_not_have_epel_branch_for_el6_pkg(self, get): + get.return_value.json.return_value = { + 'arches': [ + 'i686', 'noarch', 'i386', 'ppc64', 'ppc', 'x86_64' + ], + 'packages': { + 'pkg1': { + # For el6, these arches will cause error raised. + 'arch': ['i686', 'noarch', 'ppc64', 'x86_64'] + } + } + } + + six.assertRaisesRegex( + self, rpkgError, 'is built on all supported arches', + utils.assert_valid_epel_package, 'pkg1', 'el6') + + def test_should_not_have_epel_branch_for_el7_pkg(self, get): + get.return_value.json.return_value = { + 'arches': [ + 'i686', 'noarch', 'i386', 'ppc64', 'ppc', 'x86_64' + ], + 'packages': { + 'pkg1': { + # For epel7, these arches will cause error raised. + 'arch': ['i386', 'noarch', 'ppc64', 'x86_64'] + } + } + } + + six.assertRaisesRegex( + self, rpkgError, 'is built on all supported arches', + utils.assert_valid_epel_package, 'pkg1', 'epel7') + + def test_raise_error_if_package_has_noarch_only(self, get): + get.return_value.json.return_value = { + 'arches': [ + 'i686', 'noarch', 'i386', 'ppc64', 'ppc', 'x86_64' + ], + 'packages': { + 'pkg1': { + 'arch': ['noarch'] + } + } + } + + six.assertRaisesRegex( + self, rpkgError, 'This package is already an EL package', + utils.assert_valid_epel_package, 'pkg1', 'epel7') + + +@patch('requests.post') +class TestNewPagureIssue(unittest.TestCase): + """Test new_pagure_issue""" + + def test_raise_error_if_connection_error(self, post): + post.side_effect = ConnectionError + + six.assertRaisesRegex( + self, rpkgError, 'The connection to Pagure failed', + utils.new_pagure_issue, + 'http://distgit/', '123456', 'new package', {'repo': 'pkg1'}) + + def test_responses_not_ok_and_response_body_is_not_json(self, post): + rv = Mock(ok=False, text='error') + rv.json.side_effect = ValueError + post.return_value = rv + + six.assertRaisesRegex( + self, rpkgError, + 'The following error occurred while creating a new issue', + utils.new_pagure_issue, + 'http://distgit/', '123456', 'new package', {'repo': 'pkg1'}) + + def test_create_pagure_issue(self, post): + rv = Mock(ok=True) + rv.json.return_value = {'issue': {'id': 1}} + post.return_value = rv + + pagure_api_url = 'http://distgit' + issue_ticket_body = {'repo': 'pkg1'} + + issue_url = utils.new_pagure_issue(pagure_api_url, + '123456', + 'new package', + issue_ticket_body) + + expected_issue_url = ( + '{0}/releng/fedora-scm-requests/issue/1' + .format(pagure_api_url) + ) + self.assertEqual(expected_issue_url, issue_url) + + post.assert_called_once_with( + '{0}/api/0/releng/fedora-scm-requests/new_issue' + .format(pagure_api_url), + headers={ + 'Authorization': 'token {0}'.format(123456), + 'Accept': 'application/json', + 'Content-Type': 'application/json' + }, + data=json.dumps({ + 'title': 'new package', + 'issue_content': issue_ticket_body, + }), + timeout=60 + ) From 2e0228e663acd5a6903769695cb11cdd6bbe79be Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jul 25 2018 09:18:18 +0000 Subject: [PATCH 2/3] Add tests for some commands Signed-off-by: Chenxiong Qi --- diff --git a/fedpkg/__init__.py b/fedpkg/__init__.py index 2f545ac..f3b2efe 100644 --- a/fedpkg/__init__.py +++ b/fedpkg/__init__.py @@ -218,8 +218,8 @@ class Commands(pyrpkg.Commands): 'rawhide') except Exception: # We couldn't hit koji, bail. - raise pyrpkg.rpkgError('Unable to query koji to find rawhide \ - target') + raise pyrpkg.rpkgError( + 'Unable to query koji to find rawhide target') return self._tag2version(rawhidetarget['dest_tag_name']) def _determine_runtime_env(self): diff --git a/test/test_cli.py b/test/test_cli.py index 78e8c49..accddb9 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -11,10 +11,11 @@ # the full text of the license. import io -import os -import sys import json +import os import pkg_resources +import six +import sys try: import unittest2 as unittest @@ -27,21 +28,17 @@ except ImportError: bodhi = None from datetime import datetime, timedelta -from tempfile import mkdtemp -from os import rmdir +from fedpkg.bugzilla import BugzillaClient +from fedpkg.cli import check_bodhi_version from freezegun import freeze_time - -import six +from mock import call, patch, PropertyMock, Mock +from os import rmdir +from pyrpkg.errors import rpkgError +from six.moves import StringIO from six.moves.configparser import NoOptionError from six.moves.configparser import NoSectionError -from six.moves import StringIO - -from pyrpkg.errors import rpkgError +from tempfile import mkdtemp from utils import CliTestCase -from fedpkg.bugzilla import BugzillaClient -from fedpkg.cli import check_bodhi_version - -from mock import call, patch, PropertyMock, Mock class TestUpdate(CliTestCase): @@ -1200,6 +1197,25 @@ class TestBodhiOverride(CliTestCase): self.cbv_p.stop() super(TestBodhiOverride, self).tearDown() + def test_raise_error_if_build_not_exist(self): + self.kojisession.getBuild.return_value = None + + build_nvr = 'rpkg-1.54-1.fc28' + cli_cmd = [ + 'fedpkg', '--path', self.cloned_repo_path, + 'override', 'create', + '--duration', '7', '--notes', 'build for fedpkg', + build_nvr + ] + + with patch('sys.argv', new=cli_cmd): + cli = self.new_cli() + six.assertRaisesRegex( + self, rpkgError, 'Build {0} does not exist'.format(build_nvr), + cli.create_buildroot_override) + + self.kojisession.getBuild.assert_called_once_with(build_nvr) + @patch('bodhi.client.bindings.BodhiClient.list_overrides') @patch('bodhi.client.bindings.BodhiClient.save_override') @patch('bodhi.client.bindings.BodhiClient.override_str') @@ -1353,6 +1369,34 @@ class TestBodhiOverride(CliTestCase): notes='build for fedpkg') ]) + def test_invalid_duration_option(self): + cli_cmds = ( + ( + [ + 'fedpkg', '--path', self.cloned_repo_path, + 'override', 'create', + '--duration', 'abc', '--notes', 'build for fedpkg', + ], + 'duration must be an integer' + ), + ( + [ + 'fedpkg', '--path', self.cloned_repo_path, + 'override', 'create', + '--duration', '0', '--notes', 'build for fedpkg', + ], + 'override should have 1 day to exist at least' + ) + ) + + for cmd, expected_output in cli_cmds: + with patch('sys.argv', new=cmd): + with patch('sys.stderr', new=StringIO()): + with self.assertRaises(SystemExit): + self.new_cli() + output = sys.stderr.getvalue() + self.assertIn(expected_output, output) + @unittest.skipUnless(bodhi, 'Skip if no supported bodhi-client is available') class TestBodhiOverrideExtend(CliTestCase): @@ -1680,3 +1724,46 @@ class TestBodhiOverrideExtend(CliTestCase): }) for token in csrf.side_effect ]) + + @freeze_time('2018-07-22') + @patch('fedpkg.BodhiClient.list_overrides') + def test_raise_error_if_duration_less_than_today(self, list_overrides): + build_nvr = 'somepkg-1.54-2.fc28' + build_override = { + 'expiration_date': '2018-03-01 12:12:12', + 'nvr': build_nvr, + 'notes': 'build for other package', + 'build': {'nvr': build_nvr}, + 'submitter': {'name': 'someone'}, + 'expired_date': '2018-03-01 12:12:12', + } + + list_overrides.return_value = { + 'total': 1, + 'overrides': [build_override] + } + + # This duration should cause the expected error. + duration = '2018-07-18' + cli_cmd = [ + 'fedpkg', '--path', self.cloned_repo_path, + 'override', 'extend', duration, build_nvr + ] + with patch('sys.argv', new=cli_cmd): + cli = self.new_cli() + six.assertRaisesRegex( + self, rpkgError, + 'specified expiration date .+ should be future date', + cli.extend_buildroot_override) + + def test_invalid_duration_of_concrete_date_format(self): + cli_cmd = [ + 'fedpkg', '--path', self.cloned_repo_path, + 'override', 'extend', '2019/01/10', 'rpkg-1.10-1.fc28' + ] + with patch('sys.argv', new=cli_cmd): + with patch('sys.stderr', new=StringIO()): + with self.assertRaises(SystemExit): + cli = self.new_cli() + output = sys.stderr.getvalue() + self.assertIn('Invalid expiration date', output) diff --git a/test/test_commands.py b/test/test_commands.py index 835ba00..30fe8c5 100644 --- a/test/test_commands.py +++ b/test/test_commands.py @@ -9,10 +9,12 @@ # option) any later version. See http://www.gnu.org/copyleft/gpl.html for # the full text of the license. -from pyrpkg.errors import rpkgError -from utils import CommandTestCase +import six + from mock import call, patch, Mock, PropertyMock, mock_open +from pyrpkg.errors import rpkgError from six.moves import builtins +from utils import CommandTestCase class TestDetermineRuntimeEnv(CommandTestCase): @@ -30,7 +32,7 @@ class TestDetermineRuntimeEnv(CommandTestCase): self.assertEqual('fc25', result) @patch('platform.linux_distribution') - def test_return_None_if_cannot_os_is_unknown(self, linux_distribution): + def test_return_None_if_os_is_unknown(self, linux_distribution): linux_distribution.side_effect = ValueError self.assertEqual(None, self.cmd._determine_runtime_env()) @@ -44,10 +46,12 @@ class TestDetermineRuntimeEnv(CommandTestCase): result = self.cmd._determine_runtime_env() self.assertEqual('el6', result) - def test_return_for_centos(self): + def test_return_for_el(self): dists = [ (('CentOS', '6.9', 'Final'), 'el6'), (('CentOS Linux', '7.3.1611', 'Core'), 'el7'), + (('redhat', '6', None), 'el6'), + (('centos', '6', None), 'el6'), ] for dist, expected_dist_tag in dists: @@ -311,6 +315,20 @@ class TestFindMasterBranch(CommandTestCase): koji_session.getBuildTarget.assert_called_once_with('rawhide') self.assertEqual('28', result) + @patch('pyrpkg.Commands.anon_kojisession', new_callable=PropertyMock) + @patch('pyrpkg.Commands.repo', new_callable=PropertyMock) + def test_raise_error_if_koji_api_call_fails(self, repo, anon_kojisession): + # No f* branches in order to call Koji API to get dest_tag_name + repo.return_value.refs = ['rhel', 'private-branch'] + + koji_session = anon_kojisession.return_value + # As the code shows, any error will be caught + koji_session.getBuildTarget.side_effect = ValueError + + six.assertRaisesRegex( + self, rpkgError, 'Unable to query koji to find rawhide target', + self.cmd._findmasterbranch) + class TestOverrideBuildURL(CommandTestCase): """Test Commands.construct_build_url""" diff --git a/test/test_retire.py b/test/test_retire.py index 26a5ebd..eb7d9ba 100644 --- a/test/test_retire.py +++ b/test/test_retire.py @@ -91,3 +91,16 @@ class RetireTestCase(unittest.TestCase): self.assertRetired('my reason') self.assertEqual(len(client.cmd.push.call_args_list), 1) + + def test_package_is_retired_already(self): + self._setup_repo('ssh://git@pkgs.example.com/fedpkg') + with open(os.path.join(self.tmpdir, 'dead.package'), 'w') as f: + f.write('deak package') + + args = ['fedpkg', '--release=master', 'retire', 'my reason'] + client = self._fake_client(args) + client.log = mock.Mock() + client.retire() + args, kwargs = client.log.warn.call_args + self.assertIn('dead.package found, package probably already retired', + args[0]) From 96269b99a17b96b16d8fdf2655b8521d7afc8972 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jul 25 2018 09:20:05 +0000 Subject: [PATCH 3/3] Fix flake8 errors and typo in tests Signed-off-by: Chenxiong Qi --- diff --git a/test/test_cli.py b/test/test_cli.py index accddb9..69f657a 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -1764,6 +1764,6 @@ class TestBodhiOverrideExtend(CliTestCase): with patch('sys.argv', new=cli_cmd): with patch('sys.stderr', new=StringIO()): with self.assertRaises(SystemExit): - cli = self.new_cli() + self.new_cli() output = sys.stderr.getvalue() self.assertIn('Invalid expiration date', output) diff --git a/test/test_retire.py b/test/test_retire.py index eb7d9ba..dc205cc 100644 --- a/test/test_retire.py +++ b/test/test_retire.py @@ -95,7 +95,7 @@ class RetireTestCase(unittest.TestCase): def test_package_is_retired_already(self): self._setup_repo('ssh://git@pkgs.example.com/fedpkg') with open(os.path.join(self.tmpdir, 'dead.package'), 'w') as f: - f.write('deak package') + f.write('dead package') args = ['fedpkg', '--release=master', 'retire', 'my reason'] client = self._fake_client(args)