From d03dda9d22f08156dc008ba72c1f5e476c9a78f7 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Feb 19 2019 11:50:44 +0000 Subject: [PATCH 1/7] Fix unit tests --- diff --git a/Makefile b/Makefile index 155ec9d..ffd1b88 100644 --- a/Makefile +++ b/Makefile @@ -80,10 +80,9 @@ test3: coverage3 erase PYTHONPATH=hub/.:plugins/hub/.:plugins/builder/.:plugins/cli/.:cli/. coverage3 run \ --rcfile .coveragerc3 --source . \ - /usr/bin/nosetests-3 \ - tests/test_lib tests/test_cli tests/test_hub - coverage report --rcfile .coveragerc3 - coverage html --rcfile .coveragerc3 + /usr/bin/nosetests-3 + coverage3 report --rcfile .coveragerc3 + coverage3 html --rcfile .coveragerc3 @echo Full coverage report at file://${PWD}/htmlcov/index.html test-tarball: diff --git a/koji/tasks.py b/koji/tasks.py index ba4cd31..999c37c 100644 --- a/koji/tasks.py +++ b/koji/tasks.py @@ -479,7 +479,7 @@ class BaseTaskHandler(object): fsrc = six.moves.urllib.request.urlopen(url) if not os.path.exists(os.path.dirname(fn)): os.makedirs(os.path.dirname(fn)) - with open(fn, 'wb') as fdst: + with open(fn, 'w') as fdst: shutil.copyfileobj(fsrc, fdst) fsrc.close() else: diff --git a/tests/test_builder/loadkojid.py b/tests/test_builder/loadkojid.py index d4e26e5..298ed91 100644 --- a/tests/test_builder/loadkojid.py +++ b/tests/test_builder/loadkojid.py @@ -6,7 +6,9 @@ import sys KOJID_FILENAME = os.path.dirname(__file__) + "/../../builder/kojid" if sys.version_info[0] >= 3: import importlib.util - spec = importlib.util.spec_from_file_location("koji_kojid", KOJID_FILENAME) + import importlib.machinery + loader = importlib.machinery.SourceFileLoader('koji_kojid', KOJID_FILENAME) + spec = importlib.util.spec_from_file_location("koji_kojid", loader=loader) kojid = importlib.util.module_from_spec(spec) spec.loader.exec_module(kojid) else: diff --git a/tests/test_builder/test_build_notification.py b/tests/test_builder/test_build_notification.py index 7bff6c1..5dae10e 100644 --- a/tests/test_builder/test_build_notification.py +++ b/tests/test_builder/test_build_notification.py @@ -2,7 +2,6 @@ from __future__ import absolute_import import json import mock import os -import smtplib import tempfile try: import unittest2 as unittest @@ -25,7 +24,7 @@ class MyClientSession(koji.ClientSession): fn = os.path.join(os.path.dirname(__file__), 'data/calls', name,'calls.json') with open(fn) as fp: data = json.load(fp) - data = koji.fixEncodingRecurse(data) + #data = koji.fixEncodingRecurse(data) for call in data: key = self._munge([call['method'], call['args'], call['kwargs']]) self._testcalls[key] = call @@ -101,6 +100,6 @@ class TestBuildNotification(unittest.TestCase): self.assertEqual(from_addr, "koji@example.com") self.assertEqual(recipients, ["user@example.com"]) fn = os.path.join(os.path.dirname(__file__), 'data/calls', 'build_notif_1', 'message.txt') - with open(fn) as fp: + with open(fn, 'rb') as fp: msg_expect = fp.read() self.assertEqual(message, msg_expect) diff --git a/tests/test_builder/test_volume_id.py b/tests/test_builder/test_volume_id.py index 0aff870..c37f622 100644 --- a/tests/test_builder/test_volume_id.py +++ b/tests/test_builder/test_volume_id.py @@ -84,7 +84,7 @@ class TestVolumeID(unittest.TestCase): def test_volume_id_substitutions(self): """Check that volume ID is shorten corect by shortenVolID method.""" - for test_name, values in self.test_cases.iteritems(): + for test_name, values in self.test_cases.items(): name = values['name'] expected_vol_id = values['expected-id'] result_vol_id = self.handler._shortenVolID(name, self.version, self.release) From a4f8dbd52e7959bb8109cb2ba25c9515dd433c74 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Feb 19 2019 12:52:01 +0000 Subject: [PATCH 2/7] py3 checks file path, so it needs to be non-mocked string --- diff --git a/tests/test_scm.py b/tests/test_scm.py index c97f057..f55d545 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -209,7 +209,7 @@ class TestSCMCheckouts(unittest.TestCase): self.tempdir = tempfile.mkdtemp() self.session = mock.MagicMock() self.uploadpath = mock.MagicMock() - self.logfile = mock.MagicMock() + self.logfile = '/dev/null' self.config = ''' default:* nocommon:*:no From 5e2a0cdb5ff4fe2202e3b3ad2debeda8a70b144c Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Feb 19 2019 12:52:46 +0000 Subject: [PATCH 3/7] remove sys.exc_clear() in web ui sys.exc_clear() doesn't exist in py3. It also doesn't seem to be really needed, so it should be safe to drop it. --- diff --git a/www/kojiweb/index.py b/www/kojiweb/index.py index 3a4a510..04248dc 100644 --- a/www/kojiweb/index.py +++ b/www/kojiweb/index.py @@ -669,9 +669,6 @@ def taskinfo(environ, taskID): excClass, exc = sys.exc_info()[:2] values['result'] = exc values['excClass'] = excClass - # clear the exception, since we're just using - # it for display purposes - sys.exc_clear() else: values['result'] = None values['excClass'] = None From 9a2e6cfad18c9cebee631389ba2cafeb34a5e268 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Feb 19 2019 12:55:06 +0000 Subject: [PATCH 4/7] wrapper function for writing to stdout --- diff --git a/cli/koji_cli/lib.py b/cli/koji_cli/lib.py index f05d5e1..f5558d5 100644 --- a/cli/koji_cli/lib.py +++ b/cli/koji_cli/lib.py @@ -343,6 +343,21 @@ Running Tasks: return rv +def write_to_stdout(contents): + """Helper function to write str/bytes to stdout + + https://docs.python.org/3/library/sys.html#sys.displayhook + """ + try: + sys.stdout.write(contents) + except UnicodeEncodeError: + bytes = contents.encode(sys.stdout.encoding, 'backslashreplace') + if hasattr(sys.stdout, 'buffer'): + sys.stdout.buffer.write(bytes) + else: + contents = bytes.decode(sys.stdout.encoding, 'strict') + sys.stdout.write(contents) + def watch_logs(session, tasklist, opts, poll_interval): print("Watching logs (this may be safely interrupted)...") @@ -391,10 +406,7 @@ def watch_logs(session, tasklist, opts, poll_interval): sys.stdout.write("\n") sys.stdout.write("==> %s <==\n" % currlog) lastlog = currlog - if six.PY3: - sys.stdout.buffer.write(contents) - else: - sys.stdout.write(contents) + write_to_stdout(contents) if opts.follow: diff --git a/koji/tasks.py b/koji/tasks.py index 999c37c..ba4cd31 100644 --- a/koji/tasks.py +++ b/koji/tasks.py @@ -479,7 +479,7 @@ class BaseTaskHandler(object): fsrc = six.moves.urllib.request.urlopen(url) if not os.path.exists(os.path.dirname(fn)): os.makedirs(os.path.dirname(fn)) - with open(fn, 'w') as fdst: + with open(fn, 'wb') as fdst: shutil.copyfileobj(fsrc, fdst) fsrc.close() else: diff --git a/plugins/cli/runroot.py b/plugins/cli/runroot.py index 9f0a463..bb8e31b 100644 --- a/plugins/cli/runroot.py +++ b/plugins/cli/runroot.py @@ -5,7 +5,7 @@ import time import koji from koji.plugin import export_cli from koji_cli.lib import _, activate_session, OptionParser, watch_tasks, \ - list_task_output_all_volumes + list_task_output_all_volumes, write_to_stdout import six @@ -98,10 +98,7 @@ def handle_runroot(options, session, args): log = session.downloadTaskOutput(task_id, 'runroot.log', volume=volume) # runroot output, while normally text, can be *anything*, so # treat it as binary - if six.PY3: - sys.stdout.buffer.write(log) - else: - sys.stdout.write(log) + write_to_stdout(log) info = session.getTaskInfo(task_id) if info is None: sys.exit(1) From d86711ba6e9fb75f2dc6a86e7def5e9cfab20302 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Feb 19 2019 12:55:37 +0000 Subject: [PATCH 5/7] enable py3 testing for everything --- diff --git a/.coveragerc3 b/.coveragerc3 index 6fa6072..7d4bec1 100644 --- a/.coveragerc3 +++ b/.coveragerc3 @@ -5,7 +5,6 @@ omit = /usr/* tests/* - util/* [report] exclude_lines = From 532a4f5bbc077ef852614414f95962989f062f37 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Feb 19 2019 12:56:16 +0000 Subject: [PATCH 6/7] fix py3 tests compatibility --- diff --git a/tests/test_docs_version.py b/tests/test_docs_version.py index e53c7f3..b4f33b1 100644 --- a/tests/test_docs_version.py +++ b/tests/test_docs_version.py @@ -1,5 +1,6 @@ from __future__ import absolute_import import os +import six import subprocess try: import unittest2 as unittest @@ -27,6 +28,8 @@ class TestDocsVersion(unittest.TestCase): output = popen.stdout.read() # rpm outputs a line for each subpackage version = output.splitlines()[0] + if six.PY3: + version = version.decode() return version def test_docs_version(self): diff --git a/tests/test_plugins/load_plugin.py b/tests/test_plugins/load_plugin.py index ac20795..a443a4e 100644 --- a/tests/test_plugins/load_plugin.py +++ b/tests/test_plugins/load_plugin.py @@ -19,10 +19,10 @@ def load_plugin(plugin_type, plugin_name): import importlib.machinery loader = importlib.machinery.SourceFileLoader(mod_name, CLI_FILENAME) spec = importlib.util.spec_from_loader(loader.name, loader) - kojid = importlib.util.module_from_spec(spec) - spec.loader.exec_module(kojid) - loader.exec_module(kojid) - sys.modules[mod_name] = kojid + plugin = importlib.util.module_from_spec(spec) + spec.loader.exec_module(plugin) + loader.exec_module(plugin) + sys.modules[mod_name] = plugin else: import imp plugin = imp.load_source(mod_name, CLI_FILENAME) diff --git a/tests/test_plugins/test_protonmsg.py b/tests/test_plugins/test_protonmsg.py index bd466a4..394b0d7 100644 --- a/tests/test_plugins/test_protonmsg.py +++ b/tests/test_plugins/test_protonmsg.py @@ -187,14 +187,14 @@ class TestProtonMsg(unittest.TestCase): def test_send_queued_msgs_fail(self, getLogger, Container): context.protonmsg_msgs = [('test.topic', {'testheader': 1}, 'test body')] conf = tempfile.NamedTemporaryFile() - conf.write("""[broker] + conf.write(six.b("""[broker] urls = amqps://broker1.example.com:5671 amqps://broker2.example.com:5671 cert = /etc/koji-hub/plugins/client.pem cacert = /etc/koji-hub/plugins/ca.pem topic_prefix = koji connect_timeout = 10 send_timeout = 60 -""") +""")) conf.flush() protonmsg.CONFIG_FILE = conf.name protonmsg.CONFIG = None @@ -211,14 +211,14 @@ send_timeout = 60 def test_send_queued_msgs_success(self, getLogger, Container): context.protonmsg_msgs = [('test.topic', {'testheader': 1}, 'test body')] conf = tempfile.NamedTemporaryFile() - conf.write("""[broker] + conf.write(six.b("""[broker] urls = amqps://broker1.example.com:5671 amqps://broker2.example.com:5671 cert = /etc/koji-hub/plugins/client.pem cacert = /etc/koji-hub/plugins/ca.pem topic_prefix = koji connect_timeout = 10 send_timeout = 60 -""") +""")) conf.flush() protonmsg.CONFIG_FILE = conf.name protonmsg.CONFIG = None @@ -236,7 +236,7 @@ send_timeout = 60 def test_send_queued_msgs_test_mode(self, getLogger, Container): context.protonmsg_msgs = [('test.topic', {'testheader': 1}, 'test body')] conf = tempfile.NamedTemporaryFile() - conf.write("""[broker] + conf.write(six.b("""[broker] urls = amqps://broker1.example.com:5671 amqps://broker2.example.com:5671 cert = /etc/koji-hub/plugins/client.pem cacert = /etc/koji-hub/plugins/ca.pem @@ -244,7 +244,7 @@ topic_prefix = koji connect_timeout = 10 send_timeout = 60 test_mode = on -""") +""")) conf.flush() protonmsg.CONFIG_FILE = conf.name protonmsg.CONFIG = None diff --git a/tests/test_plugins/test_runroot_builder.py b/tests/test_plugins/test_runroot_builder.py index 817c8db..42585e1 100644 --- a/tests/test_plugins/test_runroot_builder.py +++ b/tests/test_plugins/test_runroot_builder.py @@ -96,7 +96,7 @@ class FakeConfigParser(object): class TestRunrootConfig(unittest.TestCase): - @mock.patch('ConfigParser.SafeConfigParser') + @mock.patch('six.moves.configparser.SafeConfigParser') def test_bad_config_paths0(self, safe_config_parser): cp = FakeConfigParser() del cp.CONFIG['path0']['mountpoint'] @@ -109,7 +109,7 @@ class TestRunrootConfig(unittest.TestCase): self.assertEqual(cm.exception.args[0], "bad config: missing options in path0 section") - @mock.patch('ConfigParser.SafeConfigParser') + @mock.patch('six.moves.configparser.SafeConfigParser') def test_bad_config_absolute_path(self, safe_config_parser): cp = FakeConfigParser() cp.CONFIG['paths']['default_mounts'] = '' @@ -122,7 +122,7 @@ class TestRunrootConfig(unittest.TestCase): self.assertEqual(cm.exception.args[0], "bad config: all paths (default_mounts, safe_roots, path_subs) needs to be absolute: ") - @mock.patch('ConfigParser.SafeConfigParser') + @mock.patch('six.moves.configparser.SafeConfigParser') def test_valid_config(self, safe_config_parser): safe_config_parser.return_value = FakeConfigParser() session = mock.MagicMock() @@ -130,7 +130,7 @@ class TestRunrootConfig(unittest.TestCase): options.workdir = '/tmp/nonexistentdirectory' runroot.RunRootTask(123, 'runroot', {}, session, options) - @mock.patch('ConfigParser.SafeConfigParser') + @mock.patch('six.moves.configparser.SafeConfigParser') def test_valid_config_alt(self, safe_config_parser): safe_config_parser.return_value = FakeConfigParser(CONFIG2) session = mock.MagicMock() @@ -138,7 +138,7 @@ class TestRunrootConfig(unittest.TestCase): options.workdir = '/tmp/nonexistentdirectory' runroot.RunRootTask(123, 'runroot', {}, session, options) - @mock.patch('ConfigParser.SafeConfigParser') + @mock.patch('six.moves.configparser.SafeConfigParser') def test_pathnum_gaps(self, safe_config_parser): session = mock.MagicMock() options = mock.MagicMock() @@ -159,7 +159,7 @@ class TestRunrootConfig(unittest.TestCase): paths = list([CONFIG2[k] for k in ('path0', 'path1', 'path2')]) self.assertEqual(task2.config['paths'], paths) - @mock.patch('ConfigParser.SafeConfigParser') + @mock.patch('six.moves.configparser.SafeConfigParser') def test_bad_path_sub(self, safe_config_parser): session = mock.MagicMock() options = mock.MagicMock() @@ -172,7 +172,7 @@ class TestRunrootConfig(unittest.TestCase): class TestMounts(unittest.TestCase): - @mock.patch('ConfigParser.SafeConfigParser') + @mock.patch('six.moves.configparser.SafeConfigParser') def setUp(self, safe_config_parser): safe_config_parser.return_value = FakeConfigParser() self.session = mock.MagicMock() @@ -324,7 +324,7 @@ class TestMounts(unittest.TestCase): os_unlink.assert_not_called() class TestHandler(unittest.TestCase): - @mock.patch('ConfigParser.SafeConfigParser') + @mock.patch('six.moves.configparser.SafeConfigParser') def setUp(self, safe_config_parser): self.session = mock.MagicMock() self.br = mock.MagicMock() From 282e0a61ca4ed40a7d5c46b74ebc72f741877ce5 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Feb 19 2019 13:05:15 +0000 Subject: [PATCH 7/7] add PYTHONPATH for web tests --- diff --git a/Makefile b/Makefile index ffd1b88..e3bc37c 100644 --- a/Makefile +++ b/Makefile @@ -78,7 +78,7 @@ test: test3: coverage3 erase - PYTHONPATH=hub/.:plugins/hub/.:plugins/builder/.:plugins/cli/.:cli/. coverage3 run \ + PYTHONPATH=hub/.:plugins/hub/.:plugins/builder/.:plugins/cli/.:cli/.:www/lib coverage3 run \ --rcfile .coveragerc3 --source . \ /usr/bin/nosetests-3 coverage3 report --rcfile .coveragerc3