Fix #18: encode() only if type is unicode (only for Py2)

.github/workflows/main.yml

@@ -3,48 +3,54 @@ name: Unit tests
 on: [push, pull_request]
 
 jobs:
-  test_py2:
-    runs-on: ubuntu-20.04
+  test:
+    strategy:
+      matrix:
+        include:
+        - pyversion: '2.7'
+          os: ubuntu-20.04
+        - pyversion: '3'
+          os: ubuntu-latest
+    runs-on: ${{ matrix.os }}
 
     steps:
       - uses: actions/checkout@v2
         with:
           fetch-depth: 0
-      - name: Set up Python 2.7
+      - name: Set up Python ${{ matrix.pyversion }}
         uses: actions/setup-python@v2
         with:
-          python-version: '2.7'
+          python-version: ${{ matrix.pyversion }}
 
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip
           pip install -r requirements-dev.txt
           # FIXME: branding.py still has no permanent home
           curl https://gist.github.com/ydirson/3c36a7e19d762cc529a6c82340894ccc/raw/5ca39f621b1feab813e171f535c1aad1bd483f1d/branding.py -O -L
-          pip install pyliblzma
           pip install -e .
           command -v xz
 
       - name: Test
         run: |
           pytest --cov -rP
           coverage xml
-          coverage html
-          coverage html -d htmlcov-tests --include="tests/*"
-          diff-cover --html-report coverage-diff.html coverage.xml
+          coverage html -d htmlcov-${{ matrix.pyversion }}
+          coverage html -d htmlcov-tests-${{ matrix.pyversion }} --include="tests/*"
+          diff-cover --compare-branch=origin/master --html-report coverage-diff-${{ matrix.pyversion }}.html coverage.xml
 
       - name: Pylint
         run: |
           pylint --version
           pylint --exit-zero xcp/ tests/ setup.py
           pylint --exit-zero --msg-template="{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}" xcp/ tests/ setup.py > pylint.txt
-          diff-quality --violations=pylint --html-report pylint-diff.html pylint.txt
+          diff-quality --compare-branch=origin/master --violations=pylint --html-report pylint-diff-${{ matrix.pyversion }}.html pylint.txt
 
       - uses: actions/upload-artifact@v3
         with:
           name: Coverage and pylint reports
           path: |
-            coverage-diff.html
-            pylint-diff.html
-            htmlcov
-            htmlcov-tests
+            coverage-diff-${{ matrix.pyversion }}.html
+            pylint-diff-${{ matrix.pyversion }}.html
+            htmlcov-${{ matrix.pyversion }}
+            htmlcov-tests-${{ matrix.pyversion }}

requirements-dev.txt

@@ -5,6 +5,11 @@ diff_cover
 mock
 pytest
 pytest-cov
+parameterized
 # dependencies also in setup.py until they can be used
 six
 future
+
+# python-2.7 only
+configparser ; python_version < "3.0"
+pyliblzma ; python_version < "3.0"

tests/test_accessor.py

@@ -1,21 +1,42 @@
 import unittest
+import hashlib
+from tempfile import NamedTemporaryFile
+
+from parameterized import parameterized_class
 
 import xcp.accessor
 
+@parameterized_class([{"url": "file://tests/data/repo/"},
+                      {"url": "https://updates.xcp-ng.org/netinstall/8.2.1"}])
 class TestAccessor(unittest.TestCase):
-    def test_http(self):
-        raise unittest.SkipTest("comment out if you really mean it")
-        a = xcp.accessor.createAccessor("https://updates.xcp-ng.org/netinstall/8.2.1", True)
+    def test_access(self):
+        a = xcp.accessor.createAccessor(self.url, True)
         a.start()
         self.assertTrue(a.access('.treeinfo'))
         self.assertFalse(a.access('no_such_file'))
         self.assertEqual(a.lastError, 404)
         a.finish()
 
-    def test_file(self):
-        a = xcp.accessor.createAccessor("file://tests/data/repo/", True)
+    def test_file_binfile(self):
+        BINFILE = "boot/isolinux/mboot.c32"
+        a = xcp.accessor.createAccessor(self.url, True)
         a.start()
-        self.assertTrue(a.access('.treeinfo'))
-        self.assertFalse(a.access('no_such_file'))
-        self.assertEqual(a.lastError, 404)
+        self.assertTrue(a.access(BINFILE))
+        inf = a.openAddress(BINFILE)
+        with NamedTemporaryFile("wb") as outf:
+            while True:
+                data = inf.read()
+                if not data: # EOF
+                    break
+                outf.write(data)
+            outf.flush()
+            hasher = hashlib.md5()
+            with open(outf.name, "rb") as bincontents:
+                while True:
+                    data = bincontents.read()
+                    if not data: # EOF
+                        break
+                    hasher.update(data)
+                csum = hasher.hexdigest()
+                self.assertEqual(csum, "eab52cebc3723863432dc672360f6dac")
         a.finish()

tests/test_cpio.py

@@ -14,11 +14,11 @@ def writeRandomFile(fn, size, start=b'', add=b'a'):
     with open(fn, 'wb') as f:
         m = md5()
         m.update(start)
