From 03255001d6a44313cf1dd4b206b766b673bbda00 Mon Sep 17 00:00:00 2001
From: Paul Morgan <jumanjiman@gmail.com>
Date: Sat, 15 Feb 2014 23:19:29 +0000
Subject: [PATCH] add unit test and module compatibility lib

Add modules compat lib to support both python 2 and 3, and
add unit test to detect regressions.

Tito uses two modules that have changed significantly between
python 2 and 3:

* commands module merged into subprocess module
* ConfigParser module renamed as configparser
---
 src/tito/builder/main.py              |  7 ++--
 src/tito/cli.py                       | 23 ++++++-------
 src/tito/common.py                    |  8 ++---
 src/tito/compat.py                    | 49 +++++++++++++++++++++++++++
 src/tito/distributionbuilder.py       |  4 +--
 src/tito/obs.py                       |  8 ++---
 src/tito/release.py                   | 18 +++++-----
 src/tito/tagger.py                    |  4 +--
 test/functional/fetch_tests.py        |  4 +--
 test/functional/release_yum_tests.py  |  5 +--
 test/unit/fixture.py                  |  2 ++
 test/unit/pep8-tests.py               |  5 +++
 test/unit/test_build_target_parser.py |  2 +-
 13 files changed, 97 insertions(+), 42 deletions(-)
 create mode 100644 src/tito/compat.py

diff --git a/src/tito/builder/main.py b/src/tito/builder/main.py
index 68c3d0e..e482db9 100644
--- a/src/tito/builder/main.py
+++ b/src/tito/builder/main.py
@@ -19,7 +19,6 @@ and rpms.
 import os
 import sys
 import re
-import commands
 from pkg_resources import require
 from distutils.version import LooseVersion as loose_version
 from tempfile import mkdtemp
@@ -685,7 +684,7 @@ class CvsBuilder(NoTgzBuilder):
         if not reuse_cvs_checkout:
             self._verify_cvs_module_not_already_checked_out()
 
-        commands.getoutput("mkdir -p %s" % self.cvs_workdir)
+        getoutput("mkdir -p %s" % self.cvs_workdir)
         cvs_releaser = CvsReleaser(self)
         cvs_releaser.cvs_checkout_module()
         cvs_releaser.cvs_verify_branches_exist()
@@ -879,7 +878,7 @@ class UpstreamBuilder(NoTgzBuilder):
         debug("Generating patch with: %s" % patch_command)
         output = run_command(patch_command)
         print(output)
