Fix Python 3 testcases [CLOUDDST-27]

After the automatically detected problems were fixed, the following issues had to be resolved to ensure that the whole test suite passes in Python 3: - in testcase 'test_get_state_with_error_other_than_enoent', '__builtin__' was changed to 'six.moves.builtins' because the built-in library was renamed in Python 3. Also, the testcase itself had been broken because it had raised an IOError due to an incorrect setup (file was assigned as a directory) and not because of the patched 'open' function. It was fixed despite incidentally fulfilling its desired function in order to prevent user confusion. - In Python 3, output for invoked bash commands is a bytes type, which can cause many issues because the code expects to work with a string. Adding the 'universal_newlines' forces the output to be returned as a string. Also, print statements were changed to logger messages.
release-engineering · Apr 16, 2020 · a9098f8 · a9098f8
1 parent cf1fb7a
commit a9098f8
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 94 deletions.
diff --git a/alt_src/alt_src.py b/alt_src/alt_src.py
@@ -2,12 +2,8 @@
 '''
 Given an srpm and product, stage for alt-src release
 '''
-from __future__ import print_function
 
-import six
-from six.moves import configparser
 import copy
-from six.moves import cStringIO as StringIO
 import datetime
 import errno
 import fcntl
@@ -19,15 +15,18 @@
 import os.path
 import re
 import shutil
+import simplejson as json
+import six
+from six.moves import configparser
+from six.moves import cStringIO as StringIO
+from six.moves.urllib.parse import urlencode
+from six.moves.urllib.request import Request, urlopen
 import smtplib
 import subprocess
 import sys
 import time
 import traceback
-from six.moves.urllib.parse import urlencode
-from six.moves.urllib.request import Request, urlopen
 import yaml
-import simplejson as json
 
 import koji
 import rpm
@@ -206,7 +205,7 @@ def setup_logfile(self, logname):
             if ntimes > 1024:
                 raise SanityError("Too many log backups")
             fname = "%s.%d" % (fname_, ntimes)
-        self.logfile = file(fname, 'w')
+        self.logfile = open(fname, 'w')
         handler = logging.StreamHandler(self.logfile)
         handler.setFormatter(logging.Formatter(self.options.config['log_format']))
         handler.setLevel(self.options.file_log_level)
@@ -265,13 +264,13 @@ def get_output(self, cmd, fatal=True, **kwargs):
         if 'stderr' in kwargs:
             # convenience values
             if kwargs['stderr'] == 'null':
-                kwargs['stderr'] = file('/dev/null', 'w')
+                kwargs['stderr'] = open('/dev/null', 'w')
             elif kwargs['stderr'] == 'keep':
                 kwargs['stderr'] = subprocess.STDOUT
         elif self.logfile:
             self.logfile.flush()
             kwargs['stderr'] = self.logfile
-        proc = subprocess.Popen(cmd, **kwargs)
+        proc = subprocess.Popen(cmd, universal_newlines=True, **kwargs)
         output = proc.communicate()[0]
         self.logger.debug("Command output was:\n%s", output)
         retval = proc.wait()
@@ -377,7 +376,7 @@ def check_package(self):
             self.logger.debug("Got blacklist: %r", blacklist)
             if blacklist and koji.util.multi_fnmatch(self.package, blacklist):
                 # raise FilterError, 'Blacklisted package: %s' % self.package
-                print('Blacklisted package: %s, quitting' % self.package)
+                self.logger.info('Blacklisted package: %s, quitting' % self.package)
                 sys.exit(0)
 
     def git_push_url(self):
@@ -805,15 +804,15 @@ def init_new_repo(self):
         cmd = ['git', 'clone', '--bare', initdir, "repo_init.git"]
         self.log_cmd(cmd, cwd=self.workdir)
         descfile = os.path.join(self.workdir, "repo_init.git", "description")
-        fobj = file(descfile, 'w')
+        fobj = open(descfile, 'w')
         fobj.write(self.summary)
         fobj.write('\n')
         fobj.close()
         if not self.check_push_to_pagure():
             # add gitblit options to git config
             # XXX this content should not be hard coded
             git_config = os.path.join(self.workdir, "repo_init.git", "config")