-        assert(len(add) != 0)
+        assert add
         while size > 0:
             d = m.digest()
             if size < len(d):
-                d=d[:size]
+                d = d[:size]
             f.write(d)
             size -= len(d)
             m.update(add)

tests/test_dom0.py

@@ -30,7 +30,7 @@ def mock_version(open_mock, version):
             (2*1024, 4*1024, 8*1024), # Above max
             ]
 
-        with patch("__builtin__.open") as open_mock:
+        with patch("xcp.dom0.open") as open_mock:
             for host_gib, dom0_mib, _ in test_values:
                 mock_version(open_mock, '2.8.0')
                 expected = dom0_mib * 1024;
@@ -39,7 +39,7 @@ def mock_version(open_mock, version):
 
             open_mock.assert_called_with("/etc/xensource-inventory")
 
-        with patch("__builtin__.open") as open_mock:
+        with patch("xcp.dom0.open") as open_mock:
             for host_gib, _, dom0_mib in test_values:
                 mock_version(open_mock, '2.9.0')
                 expected = dom0_mib * 1024;

tests/test_ifrename_dynamic.py

@@ -125,10 +125,10 @@ def test_pci_matching_invert(self):
                      MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", kname="eth1",
                             ppn="", label="")])
 
-        self.assertEqual(dr.rules,[
+        self.assertEqual(set(dr.rules), set([
                 MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth1"),
                 MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth0")
-                ])
+        ]))
 
     def test_pci_missing(self):
 

tests/test_ifrename_logic.py

@@ -518,6 +518,7 @@ def test_ibft_nic_to_ibft(self):
 
 
 class TestInputSanitisation(unittest.TestCase):
+    # pylint: disable=no-member
 
     def setUp(self):
         """

tests/test_ifrename_static.py

@@ -375,10 +375,10 @@ def test_pci_matching(self):
 
         sr.generate(self.state)
 
-        self.assertEqual(sr.rules,[
-                MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth1"),
-                MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth0")
-                ])
+        self.assertEqual(set(sr.rules), set([
+            MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth1"),
+            MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth0")
+        ]))
 
     def test_pci_matching_invert(self):
 
@@ -389,10 +389,10 @@ def test_pci_matching_invert(self):
 
         sr.generate(self.state)
 
-        self.assertEqual(sr.rules,[
-                MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth1"),
-                MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth0")
-                ])
+        self.assertEqual(set(sr.rules), set([
+            MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth1"),
+            MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth0")
+        ]))
 
     def test_pci_matching_mixed(self):
 
@@ -403,10 +403,10 @@ def test_pci_matching_mixed(self):
 
         sr.generate(self.state)
 
-        self.assertEqual(sr.rules,[
-                MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth0"),
-                MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth1")
-                ])
+        self.assertEqual(set(sr.rules), set([
+            MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth0"),
+            MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth1")
+        ]))
 
     def test_pci_missing(self):
 

tests/test_repository.py

@@ -1,22 +1,15 @@
 import unittest
+from parameterized import parameterized_class
 
 import xcp.accessor
 from xcp import repository
 from xcp.version import Version
 
+@parameterized_class([{"url": "file://tests/data/repo/"},
+                      {"url": "https://updates.xcp-ng.org/netinstall/8.2.1"}])
 class TestRepository(unittest.TestCase):
-    def test_http(self):
-        raise unittest.SkipTest("comment out if you really mean it")
-        a = xcp.accessor.createAccessor("https://updates.xcp-ng.org/netinstall/8.2.1", True)
-        repo_ver = repository.BaseRepository.getRepoVer(a)
-        self.assertEqual(repo_ver, Version([3, 2, 1]))
-        product_ver = repository.BaseRepository.getProductVersion(a)
-        self.assertEqual(product_ver, Version([8, 2, 1]))
-        repos = repository.BaseRepository.findRepositories(a)
-        self.assertEqual(len(repos), 1)
-
-    def test_file(self):
-        a = xcp.accessor.createAccessor("file://tests/data/repo/", True)
+    def test_basicinfo(self):
+        a = xcp.accessor.createAccessor(self.url, True)
         repo_ver = repository.BaseRepository.getRepoVer(a)
         self.assertEqual(repo_ver, Version([3, 2, 1]))
         product_ver = repository.BaseRepository.getProductVersion(a)

xcp/bootloader.py

@@ -336,19 +336,19 @@ def create_label(title):
         try:
             for line in fh:
                 l = line.strip()
-                menu_match = re.match("menuentry ['\"]([^']*)['\"](.*){", l)
+                menu_match = re.match(r"menuentry ['\"]([^']*)['\"](.*){", l)
 
                 # Only parse unindented default and timeout lines to prevent
                 # changing these lines in if statements.
                 if l.startswith('set default=') and l == line.rstrip():
                     default = l.split('=')[1]
-                    match = re.match("['\"](.*)['\"]$", default)
+                    match = re.match(r"['\"](.*)['\"]$", default)
                     if match:
                         default = match.group(1)
                 elif l.startswith('set timeout=') and l == line.rstrip():
                     timeout = int(l.split('=')[1]) * 10
                 elif l.startswith('serial'):
-                    match = re.match("serial --unit=(\d+) --speed=(\d+)", l)
+                    match = re.match(r"serial --unit=(\d+) --speed=(\d+)", l)
                     if match:
                         serial = {
                             'port': int(match.group(1)),

xcp/cmd.py

@@ -24,15 +24,16 @@
 """Command processing"""
 
 import subprocess
+import six
 
 import xcp.logger as logger
 
 def runCmd(command, with_stdout = False, with_stderr = False, inputtext = None):
-    cmd = subprocess.Popen(command, bufsize = 1,
-                           stdin = (inputtext and subprocess.PIPE or None),
-                           stdout = subprocess.PIPE,
-                           stderr = subprocess.PIPE,
-                           shell = isinstance(command, str))
+    cmd = subprocess.Popen(command, bufsize=1,
+                           stdin=(inputtext and subprocess.PIPE or None),
+                           stdout=subprocess.PIPE,
+                           stderr=subprocess.PIPE,
+                           shell=isinstance(command, six.string_types))
 
     (out, err) = cmd.communicate(inputtext)
     rv = cmd.returncode

xcp/cpiofile.py

@@ -310,7 +310,7 @@ def _init_write_gz(self):
         self.__write(b"\037\213\010\010%s\002\377" % timestamp)
         if self.name.endswith(".gz"):
             self.name = self.name[:-3]
-        self.__write(self.name + NUL)
+        self.__write(six.ensure_binary(self.name) + NUL)
 
     def write(self, s):
         """Write string s to the stream.
