From d03dda9d22f08156dc008ba72c1f5e476c9a78f7 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
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 <tkopecek@redhat.com>
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 <tkopecek@redhat.com>
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 <tkopecek@redhat.com>
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 <tkopecek@redhat.com>
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 <tkopecek@redhat.com>
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 <tkopecek@redhat.com>
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

