CP-41819: Enable unit tests to run for Python3 as well

.github/workflows/main.yml

@@ -1,24 +1,54 @@
-name: Build and test
+#--------------------------------------------------------------------------------------
+# This GitHub Actions workflow can be run locally using https://github.com/nektos/act
+#
+# act normally uses docker, but it can also be run using podman on Fedora 37:
+# dnf install act-cli podman
+# podman system service -t 0 &
+# act --bind --container-daemon-socket $XDG_RUNTIME_DIR/podman/podman.sock -W .github/workflows/main.yml
+#--------------------------------------------------------------------------------------
+name: "GitHub CI"
 
-on:
-  push:
-  pull_request:
+# The GitHub events that trigger this workflow:
+# Checks can be skipped by adding "skip-checks: true" to a commit message,
+# or requested by adding "request-checks: true" if disabled by default for pushes:
+# https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks#skipping-and-requesting-checks-for-individual-commits
+on: [push, pull_request]
 
-concurrency:  # Cancel pending and in-progress workflows for the same PR, branch or tag:
-  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+# Cancel a currently running workflow from the same PR, branch or tag
+# when a new workflow is triggered:
+# https://stackoverflow.com/questions/66335225/how-to-cancel-previous-runs-in-the-pr-when-you-push-new-commitsupdate-the-curre
+concurrency:
   cancel-in-progress: true
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+
 env:
+  DEBIAN_FRONTEND: noninteractive
   # No warnings for pip and pytest themselves; pytest enables warnings in conftest.py
   PYTHONWARNINGS: ignore
   # Development Mode for stronger checks: https://docs.python.org/3/library/devmode.html
   PYTHONDEVMODE: yes
 jobs:
-  python-checks:
-    name: Run the Xen-Bugtool Test Environment
+  container-tests:
+    name: "Python2: Container tests"
+    runs-on: ubuntu-22.04
+    # https://github.com/Docker-Hub-frolvlad/docker-alpine-python2
+    container: frolvlad/alpine-python2
+    steps:
+      - uses: actions/checkout@v3
+      - name: Install test tools
+        run: apk add --no-cache libxml2-utils bash
+      - name: Install python requirements
+        run: pip install -r requirements.txt
+      - name: Test sar file collection, extended by XSI-1385 with plain-text SARs
+        run: bash -x tests/integration/sar-file-collection.test.sh
+      - name: Test creating a tarball for /etc/systemd
+        run: bash -x tests/integration/xenserver-config-systemd.sh
+
+  python2-tests:
+    name: "Python2: PyLint and Pytest"
     runs-on: ubuntu-20.04
     steps:
-      - name: Checkout code
-        uses: actions/checkout@v3
+      - uses: actions/checkout@v3
 
       - name: Install dependencies
         run: |
@@ -34,18 +64,28 @@ jobs:
         run: python2 -m pylint xen-bugtool
       - name: Run python2 -m pytest to execute all unit and integration tests
         run: python2 -m pytest -v -rA
+
+  pre-commit:
+    name: "Python3: Pre-Commit Suite"
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v3
+
       # https://www.python4data.science/en/latest/productive/git/advanced/hooks/ci.html
       - uses: actions/setup-python@v4
         with:
-          python-version: '3.11'
+          python-version: '3.10'
           cache: 'pip'
+
       - run: pip install -r requirements-dev.txt
         name: Install the pytest dependencies for running the pytest suite using Python3
+
       - uses: actions/cache@v3
         name: Setup cache for running pre-commit fast
         with:
           path: ~/.cache/pre-commit
           key: pre-commit|${{ env.pythonLocation }}|${{ hashFiles('.pre-commit-config.yaml') }}
+
       - uses: pre-commit/[email protected]
         name: Run pre-commit checks
         env:

.pre-commit-config.yaml

@@ -63,10 +63,11 @@ repos:
     hooks:
     -   id: pytest
         name: check that the Xen-Bugtool Test Environment passes
-        entry: env PYTHONDEVMODE=yes python3 -m pytest
+        entry: env PYTHONDEVMODE=yes python3 -m pytest tests/unit
         pass_filenames: false
-        language: system
+        language: python
         types: [python]
+        additional_dependencies: [defusedxml, pytest, lxml, XenAPI]
 -   repo: https://github.com/PyCQA/autoflake
     rev: v2.2.1
     hooks:
@@ -92,7 +93,7 @@ repos:
         additional_dependencies:
           - isort
 -   repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.7.0
+    rev: v1.7.1
     hooks:
     -   id: mypy
         name: Run mypy to check e.g. that all expected arguments are passed to functions etc
@@ -103,3 +104,17 @@ repos:
     -   id: pylint
         name: Run pylint to check that docstrings are added (and all other enabled checks)
         log_file: ".git/pre-commit-pylint.log"
+-   repo: https://github.com/RobertCraigie/pyright-python
+    rev: v1.1.339
+    hooks:
+    -   id: pyright
+        name: Run pyright to check the unit tests for any typing warnings (use for bugtool later)
+        exclude: xen-bugtool
+        additional_dependencies: [defusedxml, pytest, lxml, XenAPI]
+-   repo: https://github.com/mattseymour/pre-commit-pytype
+    rev: '2023.5.8'
+    hooks:
+    -   id: pytype
+        name: Run pytype to check the unit tests for any typing warnings (does not work on bugtool yet)
+        exclude: xen-bugtool
+        additional_dependencies: [pytest]

pytype.cfg

