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.

- In Python 3, the bash command output is a bytes type, which can
  cause many issues since the code expects to work with a string.
  It was solved by decoding it as a UTF-8. This encoding was chosen
  because it is the most likely to be used by the invoked shell.
  The decoding is only done on Python 3 because the type would
  be changed to 'unicode' in Python 2, which is not desired either.

alt_src/alt_src.py

@@ -206,7 +206,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,14 +265,18 @@ 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)
         output = proc.communicate()[0]
+        # we'll decode it only if it's bytes type (not a PY2 'str' type)
+        if not isinstance(output, str) and isinstance(output, bytes):
+            # we'll assume that bash uses utf-8 encoding
+            output = output.decode('utf-8')
         self.logger.debug("Command output was:\n%s", output)
         retval = proc.wait()
         if retval:
@@ -805,15 +809,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 +954,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 +1001,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 +1122,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 +1171,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 +1206,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 +1283,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 +1333,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 +1357,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 +1380,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 +1472,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 +1502,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 +1540,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 +1564,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 +1602,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 +1612,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 +1643,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 +1736,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 +1747,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 +1756,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 +1928,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)

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'],