-        (status, output) = commands.getstatusoutput(
+        (status, output) = getstatusoutput(
             "grep 'Binary files .* differ' %s " % patch_file)
         if status == 0 and output != "":
             error_out("You are doomed. Diff contains binary files. You can not use this builder")
@@ -915,7 +914,7 @@ class UpstreamBuilder(NoTgzBuilder):
         with just the package release being incremented on rebuilds.
         """
         # Use upstreamversion if defined in the spec file:
-        (status, output) = commands.getstatusoutput(
+        (status, output) = getstatusoutput(
             "cat %s | grep 'define upstreamversion' | "
             "awk '{ print $3 ; exit }'" % self.spec_file)
         if status == 0 and output != "":
diff --git a/src/tito/cli.py b/src/tito/cli.py
index c2b06f9..c731365 100644
--- a/src/tito/cli.py
+++ b/src/tito/cli.py
@@ -17,12 +17,11 @@ Tito's Command Line Interface
 import sys
 import os
 import random
-import commands
-import ConfigParser
 
 from optparse import OptionParser
 
 from tito.common import *
+from tito.compat import *
 from tito.exception import *
 
 # Hack for Python 2.4, seems to require we import these so they get compiled
@@ -195,7 +194,7 @@ class BaseCliModule(object):
         # Load the global config. Later, when we know what tag/package we're
         # building, we may also load that and potentially override some global
         # settings.
-        config = ConfigParser.ConfigParser()
+        config = ConfigParser()
         config.read(filename)
 
         # Verify the config contains what we need from it:
@@ -262,13 +261,13 @@ class BaseCliModule(object):
             cmd = "git show %s:%s%s" % (tag, relative_dir,
                     BUILD_PROPS_FILENAME)
             debug(cmd)
-            (status, output) = commands.getstatusoutput(cmd)
+            (status, output) = getstatusoutput(cmd)
             if status > 0:
                 # Give it another try looking for legacy props filename:
                 cmd = "git show %s:%s%s" % (tag, relative_dir,
                         "build.py.props")
                 debug(cmd)
-                (status, output) = commands.getstatusoutput(cmd)
+                (status, output) = getstatusoutput(cmd)
 
             temp_filename = "%s-%s" % (random.randint(1, 10000),
                     BUILD_PROPS_FILENAME)
@@ -287,7 +286,7 @@ class BaseCliModule(object):
                 cmd = "git show %s:%s%s | grep NO_TAR_GZ" % \
                     (tag, relative_dir, "Makefile")
                 debug(cmd)
-                (status, output) = commands.getstatusoutput(cmd)
+                (status, output) = getstatusoutput(cmd)
                 if status == 0 and output != "":
                     properties_file = temp_props_file
                     debug("Found Makefile with NO_TAR_GZ")
@@ -507,7 +506,7 @@ class ReleaseModule(BaseCliModule):
         """
         rel_eng_dir = os.path.join(find_git_root(), "rel-eng")
         filename = os.path.join(rel_eng_dir, RELEASERS_CONF_FILENAME)
-        config = ConfigParser.ConfigParser()
+        config = ConfigParser()
         config.read(filename)
         return config
 
@@ -736,7 +735,7 @@ class InitModule(BaseCliModule):
         propsfile = os.path.join(rel_eng_dir, GLOBAL_BUILD_PROPS_FILENAME)
         if not os.path.exists(propsfile):
             if not os.path.exists(rel_eng_dir):
-                commands.getoutput("mkdir -p %s" % rel_eng_dir)
+                getoutput("mkdir -p %s" % rel_eng_dir)
                 print("   - created %s" % rel_eng_dir)
 
             # write out tito.props
@@ -750,7 +749,7 @@ class InitModule(BaseCliModule):
             out_f.close()
             print("   - wrote %s" % GLOBAL_BUILD_PROPS_FILENAME)
 
-            commands.getoutput('git add %s' % propsfile)
+            getoutput('git add %s' % propsfile)
             should_commit = True
 
         # prep the packages metadata directory
@@ -759,7 +758,7 @@ class InitModule(BaseCliModule):
 
         if not os.path.exists(readme):
             if not os.path.exists(pkg_dir):
-                commands.getoutput("mkdir -p %s" % pkg_dir)
+                getoutput("mkdir -p %s" % pkg_dir)
                 print("   - created %s" % pkg_dir)
 
             # write out readme file explaining what pkg_dir is for
@@ -771,11 +770,11 @@ class InitModule(BaseCliModule):
             out_f.close()
             print("   - wrote %s" % readme)
 
-            commands.getoutput('git add %s' % readme)
+            getoutput('git add %s' % readme)
             should_commit = True
 
         if should_commit:
-            commands.getoutput('git commit -m "Initialized to use tito. "')
+            getoutput('git commit -m "Initialized to use tito. "')
             print("   - committed to git")
 
         print("Done!")
diff --git a/src/tito/common.py b/src/tito/common.py
index bbb6f0f..5a51d07 100644
--- a/src/tito/common.py
+++ b/src/tito/common.py
@@ -17,9 +17,9 @@ Common operations.
 import os
 import re
 import sys
-import commands
 import traceback
 
+from tito.compat import *
 from tito.exception import RunCommandException
 
 DEFAULT_BUILD_DIR = "/tmp/tito"
@@ -194,7 +194,7 @@ def find_git_root():
 
     Returned as a full path.
     """
-    (status, cdup) = commands.getstatusoutput("git rev-parse --show-cdup")
+    (status, cdup) = getstatusoutput("git rev-parse --show-cdup")
     if status > 0:
         error_out(["%s does not appear to be within a git checkout." %
                 os.getcwd()])
@@ -219,12 +219,12 @@ def run_command(command, print_on_success=False):
     If print_on_success is True, print status and output even
     when command succeeds.
     """
-    (status, output) = commands.getstatusoutput(command)
+    (status, output) = getstatusoutput(command)
     return output
 
 
 def tag_exists_locally(tag):
-    (status, output) = commands.getstatusoutput("git tag | grep %s" % tag)
+    (status, output) = getstatusoutput("git tag | grep %s" % tag)
     if status > 0:
         return False
     else:
diff --git a/src/tito/compat.py b/src/tito/compat.py
new file mode 100644
index 0000000..ca8a15c
--- /dev/null
+++ b/src/tito/compat.py
@@ -0,0 +1,49 @@
+# Copyright (c) 2008-2010 Red Hat, Inc.
+#
+# This software is licensed to you under the GNU General Public License,
+# version 2 (GPLv2). There is NO WARRANTY for this software, express or
+# implied, including the implied warranties of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+# along with this software; if not, see
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+#
+# Red Hat trademarks are not licensed under GPLv2. No permission is
+# granted to use or replicate Red Hat trademarks that are incorporated
+# in this software or its documentation.
+"""
+Compatibility library for Python 2.4 up through Python 3.
+"""
+import sys
+PY2 = sys.version_info[0] == 2
+if PY2:
+    import commands
+    from ConfigParser import ConfigParser
+    from ConfigParser import NoOptionError
+    from ConfigParser import RawConfigParser
+else:
+    import subprocess
+    from configparser import ConfigParser
+    from configparser import NoOptionError
+    from configparser import RawConfigParser
+
+
+def getstatusoutput(cmd):
+    """
+    Returns (status, output) of executing cmd in a shell.
+    Supports Python 2.4 and 3.x.
+    """
+    if PY2:
+        return commands.getstatusoutput(cmd)
+    else:
+        return subprocess.getstatusoutput(cmd)
+
+
+def getoutput(cmd):
+    """
+    Returns output of executing cmd in a shell.
+    Supports Python 2.4 and 3.x.
+    """
+    if PY2:
+        return commands.getoutput(cmd)
+    else:
+        return subprocess.getoutput(cmd)
diff --git a/src/tito/distributionbuilder.py b/src/tito/distributionbuilder.py
index 53df3f4..d5d9834 100644
--- a/src/tito/distributionbuilder.py
+++ b/src/tito/distributionbuilder.py
@@ -2,7 +2,7 @@ import os
 
 from tito.builder import UpstreamBuilder
 from tito.common import debug, run_command, error_out
-import commands
+from tito.compat import *
 
 
 class DistributionBuilder(UpstreamBuilder):
@@ -32,7 +32,7 @@ class DistributionBuilder(UpstreamBuilder):
                % (self.rpmbuild_gitcopy, self.project_name, self.upstream_version, self.build_version, self.git_commit_id))
         self.patch_files = output.split("\n")
         for p_file in self.patch_files:
