CI PR #3: Fix the 1st half of the pyright warnings in the unit tests

.github/workflows/main.yml

@@ -28,8 +28,6 @@ jobs:
       fail-fast: false
       matrix:
         include:
-        - python-version: '3.6'
-          os: ubuntu-20.04
         - python-version: '3.10'
           os: ubuntu-22.04
         - python-version: '3.11'

pytest.ini

@@ -8,10 +8,11 @@
 # These are the most of the needed pytest plugins, unfortunately this list does
 # not support ;python_version<=3.0 or ;python_version>3.0. Therefore, it can
 # only list plugins available for all tested python versions (2.7, 3.6 ... 3.11):
+# pytest-localftpserver is also used, but its installation is not checked
+# to to its installation not being detected on Ubuntu 24.04:
 required_plugins =
     pytest_httpserver
     pytest-forked
-    pytest-localftpserver
     pytest-pythonpath
     pytest-subprocess
     pytest-timeout

tests/httpserver_testcase.py

@@ -1,4 +1,5 @@
 import os
+import sys
 import unittest
 from typing import Callable, Optional
 
@@ -14,6 +15,7 @@
     ErrorHandler = Optional[Callable[[Request], Response]]
 except ImportError:
     pytest.skip(allow_module_level=True)
+    sys.exit(0)  # Let pyright know that this is a dead end
 
 
 class HTTPServerTestCase(unittest.TestCase):
@@ -31,7 +33,7 @@
 
     @classmethod
     def serve_file(cls, root, file_path, error_handler=None, real_path=None):
-        # type:(str, str, Optional[Callable[[Request], Response]], Optional[str]) -> None
+        # type:(str, str, ErrorHandler, Optional[str]) -> None
         """Expect a GET request and handle it using the local pytest_httpserver.HTTPServer"""
 
         def handle_get(request):
@@ -44,6 +46,6 @@
             filepath = root + (real_path or file_path)
             assert os.path.exists(path=filepath)
             with open(file=filepath, mode="rb") as local_testdata_file:
                 return Response(local_testdata_file.read())
 
         cls.httpserver.expect_request(uri="/" + file_path).respond_with_handler(func=handle_get)

tests/test_accessor.py

@@ -1,14 +1,16 @@
 import unittest
-
-from pyfakefs.fake_filesystem import FakeFilesystem
+from typing import TYPE_CHECKING
 
 import xcp.accessor
 
 from .test_mountingaccessor import check_binary_read, check_binary_write
 
+if TYPE_CHECKING:
+    import pyfakefs
+
 
 def test_file_accessor(fs):
-    # type:(FakeFilesystem) -> None
+    # type(pyfakefs.fake_filesystem.FakeFilesystem) -> None
     """Test FileAccessor.writeFile(), .openAddress and .access using pyfakefs"""
     accessor = xcp.accessor.createAccessor("file://repo/", False)
     assert isinstance(accessor, xcp.accessor.FileAccessor)
@@ -18,7 +20,7 @@ def test_file_accessor(fs):
 
 class TestAccessor(unittest.TestCase):
     def setUp(self):
-        """Provide the refrence content of the repo/.treeinfo file for check_repo_access()"""
+        """Provide the reference content of the repo/.treeinfo file for check_repo_access()"""
         with open("tests/data/repo/.treeinfo", "rb") as dot_treeinfo:
             self.reference_treeinfo = dot_treeinfo.read()
 
@@ -35,6 +37,7 @@ def check_repo_access(self, a):
         self.assertFalse(a.access('no_such_file'))
         self.assertEqual(a.lastError, 404)
         a.finish()
+        a.finish()  # Cover the code handing a 2nd call of accessor.finish()
 
     def test_filesystem_accessor_access(self):
         """Test FilesystemAccessor.access()"""

tests/test_ftpaccessor.py

@@ -21,13 +21,14 @@
 from io import BytesIO
 
 import pytest