@@ -0,0 +1,86 @@
+# NOTE: All relative paths are relative to the location of this file.
+
+[pytype]
+
+# Space-separated list of files or directories to exclude.
+exclude =
+    xen-bugtool
+#    **/*_test.py
+#    **/test_*.py
+
+# Space-separated list of files or directories to process.
+inputs =
+    .
+
+# Keep going past errors to analyze as many files as possible.
+keep_going = True
+
+# Run N jobs in parallel. When 'auto' is used, this will be equivalent to the
+# number of CPUs on the host system.
+jobs = auto
+
+# All pytype output goes here.
+output = .pytype
+
+# Platform (e.g., "linux", "win32") that the target code runs on.
+platform = linux
+
+# Paths to source code directories, separated by ':'.
+pythonpath = .
+
+# Python version (major.minor) of the target code.
+python_version = 3.10
+
+# Bind 'self' in methods with non-transparent decorators. This flag is temporary
+# and will be removed once this behavior is enabled by default.
+bind_decorated_methods = False
+
+# Don't allow None to match bool. This flag is temporary and will be removed
+# once this behavior is enabled by default.
+none_is_not_bool = False
+
+# Enable parameter count checks for overriding methods with renamed arguments.
+# This flag is temporary and will be removed once this behavior is enabled by
+# default.
+overriding_renamed_parameter_count_checks = True
+
+# Variables initialized as None retain their None binding. This flag is
+# temporary and will be removed once this behavior is enabled by default.
+strict_none_binding = False
+
+# Support the third-party fiddle library. This flag is temporary and will be
+# removed once this behavior is enabled by default.
+use_fiddle_overlay = False
+
+# Opt-in: Do not allow Any as a return type.
+no_return_any = False
+
+# Opt-in: Require decoration with @typing.override when overriding a method or
+# nested class attribute of a parent class.
+require_override_decorator = False
+
+# Experimental: Infer precise return types even for invalid function calls.
+precise_return = False
+
+# Experimental: Solve unknown types to label with structural types.
+protocols = False
+
+# Experimental: Only load submodules that are explicitly imported.
+strict_import = False
+
+# Experimental: Enable exhaustive checking of function parameter types.
+strict_parameter_checks = False
+
+# Experimental: Emit errors for comparisons between incompatible primitive
+# types.
+strict_primitive_comparisons = True
+
+# Experimental: Check that variables are defined in all possible code paths.
+strict_undefined_checks = False
+
+# Space-separated list of error names to ignore.
+disable =
+    pyi-error
+
+# Don't report errors.
+report_errors = True

tests/integration/conftest.py

@@ -5,8 +5,8 @@
 
 import pytest
 
-from namespace_container import activate_private_test_namespace, mount, umount
-from utils import BUGTOOL_DOM0_TEMPL, BUGTOOL_OUTPUT_DIR, run
+from .namespace_container import activate_private_test_namespace, mount, umount
+from .utils import BUGTOOL_DOM0_TEMPL, BUGTOOL_OUTPUT_DIR, run
 
 
 @pytest.fixture(autouse=True, scope="session")

tests/integration/test_system_load.py

@@ -1,7 +1,7 @@
 """tests/integration/test_system_load.py: Test xen-bugtool --entries=system-load"""
 import os
 
-from utils import check_file, run_bugtool_entry, assert_content_from_dom0_template
+from .utils import check_file, run_bugtool_entry, assert_content_from_dom0_template
 
 
 # In this test case we need to sleep for 1 sec, and it is sufficient

tests/integration/test_xenserver_config.py

@@ -1,7 +1,7 @@
 """tests/integration/test_xenserver_config.py: Test xen-bugtool --entries=xenserver-config"""
 import os
 
-from utils import assert_cmd, check_file, run_bugtool_entry, assert_content_from_dom0_template
+from .utils import assert_cmd, check_file, run_bugtool_entry, assert_content_from_dom0_template
 
 
 def test_xenserver_config(output_archive_type):

tests/integration/utils.py

@@ -9,7 +9,7 @@
 import zipfile
 from subprocess import PIPE, Popen
 
-from lxml import etree
+from lxml.etree import XMLSchema, parse  # pytype: disable=import-error
 
 # pyright: ignore[reportMissingImports]
 if sys.version_info.major == 2:
@@ -102,16 +102,16 @@ def run_bugtool_entry(archive_type, test_entries):
     print(output)
     if error_code:
         raise RuntimeError(output)
-    srcdir = os.getcwd()
+    src_dir = os.getcwd()
     os.chdir(BUGTOOL_OUTPUT_DIR)
     output_file = test_entries + "." + archive_type
     print("# Unpacking " + BUGTOOL_OUTPUT_DIR + output_file + " and verifying inventory.xml")
     extract(output_file, archive_type)
     os.chdir(test_entries)
     # Validate the extracted inventory.xml using the XML schema from the test framework:
-    with open(srcdir + "/tests/integration/inventory.xsd") as xmlschema:
-        etree.XMLSchema(etree.parse(xmlschema)).assertValid(etree.parse("inventory.xml"))
-        # After successfuly validation of the inventory.xml, remove it (not removed files make the test fail):
+    with open(src_dir + "/tests/integration/inventory.xsd") as xml_schema:
+        XMLSchema(parse(xml_schema)).assertValid(parse("inventory.xml"))
+        # Remove valid inventory.xml (not removed files will make the tests fail):
         os.unlink("inventory.xml")
-    # assert_content_from_dom0_template() does not know the srcdir: add a symlink so it can reach the tests
-    os.symlink(srcdir + "/tests", "tests")
+    # Add a symlink, so assert_content_from_dom0_template() can find the tests:
+    os.symlink(src_dir + "/tests", "tests")