-            fobj = file(git_config, 'a')
+            fobj = open(git_config, 'a')
             params = {
                 'summary' : self.summary,
                 'package' : self.package,
@@ -950,8 +949,8 @@ def import_sources(self):
         to_move.sort()
 
         # move files to lookaside
-        meta = file(os.path.join(dst, ".%s.metadata" % self.package), 'w')
-        gitignore = file(os.path.join(dst, ".gitignore"), 'w')
+        meta = open(os.path.join(dst, ".%s.metadata" % self.package), 'w')
+        gitignore = open(os.path.join(dst, ".gitignore"), 'w')
         for fname in to_move:
             path = os.path.join(sourcedir, fname)
             digest = self.get_digest(path)
@@ -997,7 +996,7 @@ def get_digest(self, path):
         """Calculate hex digest for file"""
 
         csum = hashlib.sha1()
-        fobj = file(path, 'rb')
+        fobj = open(path, 'rb')
         chunk = 'IGNORE ME!'
         while chunk:
             chunk = fobj.read(8192)
@@ -1118,8 +1117,8 @@ def debrand(self):
                 if self.for_lookaside(path):
                     for_lookaside.append(fname)
         if for_lookaside:
-            meta = file(os.path.join(self.checkout, ".%s.metadata" % self.package), 'a')
-            gitignore = file(os.path.join(self.checkout, ".gitignore"), 'a')
+            meta = open(os.path.join(self.checkout, ".%s.metadata" % self.package), 'a')
+            gitignore = open(os.path.join(self.checkout, ".gitignore"), 'a')
             for fname in for_lookaside:
                 path = os.path.join(self.checkout, fname)
                 digest = self.get_digest(path)
@@ -1167,7 +1166,7 @@ def handle_debrand_fail(self):
 
         self.logger.warning("Adding debranding failure notice")
         fname = os.path.join(self.checkout, "README.debrand")
-        fobj = file(fname, 'w')
+        fobj = open(fname, 'w')
         fobj.write('''\
 Warning: This package was configured for automatic debranding, but the changes
 failed to apply.
@@ -1202,7 +1201,7 @@ def prep_changelog(self, notes):
                         parts.append('- %s\n' % line)
                     else:
                         parts.append('-  %s\n' % line)
-        fobj = file(os.path.join(self.workdir, 'changelog.txt'), 'w')
+        fobj = open(os.path.join(self.workdir, 'changelog.txt'), 'w')
         for part in parts:
             fobj.write(part)
             self.logger.debug("%s", part)
@@ -1279,15 +1278,15 @@ def rule_handler_re(self, data):
             fname = os.path.join(self.checkout, data['file'])
         else:
             fname = self.find_spec()
-        fobj = file(fname, 'r')
+        fobj = open(fname, 'r')
         text = fobj.read()
         fobj.close()
         count = int(data.get('count', '0'))
         if count:
             text = re.sub(data['match'], data['replace'], text, count)
         else:
             text = re.sub(data['match'], data['replace'], text)
-        fobj = file(fname, 'w')
+        fobj = open(fname, 'w')
         fobj.write(text)
         fobj.close()
 
@@ -1329,7 +1328,7 @@ def handle_re_line(self, data):
         else:
             fname = self.find_spec()
         self.logger.info('Applying regex substitutions to %s', fname)
-        fobj = file(fname, 'r')
+        fobj = open(fname, 'r')
         lines = fobj.readlines()
         fobj.close()
         prog = re.compile(data['match'])
@@ -1353,7 +1352,7 @@ def handle_re_line(self, data):
         else:
             self.logger.error('No matches for pattern %r', prog.pattern)
         # write it back out
-        fobj = file(fname, 'w')
+        fobj = open(fname, 'w')
         fobj.writelines(lines)
         fobj.close()
 
@@ -1376,7 +1375,7 @@ def handle_add_patch(self, data):
         patchstrip = int(data.get('strip', '1'))
         self.logger.debug("Adding patch: %r", data)
         specfile = self.find_spec()
-        fobj = file(specfile, 'r')
+        fobj = open(specfile, 'r')
         lines = fobj.readlines()
         fobj.close()
         # find highest patch number and last patch line location
@@ -1468,7 +1467,7 @@ def handle_add_patch(self, data):
                 raise SanityError("Unable to apply patch %s" % patchname)
 
         # write it back out
-        fobj = file(specfile, 'w')
+        fobj = open(specfile, 'w')
         fobj.writelines(lines)
         fobj.close()
 
@@ -1498,7 +1497,7 @@ def handle_rm_patch(self, data):
             raise SanityError('Invalid rule')
 
         specfile = self.find_spec()
-        fobj = file(specfile, 'r')
+        fobj = open(specfile, 'r')
         lines = fobj.readlines()
         fobj.close()
 
@@ -1536,7 +1535,7 @@ def handle_rm_patch(self, data):
                 raise SanityError("Unable to remove patch")
 
         # write it back out
-        fobj = file(specfile, 'w')
+        fobj = open(specfile, 'w')
         fobj.writelines(lines)
         fobj.close()
 
@@ -1560,7 +1559,7 @@ def handle_add_source(self, data):
         sourcefile = os.path.join(self.options.config['rulesdir'], data['source'])
         sourcename = os.path.basename(sourcefile)
         specfile = self.find_spec()
-        fobj = file(specfile, 'r')
+        fobj = open(specfile, 'r')
         lines = fobj.readlines()
         fobj.close()
 
@@ -1598,7 +1597,7 @@ def handle_add_source(self, data):
         self.copy_new_source(sourcefile)
 
         # write it back out
-        fobj = file(specfile, 'w')
+        fobj = open(specfile, 'w')
         fobj.writelines(lines)
         fobj.close()
 
@@ -1608,7 +1607,7 @@ def handle_replace_source(self, data):
         sourcefile = os.path.join(self.options.config['rulesdir'], data['source'])
         sourcename = os.path.basename(sourcefile)
         specfile = self.find_spec()
-        fobj = file(specfile, 'r')
+        fobj = open(specfile, 'r')
         lines = fobj.readlines()
         fobj.close()
 
@@ -1639,7 +1638,7 @@ def handle_replace_source(self, data):
         #TODO - option to remove old
 
         # write it back out
-        fobj = file(specfile, 'w')
+        fobj = open(specfile, 'w')
         fobj.writelines(lines)
         fobj.close()
 
@@ -1732,7 +1731,7 @@ def add_changelog(self):
         self.log_cmd(['git', 'checkout', stage_branch], cwd=self.checkout)
 
         # get the changelog entry
-        fobj = file(fname, 'r')
+        fobj = open(fname, 'r')
         clog = fobj.read()
         now = datetime.datetime.now().strftime('%a %b %d %Y')
         if clog.find('INSERT_DATE_HERE') == -1:
@@ -1743,7 +1742,7 @@ def add_changelog(self):
         # insert the entry into spec
         spec = self.find_spec()
         prog = re.compile(r'^(\s*%changelog.*)$', re.MULTILINE)
-        inf = file(spec, 'r')
+        inf = open(spec, 'r')
         parts = prog.split(inf.read())
         inf.close()
         if len(parts) == 1:
@@ -1752,7 +1751,7 @@ def add_changelog(self):
         elif len(parts) == 2:
             # should not be possible
             raise SanityError('Unable to split changelog from spec')
-        outf = file(spec, 'w')
+        outf = open(spec, 'w')
         for part in parts[:2]:
             outf.write(part)
         outf.write('\n')
@@ -1924,7 +1923,7 @@ def init_pagure_remote_repo(self):
 
 
     def push_lookaside(self):
-        meta = file(os.path.join(self.checkout, ".%s.metadata" % self.package), 'r')
+        meta = open(os.path.join(self.checkout, ".%s.metadata" % self.package), 'r')
         for line in meta.readlines():
             line = line.strip()
             digest, _ = line.split(None, 1)
@@ -2005,7 +2004,7 @@ def wipe_git_dir(dirname):
 
 
 def die(msg):
-    print(msg)
+    self.logger.error(msg)
     sys.exit(1)
 
 

diff --git a/setup.py b/setup.py
@@ -37,6 +37,7 @@ def get_long_description():
             "Programming Language :: Python :: 2.4",
             "Programming Language :: Python :: 2.6",
             "Programming Language :: Python :: 2.7",
+            "Programming Language :: Python :: 3",
             "License :: OSI Approved :: GNU General Public License v3 or later (GPLv3+)",
       ],
       install_requires=get_requirements(),

diff --git a/tests/test_debrand.py b/tests/test_debrand.py
@@ -8,12 +8,7 @@
 import pytest
 import yaml
 
-
-# ensure python2 before attempting to import sources
-if sys.version_info < (3, 0):
-    from alt_src.alt_src import Stager
-
-xfail = pytest.mark.xfail(sys.version_info >= (3, 0), reason='Incompatible with python3')
+from alt_src.alt_src import Stager
 
 TESTS_PATH = os.path.dirname(__file__)
 MODULES_PATH = os.path.join(TESTS_PATH, 'data', 'module_source')
@@ -294,7 +289,6 @@ def spec_file(request, checkout_dir):
     yield fname
 
 
-@xfail(strict=True)
 @pytest.fixture
 def stager_setup(request, read_source, rule_cfg, options, checkout_dir,
                  spec_file, work_dir, rules_dir):
@@ -323,7 +317,6 @@ def stager_setup_mmd_params(modname, branch, rule_cfg):
             {'fname': modname})
 
 
-@xfail(strict=True)
 @pytest.mark.parametrize(
     'stager_setup,options,rule_cfg,spec_file,expected',
     [stager_setup_params('foo-package', 'test-b', spec_rule_config) +
@@ -349,7 +342,6 @@ def test_rule_spec(stager_setup, expected):
     assert stager.log_cmd.mock_calls[0] == call(expected_cmd, cwd=stager.checkout)
 
 
-@xfail(strict=True)
 @pytest.mark.parametrize(
     'stager_setup,options,rule_cfg,spec_file,expected',
     [stager_setup_params('foo-package', 'test-b', re_rule_config) +
@@ -375,7 +367,6 @@ def test_rule_re(stager_setup, expected):
             print(spec_words)
             assert spec_words.count(word) == count
 
-@xfail(strict=True)
 @pytest.mark.parametrize('stager_setup,options,rule_cfg,spec_file,expected',
                          [stager_setup_params('foo-package', 'test-b', patch_rule_config_add) +
                           (contains_string('Patch2: foo.patch'),),
@@ -401,7 +392,6 @@ def test_rule_patch(stager_setup, expected):
             assert_that(spec, expected)
 
 
-@xfail(strict=True)
 @pytest.mark.parametrize(
     'stager_setup,options,rule_cfg,spec_file,expected',
     [stager_setup_params('foo-package', 'test-b', source_rule_config_add) +
@@ -428,7 +418,6 @@ def test_rule_source(stager_setup, expected):
             assert_that(spec, expected)
 
 
-@xfail(strict=True)
 @pytest.mark.parametrize('stager_setup,options,rule_cfg,spec_file,expected',
                          [stager_setup_params('foo-package', 'test-b', script_rule_config) +
                           (("{rules_dir}/some-script {checkout_dir} {spec_file}",),),
@@ -449,7 +438,6 @@ def test_rule_script(stager_setup, expected):
             expected_cmd.append(e)
     assert stager.log_cmd.mock_calls[0] == call(expected_cmd, cwd=stager.checkout)
 
-@xfail(strict=True)
 @pytest.mark.parametrize('stager_setup,options,rule_cfg,spec_file,expected',
                          [stager_setup_mmd_params('postgresql', 'test-b', mmd_rule_config) +
                           (contains_string('ref: stream-centos-9.6'),),
@@ -473,7 +461,6 @@ def test_rule_mmd(stager_setup, expected):
         assert_that(mmd, expected)
 
 
-@xfail(strict=True)
 @pytest.mark.parametrize('stager_setup,options,rule_cfg,spec_file',
                          [stager_setup_mmd_params('postgresql', 'test-b', mmd_rule_config2)],
                          ids=['mmd-replace-no-change'],