-import pytest_localftpserver  # pylint: disable=unused-import # Ensure that it is installed
+import pytest_localftpserver  # Ensure that it is installed
 from six import ensure_binary, ensure_str
 
 import xcp.accessor
 
 binary_data = b"\x80\x91\xaa\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xcc\xdd\xee\xff"
 text_data = "✋➔Hello Accessor from the 🗺, download and verify ✅ me!"
+assert pytest_localftpserver
 
 
 def upload_textfile(ftpserver, accessor):
@@ -47,7 +48,7 @@
     return next(ftp_content_generator)["content"]
 
 
 @pytest.fixture
 def ftp_accessor(ftpserver):
     upload = {"src": "tests/test_ftpaccessor.py", "dest": "testdir/dummy-file-to-create-testdir"}
     ftpserver.put_files(upload, anon=False, overwrite=True)

tests/test_ifrename_logic.py

@@ -1,4 +1,4 @@
-# pyright: reportGeneralTypeIssues=false
+# pyright: reportGeneralTypeIssues=false,reportAttributeAccessIssue=false
 # pytype: disable=attribute-error
 from __future__ import print_function
 from __future__ import unicode_literals

tests/test_logger.py

@@ -31,3 +31,4 @@ def test_openLog_mock_stdin():
         assert openLog("test.log") is True
         os.close(slave_fd)
         os.close(master_fd)
+        open_mock.assert_called_once_with("test.log", "a", **open_utf8)

tests/test_mountingaccessor.py

@@ -4,7 +4,7 @@
 from typing import TYPE_CHECKING, cast
 
 from mock import patch
 from pyfakefs.fake_filesystem import FakeFileOpen, FakeFilesystem
 
 import xcp.accessor
 import xcp.mount
@@ -20,6 +20,7 @@
     import pytest
 
     pytest.skip(allow_module_level=True)
+    sys.exit(0)  # Let pyright know that this is a dead end
 
 binary_data = b"\x00\x1b\x5b\x95\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xcc\xdd\xee\xff"
 

tests/test_pci.py

@@ -3,7 +3,7 @@
 import unittest
 from os import environ
 
 import pyfakefs.fake_filesystem_unittest
 import pytest
 from mock import Mock, patch
 
@@ -11,6 +11,8 @@
 
 if sys.version_info >= (3, 6):
     from pytest_subprocess.fake_process import FakeProcess
+else:
+    pytest.skip(allow_module_level=True)
 
 class TestInvalid(unittest.TestCase):
 
@@ -31,6 +33,7 @@
 
     def test_null_with_segment(self):
 
+        assert PCI.is_valid("0000:00:00.0") is True
         c = PCI("0000:00:00.0")
 
         self.assertEqual(c.segment, 0)
@@ -43,6 +46,7 @@
 
     def test_null_without_segment(self):
 
+        assert PCI.is_valid("00:00.0") is True
         c = PCI("00:00.0")
 
         self.assertEqual(c.segment, 0)
@@ -54,28 +58,47 @@
 
     def test_valid(self):
 
+        assert PCI.is_valid("8765:43:1f.3") is True
         c = PCI("8765:43:1f.3")
 
         self.assertEqual(c.segment, 0x8765)
         self.assertEqual(c.bus, 0x43)
         self.assertEqual(c.device, 0x1f)
         self.assertEqual(c.function, 0x3)
 
+    def test_valid_index(self):
+
+        assert PCI.is_valid("8765:43:1f.3[0]") is True
+        assert PCI.is_valid("1234:56:01.7[1]") is True
+        c = PCI("1234:56:01.7[1]")
+
+        self.assertEqual(c.segment, 0x1234)
+        self.assertEqual(c.bus, 0x56)
+        self.assertEqual(c.device, 0x01)
+        self.assertEqual(c.function, 0x7)
+        self.assertEqual(c.index, 0x1)
+
     def test_equality(self):
 
         self.assertEqual(PCI("0000:00:00.0"), PCI("00:00.0"))