-            (status, output) = commands.getstatusoutput(
+            (status, output) = getstatusoutput(
                 "grep 'Binary files .* differ' %s/%s " % (self.rpmbuild_gitcopy, p_file))
             if status == 0 and output != "":
                 error_out("You are doomed. Diff contains binary files. You can not use this builder")
diff --git a/src/tito/obs.py b/src/tito/obs.py
index df8e1c1..0ced1c7 100644
--- a/src/tito/obs.py
+++ b/src/tito/obs.py
@@ -12,12 +12,12 @@
 # in this software or its documentation.
 
 import os
-import commands
 import tempfile
 import subprocess
 import sys
 
 from tito.common import run_command, debug, extract_bzs
+from tito.compat import *
 from tito.release import Releaser
 
 
@@ -49,7 +49,7 @@ class ObsReleaser(Releaser):
         self.dry_run = dry_run
         self.no_build = no_build
 
-        commands.getoutput("mkdir -p %s" % self.working_dir)
+        getoutput("mkdir -p %s" % self.working_dir)
         os.chdir(self.working_dir)
         run_command("%s co %s %s" % (self.cli_tool, self.obs_project_name, self.obs_package_name))
 
@@ -109,7 +109,7 @@ class ObsReleaser(Releaser):
 
         os.chdir(project_checkout)
 
-        (status, diff_output) = commands.getstatusoutput("%s diff" % self.cli_tool)
+        (status, diff_output) = getstatusoutput("%s diff" % self.cli_tool)
 
         if diff_output.strip() == "":
             print("No changes in main branch, skipping commit.")
@@ -138,7 +138,7 @@ class ObsReleaser(Releaser):
             os.unlink(commit_msg_file)
 
         if self.no_build:
-            commands.getstatusoutput("%s abortbuild %s %s" % (
+            getstatusoutput("%s abortbuild %s %s" % (
                 self.cli_tool, self.obs_project_name, self.obs_package_name))
             print("Aborting automatic rebuild because --no-build has been specified.")
 
diff --git a/src/tito/release.py b/src/tito/release.py
index 8c49599..797b661 100644
--- a/src/tito/release.py
+++ b/src/tito/release.py
@@ -17,7 +17,6 @@ Code for submitting builds for release.
 import copy
 import os
 import sys
-import commands
 import tempfile
 import subprocess
 import rpm
@@ -26,6 +25,7 @@ from tempfile import mkdtemp
 import shutil
 
 from tito.common import *
+from tito.compat import *
 from tito.buildparser import BuildTargetParser
 from tito.exception import TitoException
 from tito.config_object import ConfigObject
@@ -522,7 +522,7 @@ class FedoraGitReleaser(Releaser):
 
     def _git_release(self):
 
-        commands.getoutput("mkdir -p %s" % self.working_dir)
+        getoutput("mkdir -p %s" % self.working_dir)
         os.chdir(self.working_dir)
         run_command("%s clone %s" % (self.cli_tool, self.project_name))
 
@@ -588,10 +588,10 @@ class FedoraGitReleaser(Releaser):
         os.chdir(project_checkout)
 
         # Newer versions of git don't seem to want --cached here? Try both:
-        (status, diff_output) = commands.getstatusoutput("git diff --cached")
+        (status, diff_output) = getstatusoutput("git diff --cached")
         if diff_output.strip() == "":
             debug("git diff --cached returned nothing, falling back to git diff.")
-            (status, diff_output) = commands.getstatusoutput("git diff")
+            (status, diff_output) = getstatusoutput("git diff")
 
         if diff_output.strip() == "":
             print("No changes in main branch, skipping commit for: %s" % main_branch)
@@ -680,7 +680,7 @@ class FedoraGitReleaser(Releaser):
             return
 
         print("Submitting build: %s" % build_cmd)
-        (status, output) = commands.getstatusoutput(build_cmd)
+        (status, output) = getstatusoutput(build_cmd)
         if status > 0:
             if "already been built" in output:
                 print("Build has been submitted previously, continuing...")
@@ -797,7 +797,7 @@ class CvsReleaser(Releaser):
         self._verify_cvs_module_not_already_checked_out()
 
         print("Building release in CVS...")
-        commands.getoutput("mkdir -p %s" % self.working_dir)
+        getoutput("mkdir -p %s" % self.working_dir)
         debug("cvs_branches = %s" % self.cvs_branches)
 
         self.cvs_checkout_module()
@@ -873,7 +873,7 @@ class CvsReleaser(Releaser):
 
             # For entirely new files we need to cvs add:
             for add_file in new:
-                commands.getstatusoutput("cvs add %s" % add_file)
+                getstatusoutput("cvs add %s" % add_file)
 
             # Cleanup obsolete files:
             for cleanup_file in old:
@@ -913,7 +913,7 @@ class CvsReleaser(Releaser):
         print("")
 
         os.chdir(self.package_workdir)
-        (status, diff_output) = commands.getstatusoutput("cvs diff -u")
+        (status, diff_output) = getstatusoutput("cvs diff -u")
         print(diff_output)
 
         print("")
@@ -978,7 +978,7 @@ class CvsReleaser(Releaser):
             branch_dir = os.path.join(self.working_dir, self.project_name,
                     branch)
             os.chdir(branch_dir)
-            (status, output) = commands.getstatusoutput(cmd)
+            (status, output) = getstatusoutput(cmd)
             print(output)
             if status > 1:
                 self.cleanup()
diff --git a/src/tito/tagger.py b/src/tito/tagger.py
index bedcb74..1a5f185 100644
--- a/src/tito/tagger.py
+++ b/src/tito/tagger.py
@@ -17,7 +17,6 @@ Code for tagging Spacewalk/Satellite packages.
 import os
 import re
 import rpm
-import commands
 import StringIO
 import shutil
 import subprocess
@@ -34,6 +33,7 @@ from tito.common import (debug, error_out, run_command,
         get_script_path, get_spec_version_and_release, replace_version,
         tag_exists_locally, tag_exists_remotely, head_points_to_tag, undo_tag,
         increase_version, reset_release, increase_zstream)
+from tito.compat import *
 from tito.exception import TitoException
 from tito.config_object import ConfigObject
 
@@ -470,7 +470,7 @@ class VersionTagger(ConfigObject):
         print("   Push: git push && git push origin %s" % new_tag)
 
     def _check_tag_does_not_exist(self, new_tag):
-        status, output = commands.getstatusoutput(
+        status, output = getstatusoutput(
             'git tag -l %s|grep ""' % new_tag)
         if status == 0:
             raise Exception("Tag %s already exists!" % new_tag)
diff --git a/test/functional/fetch_tests.py b/test/functional/fetch_tests.py
index ea01a4c..264d275 100644
--- a/test/functional/fetch_tests.py
+++ b/test/functional/fetch_tests.py
@@ -15,7 +15,6 @@
 Functional Tests for the FetchBuilder.
 """
 
-import ConfigParser
 import glob
 import os
 import shutil
@@ -24,6 +23,7 @@ import tempfile
 from os.path import join
 
 from tito.common import run_command
+from tito.compat import *
 from fixture import TitoGitTestFixture, tito
 
 EXT_SRC_PKG = "extsrc"
@@ -44,7 +44,7 @@ class FetchBuilderTests(TitoGitTestFixture):
         spec = join(os.path.dirname(__file__), "specs/extsrc.spec")
 
         # Setup test config:
-        self.config = ConfigParser.RawConfigParser()
+        self.config = RawConfigParser()
         self.config.add_section("buildconfig")
         self.config.set("buildconfig", "builder",
                 "tito.builder.FetchBuilder")
diff --git a/test/functional/release_yum_tests.py b/test/functional/release_yum_tests.py
index 867c22c..d2b4e89 100644
--- a/test/functional/release_yum_tests.py
+++ b/test/functional/release_yum_tests.py
@@ -15,7 +15,6 @@
 Functional Tests for the FetchBuilder.
 """
 
-import ConfigParser
 import glob
 import os
 import shutil
@@ -25,6 +24,8 @@ from os.path import join
 
 from fixture import TitoGitTestFixture, tito
 
+from tito.compat import *
+
 PKG_NAME = "releaseme"
 
 RELEASER_CONF = """
@@ -42,7 +43,7 @@ class YumReleaserTests(TitoGitTestFixture):
         self.create_project(PKG_NAME)
 
         # Setup test config:
-        self.config = ConfigParser.RawConfigParser()
+        self.config = RawConfigParser()
         self.config.add_section("buildconfig")
         self.config.set("buildconfig", "builder",
                 "tito.builder.Builder")
diff --git a/test/unit/fixture.py b/test/unit/fixture.py
index dbd2d16..58908ac 100644
--- a/test/unit/fixture.py
+++ b/test/unit/fixture.py
@@ -14,6 +14,8 @@
 import os
 import unittest
 
+from tito.compat import *
+
 UNIT_DIR = os.path.abspath(os.path.dirname(__file__))
 REPO_DIR = os.path.join(UNIT_DIR, '..', '..')
 
diff --git a/test/unit/pep8-tests.py b/test/unit/pep8-tests.py
index 8a5ec36..bf099fd 100644
--- a/test/unit/pep8-tests.py
+++ b/test/unit/pep8-tests.py
@@ -84,3 +84,8 @@ class UglyHackishTest(TitoUnitTestFixture):
         cmd = "find . -type f -regex '.*\.py$' -exec egrep %s {} + | wc -l" % regex
         result = int(getoutput(cmd))
         self.assertEqual(result, 0, "Found except clause not supported in Python 3")
+
+    def test_import_commands(self):
+        cmd = "find . -type f -regex '.*\.py$' -exec egrep '^(import|from) commands\.' {} + | grep -v 'compat\.py' | wc -l"
+        result = int(getoutput(cmd))
+        self.assertEqual(result, 0, "Found commands module (not supported in Python 3)")
diff --git a/test/unit/test_build_target_parser.py b/test/unit/test_build_target_parser.py
index 28a6362..79f0b3b 100644
--- a/test/unit/test_build_target_parser.py
+++ b/test/unit/test_build_target_parser.py
@@ -1,7 +1,7 @@
 
 import unittest
 from tito.buildparser import BuildTargetParser
-from ConfigParser import ConfigParser
+from tito.compat import *
 from tito.exception import TitoException