Add Python3 pre-commit config checks with pylint and mypy

Also enables spell checking when pre-commit runs pylint.

- Add pyproject.toml with config for mypy, black and isort
- Add .pre-commit-config.yaml for https://github.com/pre-commit
- Add GitHub action with cache to .github/workflows/build.yml
- Fix execute permissions on files found by pre-commit checks.
- build/mkinfo.py: Add module docstring as requested by pylint.
- tests/python/conftest.py: Add a pytest fixture for unit-tests
- tests/python/test_parse_test_instance_string.py: Add an
    initial unit test for xtf-runner using the pytest fixture.
- xtf-runner:
  - Import json always as Python2.7 is the minium version
  - Add comments to locally disable checks where needed.
  - Resolve a line-too-long while resolving a no-else-return

Signed-off-by: Bernhard Kaindl <[email protected]>

Author: bernhardkaindl

.github/workflows/build.yml

@@ -3,6 +3,46 @@ name: build
 on: [push, pull_request]
 
 jobs:
+  python-tests:
+    name: "Python Tests"
+    runs-on: ubuntu-20.04
+    steps:
+    - name: Checkout code
+      uses: actions/checkout@v3
+      with:
+        # To have access to secrets, use the PR branch:
+        # https://github.com/orgs/community/discussions/26409
+        ref: ${{github.event.pull_request.head.ref}}
+        repository: ${{github.event.pull_request.head.repo.full_name}}
+
+    - name: pre-commit checks - setup cache
+      uses: actions/cache@v3
+      with:
+        path: ~/.cache/pre-commit
+        key: pre-commit|${{ env.pythonLocation }}|${{ hashFiles('.pre-commit-config.yaml') }}
+
+    - name: pre-commit checks - run checks
+      uses: pre-commit/[email protected]
+      env:
+        # For merging PRs to master, skip the no-commit-to-branch check:
+        SKIP: no-commit-to-branch
+
+    - name: Install Python2 dependencies
+      run: |
+        #: Install Python 2.7 from Ubuntu 20.04 using apt-get install
+        sudo apt-get update && sudo apt-get install -y python2
+        curl -sSL https://bootstrap.pypa.io/pip/2.7/get-pip.py -o get-pip.py
+        python2 get-pip.py
+        if [ -f requirements.txt ]; then pip2 install -r requirements.txt; fi
+        if [ -f requirements-dev.txt ]; then pip2 install -r requirements-dev.txt; fi
+        pip2 install pytest pylint==1.9.4
+
+    - name: Run pylint-1.9.4
+      run: python2 -m pylint xtf-runner build/*.py
+
+    - name: Run python2 -m pytest to execute all unit and integration tests
+      run: python2 -m pytest -v -rA
+
   build:
 
     strategy:

.pre-commit-config.yaml

@@ -0,0 +1,108 @@
+#
+# Installation and activation of pre-commit:
+#
+# $ pip3 install pre-commit && pre-commit install
+#
+# With this, you get git pre-commit hook checks like shown in the terminal
+# on this page: https://pre-commit.com/index.html#usage
+#
+# It is easy to handle as a pre-commit hook once you get the hang of it.
+# If not skipped using git commit --no-verify, it makes each commit bisect-able,
+# whereas a check only run on a GitHub CI push will normally only run its checks
+# on the latest commit of a tree.
+#
+# However, it does not have to be used as a pre-commit hook:
+# Of course it can be used just for GitHub CI only:
+#
+# pre-commit runs in the GitHub Workflow of this project on each push and PR.
+# You can run it locally as well on demand after making changes to check them:
+#
+# $ pip3 install pre-commit
+# $ pre-commit run    # run only needed checks
+# $ pre-commit run -a # run all fixes and checks
+#
+# You can enable it as git pre-commit hook for your local clone using:
+# $ pre-commit install
+#
+# Then, the fixups and checks defined below run by default when you commit.
+#
+# To commit with all git pre-commit hooks skipped (for exceptional cases):
+# $ git commit --no-verify
+#
+# To commit with some checks skipped:
+# $ SKIP=mypy,pylint git commit -m "unchecked emergency commit"
+#
+# Documentation: https://pre-commit.com/#temporarily-disabling-hooks
+# All hooks: https://pre-commit.com/hooks.html
+#
+fail_fast: false
+default_stages: [commit, push]
+repos:
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.5.0
+    # https://github.com/pre-commit/pre-commit-hooks/blob/main/README.md:
+    hooks:
+    -   id: no-commit-to-branch
+        name: "ensure that you don't commit to the local master branch"
+        args: [--branch, master]
+        always_run: true
+    -   id: trailing-whitespace
+        name: 'check and fix files to have no trailing whitespace'
+        exclude: install-sh
+    -   id: end-of-file-fixer
+        name: 'check and fix that files have a trailing newline'
+        exclude: |
+            (?x)^(
+                arch/x86/include/arch/msr-index.h
+            )
+    -   id: mixed-line-ending
+        args: ['--fix=lf']
+        name: 'check and fix that line endings are line feeds'
+    -   id: check-added-large-files
+        args: ['--maxkb=12']
+        name: 'check that no large files are added'
+    -   id: check-executables-have-shebangs
+    -   id: debug-statements
+        name: 'check for debugger imports and breakpoint calls'
+    -   id: check-shebang-scripts-are-executable
+    -   id: check-merge-conflict
+    -   id: check-yaml
+        name: 'check the syntax of yaml files'
+-   repo: local
+    hooks:
+    -   id: pytest
+        name: run Python3 unit tests
+        entry: env PYTHONDEVMODE=yes python3 -m pytest -v
+        pass_filenames: false
+        language: python
+        types: [python]
+        additional_dependencies: [pytest]
+-   repo: https://github.com/akaihola/darker
+    rev: 1.7.2
+    hooks:
+    -   id: darker
+        name: format staged changes like black and isort would format them
+        args: [--skip-string-normalization, --isort, -tpy36]
+        additional_dependencies: [isort]
+-   repo: https://github.com/pre-commit/mirrors-mypy
+    rev: v1.8.0
+    hooks:
+    -   id: mypy
+        name: run mypy
+-   repo: https://github.com/pylint-dev/pylint
+    rev: v3.0.3
+    hooks:
+    -   id: pylint
+        name: run pylint
+        args: [--jobs=4, --spelling-dict=en_US]  # Spell checks: See .pylintrc
+        files: '(^xtf-runner|\.py)$'
+        log_file: ".git/pre-commit-pylint.log"
+        additional_dependencies: ['pylint[spelling]', pytest]
+-   repo: local
+    hooks:
+    -   id: git-diff # Ref: https://github.com/pre-commit/pre-commit/issues/1712
+        name: Show and fail on not staged changes, also fixups may make them
+        entry: git diff --exit-code
+        language: system
+        pass_filenames: false
+        always_run: true

.pylintrc

@@ -37,7 +37,7 @@ extension-pkg-whitelist=
 # Enable the message, report, category or checker with the given id(s). You can
 # either give multiple identifier separated by comma (,) or put this option
 # multiple time. See also the "--disable" option for examples.
-#enable=
+enable=spelling
 
 # Disable the message, report, category or checker with the given id(s). You
 # can either give multiple identifiers separated by comma (,) or put this
@@ -48,7 +48,24 @@ extension-pkg-whitelist=
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use"--disable=all --enable=classes
 # --disable=W"
-disable=bad-whitespace, bad-continuation, global-statement, star-args
+disable=bad-whitespace, bad-continuation, global-statement, star-args,
+    # Allow older pylint-1.9.x for Python2 to tolerate newer pylint options:
+    bad-option-value,
+    unrecognized-inline-option,
+    # Not real problems, returns on the same indentation level can be easier:
+    consider-using-with,
+    no-else-raise,
+    no-else-return,
+    multiple-imports,
+    len-as-condition,
+    # For Python3-only projects:
+    consider-using-f-string,
+    deprecated-module,
+    unrecognized-option,
+    unspecified-encoding,
+    use-implicit-booleaness-not-len,
+    useless-object-inheritance,
+    useless-option-value,
 
 
 [REPORTS]
@@ -103,6 +120,23 @@ ignore-docstrings=yes
 ignore-imports=no
 
 
+[SPELLING]
+
+# Spelling dictionary name. Available dictionaries: en (aspell), en_AU
+# (aspell), en_CA (aspell), en_GB (aspell), en_US (aspell).
+# To support spelling checks for older python2 pylint versions,
+# `sudo apt-get install -y libenchant-2-2` would be needed,
+# so we enable it for newer Python3 pylint in .pre-commit-config.yaml:
+#spelling-dict=en
+
+# A path to a file that contains the private dictionary; one word per line.
+spelling-private-dict-file=.pylintrc.project-dict.txt
+
+# Tells whether to store unknown words to the private dictionary (see the
+# --spelling-private-dict-file option) instead of raising a message.
+spelling-store-unknown-words=y
+
+
 [TYPECHECK]
 
 # Tells whether missing members accessed in mixin class should be ignored. A
@@ -337,4 +371,4 @@ max-public-methods=20
 
 # Exceptions that will emit a warning when being caught. Defaults to
 # "Exception"
-overgeneral-exceptions=Exception
+overgeneral-exceptions=builtins.Exception

.pylintrc.project-dict.txt

@@ -0,0 +1,49 @@
+CFG
+CWD
+ENVS
+Normalise
+O_CREAT
+O_RDONLY
+VM
+arg
+basestring
+config
+conftest
+env
+dev
+dirs
+entrypoint
+epilog
+hvm
+init
+invlpg
+iopl
+json
+logfile
+logline
+logpath
+mkcfg
+nonexisting
+pseduo
+pv
+py
+pylintrc
+pyright
+pytest
+reportMissingImports
+reportUndefinedVariable
+stdout
+subproc
+substitue
+sys
+toolstack
+unioned
+Unrecognised
+xenconsole
+xenconsoled
+xl
+xtf
+os
+pre
+src
+dir

build/mkcfg.py

@@ -9,7 +9,7 @@
 import sys, os
 
 # Usage: mkcfg.py $OUT $DEFAULT-CFG $EXTRA-CFG $VARY-CFG
-_, out, defcfg, vcpus, extracfg, varycfg = sys.argv
+_, out, defcfg, vcpus, extracfg, varycfg = sys.argv  # pylint: disable=unbalanced-tuple-unpacking
 
 # Evaluate environment and name from $OUT
 _, env, name = out.split('.')[0].split('-', 2)

build/mkinfo.py

@@ -1,10 +1,43 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
+"""
+mkcfg.py - Generate a configuration JSON file based on provided parameters.
+
+Usage:
+    python mkcfg.py $OUT $NAME $CATEGORY $ENVS $VARIATIONS
+
+Arguments:
+
+    $OUT:        Path to the output file where the generated JSON configuration
+                 will be saved.
+    $NAME:       Name to be assigned in the configuration.
+    $CATEGORY:   Category designation in the configuration.
+    $ENVS:       Optional space-separated list of environments (can be empty).
+    $VARIATIONS: Optional space-separated list of variations (can be empty).
+
+Description:
+
+    This script generates a JSON configuration file using provided parameters
+    and saves it to the specified output file. The generated JSON structure
+    includes fields for 'name', 'category', 'environments', and 'variations'.
+    The 'environments' and 'variations' fields can be populated with
+    space-separated lists if corresponding arguments ($ENVS and $VARIATIONS)
+    are provided.
+
+Example:
+
+    python mkcfg.py config.json ExampleConfig Utilities prod dev test
+
+    This example will create a configuration file named 'config.json' with
+       'name' as 'ExampleConfig',
+       'category' as 'Utilities', and
+       'environments' as ['prod', 'dev', 'test'].
+"""
 
 import sys, json
 
 # Usage: mkcfg.py $OUT $NAME $CATEGORY $ENVS $VARIATIONS
-_, out, name, cat, envs, variations = sys.argv
+_, out, name, cat, envs, variations = sys.argv  # pylint: disable=unbalanced-tuple-unpacking
 
 template = {
     "name": name,

include/xen/sysctl.h

@@ -0,0 +1,55 @@
+"""pytest fixtures for unit-testing functions in the xtf-runner script"""
+import os
+import sys
+
+import pytest
+
+
+def import_script_as_module(relative_script_path):
+    "Import a Python script without the .py extension as a python module"
+
+    script_path = os.path.join(os.path.dirname(__file__), relative_script_path)
+    module_name = os.path.basename(script_path)
+
+    if sys.version_info.major == 2:
+        # Use deprecated imp module because it needs also to run with Python27:
+        # pylint: disable-next=import-outside-toplevel
+        import imp  # pyright: ignore[reportMissingImports]
+
+        return imp.load_source(module_name, script_path)
+    else:
+        # For Python 3.11+: Import Python script without the .py extension:
+        # https://gist.github.com/bernhardkaindl/1aaa04ea925fdc36c40d031491957fd3:
+
+        # pylint: disable-next=import-outside-toplevel
+        from importlib import (  # pylint: disable=no-name-in-module
+            machinery,
+            util,
+        )
+
+        loader = machinery.SourceFileLoader(module_name, script_path)
+        spec = util.spec_from_loader(module_name, loader)
+        assert spec
+        assert spec.loader
+        module = util.module_from_spec(spec)
+        # It is probably a good idea to add the imported module to sys.modules:
+        sys.modules[module_name] = module
+        spec.loader.exec_module(module)
+        return module
+
+
+@pytest.fixture(scope="session")
+def imported_xtf_runner():
+    """Fixture to import a script as a module for unit testing its functions"""
+    return import_script_as_module("../../xtf-runner")
+
+
+@pytest.fixture(scope="function")
+def xtf_runner(imported_xtf_runner):  # pylint: disable=redefined-outer-name
+    """Test fixture for unit tests: initializes module for each test function"""
+    # Init the imported xtf-runner, so each unit test function gets it pristine:
+    # May be used to unit-test xtf-runner with other, different test dirs:
+    imported_xtf_runner._all_test_info = {}  # pylint: disable=protected-access
+    # The GitHub pre-commit action for does not start the checks in the src dir:
+    # os.chdir(os.path.join(os.path.dirname(__file__), "../.."))
+    return imported_xtf_runner