+        assert PCI("1234:56:01.7[1]") != PCI("1234:56:01.7[2]")
+        assert PCI("1234:56:01.2") >= PCI("1234:56:01.2")
+        assert PCI("1234:56:01.1") <= PCI("1234:56:01.2")
+        assert PCI("1234:56:01.3") > PCI("1234:56:01.2")
+        assert PCI("1234:56:01.1") < PCI("1234:56:02.2")
 
 
 if sys.version_info >= (2, 7):
     def assert_videoclass_devices(ids, devs):  # type: (PCIIds, PCIDevices) -> None
-        """Verification function for checking the otuput of PCIDevices.findByClass()"""
+        """Verification function for checking the output of PCIDevices.findByClass()"""
         video_class = ids.lookupClass('Display controller')
         assert video_class == ["03"]
         sorted_devices = sorted(devs.findByClass(video_class),
                                 key=lambda x: x['id'])
 
         # Assert devs.findByClass() finding 3 GPUs from tests/data/lspci-mn in our mocked PCIIds DB:
         assert len(sorted_devices) == 3
+        assert len(devs.findByClass("03", "80")) == 2
 
         # For each of the found devices, assert these expected values:
         for (video_dev,
@@ -135,7 +158,7 @@
         with all Python versions.
         (The old test using moc could not detect a missing step in the Py3 migration)
         """
         with pyfakefs.fake_filesystem_unittest.Patcher() as p:
             assert p.fs
             p.fs.add_real_file("tests/data/pci.ids", target_path="/usr/share/hwdata/pci.ids")
             ids = PCIIds.read()
@@ -164,6 +187,7 @@
     def mock_lspci_using_open_testfile(fp):
         """Mock xcp.pci.PCIDevices.Popen() using open(tests/data/lspci-mn)"""
         with open("tests/data/lspci-mn", "rb") as fake_data:
-            assert isinstance(fp, FakeProcess)
+            if sys.version_info >= (3, 6):
+                assert isinstance(fp, FakeProcess)
             fp.register_subprocess(["lspci", "-mn"], stdout=fake_data.read())
         return PCIDevices()

tox.ini

@@ -5,7 +5,7 @@
 # 2. python 3.6 test and pylint warnings from changed lines
 # 3. pytype (needs Python 3.8 for best results)
 # 4. pyre and pyright checks, pytest test report as markdown for GitHub Actions summary
-envlist = py38-covcombine-check, py36-lint-test, py310-pytype, py311-pyre-mdreport
+envlist = py38-covcombine-check, py311-lint-test, py310-pytype, py311-pyre-mdreport
 isolated_build = true
 skip_missing_interpreters = true
 requires =
@@ -28,9 +28,9 @@ commands    =
     # https://github.com/actions/toolkit/blob/main/docs/commands.md#problem-matchers
     echo "::add-matcher::.github/workflows/PYTHONWARNINGS-problemMatcher.json"
     pytest --cov -v --new-first -x --show-capture=all -rA
-    sh -c 'ls -l {env:COVERAGE_FILE}'
     sh -c 'if [ -n "{env:PYTEST_MD_REPORT_OUTPUT}" -a -n "{env:GITHUB_STEP_SUMMARY}" ];then    \
-      sed -i "s/tests\(.*py\)/[&](&)/" {env:PYTEST_MD_REPORT_OUTPUT}; sed "/title/,/\/style/d" \
+      mkdir -p $(dirname "{env:GITHUB_STEP_SUMMARY:.git/sum.md}");                             \
+      sed "s/tests\(.*py\)/[&](&)/"                                                            \
       {env:PYTEST_MD_REPORT_OUTPUT} >{env:GITHUB_STEP_SUMMARY:.git/sum.md};fi'
 
 [testenv]
@@ -202,8 +202,6 @@ max-line-length = 129
 
 [pyre]
 commands =
-    pyre: python3.11 --version -V # Needs py311-pyre, does not work with py310-pyre
-    python pyre_runner.py
     -pyright
 
 [pytype]