Merge branch 'testsuite-driven-py3'

* testsuite-driven-py3:
  CI: also run tests with python3
  Pylint complements: avoid no-else-raise "refactor" issues
  Pylint complements: test_ifrename_logic: disable "no-member" warning
  Pylint complements: whitespace in expressions
  Pylint complements: honor len-as-condition convention
  WIP xcp.repository: switch from ConfigParser to configparser
  xcp.repository: switch from md5 to hashlib.md5
  WIP python3: fix xmlunwrap and its test to align with the use of bytes
  cpiofile: migrate last "list.sort()" call still using a "cmp" argument
  test_cpio: ensure paths are handled as text
  ifrename: don't rely on dict ordering in tests
  test_dom0: mock "open()" in a python3-compatible way
  python3: use raw strings for regexps, fixes insufficient quoting
  xcp.net.ifrename.logic: use "logger.warning", "logger.warn" is deprecated
  python3: xcp.net.mac: use six.python_2_unicode_compatible for stringification
  Remove direct call's to file's constructor and replace them with calls to open() as ths is considered best practice.
  python3: use "six.ensure_binary" and "six.ensure_text" for str/bytes conversion
  python3: use six.string_types not version-dependant types

Signed-off-by: Yann Dirson <[email protected]>

Author: ydirson

.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/data/repo/boot/isolinux/mboot.c32

@@ -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_accessor.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_cpio.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_dom0.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_dynamic.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_logic.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_ifrename_static.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)