#1265 py3 tests + related fixes
Merged 4 years ago by mikem. Opened 4 years ago by tkopecek.
tkopecek/koji issue1264  into  master

file modified
-1
@@ -5,7 +5,6 @@ 

  omit =

      /usr/*

      tests/*

-     util/*

  

  [report]

  exclude_lines =

file modified
+4 -5
@@ -78,12 +78,11 @@ 

  

  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 \

- 	        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:

file modified
+16 -4
@@ -343,6 +343,21 @@ 

      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 @@ 

                                  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:

file modified
+2 -5
@@ -5,7 +5,7 @@ 

  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 @@ 

                  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)

@@ -6,7 +6,9 @@ 

  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:

@@ -2,7 +2,6 @@ 

  import json

  import mock

  import os

- import smtplib

  import tempfile

  try:

      import unittest2 as unittest
@@ -25,7 +24,7 @@ 

          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)

Why is this commented out?

          for call in data:

              key = self._munge([call['method'], call['args'], call['kwargs']])

              self._testcalls[key] = call
@@ -101,6 +100,6 @@ 

          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)

@@ -84,7 +84,7 @@ 

  

      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)

@@ -1,5 +1,6 @@ 

  from __future__ import absolute_import

  import os

+ import six

  import subprocess

  try:

      import unittest2 as unittest
@@ -27,6 +28,8 @@ 

          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):

@@ -19,10 +19,10 @@ 

          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)

@@ -187,14 +187,14 @@ 

      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 @@ 

      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 @@ 

      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 @@ 

  connect_timeout = 10

  send_timeout = 60

  test_mode = on

- """)

+ """))

          conf.flush()

          protonmsg.CONFIG_FILE = conf.name

          protonmsg.CONFIG = None

@@ -96,7 +96,7 @@ 

  

  

  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 @@ 

          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 @@ 

          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 @@ 

          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 @@ 

          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 @@ 

          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 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 @@ 

          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()

file modified
+1 -1
@@ -209,7 +209,7 @@ 

          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

file modified
-3
@@ -669,9 +669,6 @@ 

              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

no initial comment

1 new commit added

  • add PYTHONPATH for web tests
4 years ago

Why is this commented out?

To be honest - I was not able to figure out, why it is there. json should store calls to hub and return values. Values in json don't need to be converted at all (no change of data in py2). If it stays there, even keys in dict are bytes in py3 and call['method'] should be call[six.b('method')], etc.

It is originally written by @mikem - Mike, any concerns with this?

It is originally written by @mikem - Mike, any concerns with this?

I don't recall precisely. This was from #628, and even right after that merge, the line appears to be unnecessary,

My best guess is that it is an artifact of an earlier version of the test. I think I played around with different ways to store the cached calls before setting on json. Unfortunately, I dropped my original branch some time after the merge, so I cannot see the reflog for it anymore.

Commit 4f45cce fixes this pull-request

Pull-Request has been merged by mikem

4 years ago