@@ -951,7 +951,7 @@ def __init__(self, name=None, mode="r", fileobj=None):
         self.mode = {"r": "rb", "a": "r+b", "w": "wb"}[mode]
 
         if not fileobj:
-            fileobj = file(name, self.mode)
+            fileobj = bltn_open(name, self.mode)
             self._extfileobj = False
         else:
             if name is None and hasattr(fileobj, "name"):
@@ -1109,7 +1109,7 @@ def gzopen(cls, name, mode="r", fileobj=None, compresslevel=9):
             raise CompressionError("gzip module is not available")
 
         if fileobj is None:
-            fileobj = file(name, mode + "b")
+            fileobj = bltn_open(name, mode + "b")
 
         try:
             t = cls.cpioopen(name, mode, gzip.GzipFile(name, mode, compresslevel, fileobj))
@@ -1354,7 +1354,7 @@ def add(self, name, arcname=None, recursive=True):
 
         # Append the cpio header and data to the archive.
         if cpioinfo.isreg():
-            f = file(name, "rb")
+            f = bltn_open(name, "rb")
             self.addfile(cpioinfo, f)
             f.close()
 
@@ -1420,20 +1420,20 @@ def extractall(self, path=".", members=None):
                 # Extract directory with a safe mode, so that
                 # all files below can be extracted as well.
                 try:
-                    os.makedirs(os.path.join(path, cpioinfo.name), 0o777)
+                    os.makedirs(os.path.join(path, six.ensure_text(cpioinfo.name)), 0o777)
                 except EnvironmentError:
                     pass
                 directories.append(cpioinfo)
             else:
                 self.extract(cpioinfo, path)
 
         # Reverse sort directories.
-        directories.sort(lambda a, b: cmp(a.name, b.name))
+        directories.sort(key=lambda x: x.name)
         directories.reverse()
 
         # Set correct owner, mtime and filemode on directories.
         for cpioinfo in directories:
-            path = os.path.join(path, cpioinfo.name)
+            path = os.path.join(path, six.ensure_text(cpioinfo.name))
             try:
                 self.chown(cpioinfo, path)
                 self.utime(cpioinfo, path)
@@ -1462,7 +1462,7 @@ def extract(self, member, path=""):
             cpioinfo._link_path = path
 
         try:
-            self._extract_member(cpioinfo, os.path.join(path, cpioinfo.name))
+            self._extract_member(cpioinfo, os.path.join(path, six.ensure_text(cpioinfo.name)))
         except EnvironmentError as e:
             if self.errorlevel > 0:
                 raise
@@ -1594,7 +1594,7 @@ def makefile(self, cpioinfo, targetpath):
 
         if extractinfo:
             source = self.extractfile(extractinfo)
-            target = file(targetpath, "wb")
+            target = bltn_open(targetpath, "wb")
             copyfileobj(source, target)
             source.close()
             target.close()
@@ -1926,5 +1926,5 @@ def is_cpiofile(name):
     except CpioError:
         return False
 
-def cpioOpen(*al, **ad):
-    return CpioFile.open(*al, **ad)
+bltn_open = open
+open = CpioFile.open

xcp/dom0.py

@@ -96,7 +96,7 @@ def default_memory(host_mem_kib):
     return default_memory_for_version(host_mem_kib, platform_version)
 
 
-_size_and_unit_re = re.compile("^(-?\d+)([bkmg]?)$", re.IGNORECASE)
+_size_and_unit_re = re.compile(r"^(-?\d+)([bkmg]?)$", re.IGNORECASE)
 
 def _parse_size_and_unit(s):
     m = _size_and_unit_re.match(s)