tox: add tox

`tox` is a standard testing tool for Python projects, this allows you to
test locally with all your installed Python version with the following
command:

`tox -m test -p all`

To run the tests in parallel for all supported Python versions.

To run linters or type analysis:

```
tox -m lint -p all
tox -m type -p all
```

This commit *also* disables the `import-error` warning from `pylint`,
not all Python versions have the system-installed Python libraries
available and they can't be fetched from PyPI.

Some linters have been added and the general order linters run in has
been changed. This allows for quicker test failure when running
`tox -m lint`. As a consequence the `test_pylint` test has been removed
as it's role can now be fulfilled by `tox`.

Other assorted linter fixes due to newer versions:
- use a str.join method (`consider-using-join`)
- fix various (newer) mypy and pylint issues
- comments starting with `#` and no space due to `autopep8`

This also changes our CI to use the new `tox` setup and on top of that
pins the versions of linters used. This might move into separate
requirements.txt files later on to allow for easier updating of those
dependencies.

.github/workflows/check.yml

@@ -0,0 +1,72 @@
+name: Checks
+
+on: [pull_request, push]
+
+permissions:
+  contents: read
+
+jobs:
+  spelling_checker:
+    name: "Spelling"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: codespell-project/actions-codespell@master
+        with:
+          ignore_words_list: msdos, pullrequest
+          skip: ./.git,coverity,rpmbuild,samples
+
+  python_code_linters:
+      name: "Python Linters"
+      runs-on: ubuntu-latest
+      strategy:
+        fail-fast: false
+        matrix:
+          linter:
+          - "ruff"
+          - "pylint"
+          - "autopep8"
+          - "isort"
+      steps:
+      - name: "Clone Repository"
+        uses: actions/checkout@v3
+      - name: "Run Linters"
+        uses: osbuild/containers/src/actions/privdocker@552e30cf1b4ed19c6ddaa57f96c342b3dff4227b
+        with:
+          image: ghcr.io/osbuild/osbuild-ci:latest-202304251412
+          run: |
+            tox -e "${{ matrix.linter }}"
+
+  python_code_types:
+    name: "Python Typing"
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        typer:
+        - "mypy"
+    steps:
+    - name: "Clone Repository"
+      uses: actions/checkout@v3
+    - name: "Run Linters"
+      uses: osbuild/containers/src/actions/privdocker@552e30cf1b4ed19c6ddaa57f96c342b3dff4227b
+      with:
+        image: ghcr.io/osbuild/osbuild-ci:latest-202304251412
+        run: |
+          tox -e "${{ matrix.typer }}"
+
+  shell_linters:
+    name: "Shell Linters"
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: "Clone Repository"
+      uses: actions/checkout@v3
+      with:
+        fetch-depth: 0
+
+    - name: "Differential ShellCheck"
+      uses: redhat-plumbers-in-action/differential-shellcheck@v3
+      with:
+        severity: warning
+        token: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/differential-shellcheck.yml

@@ -1,24 +0,0 @@
-name: "Differential ShellCheck"
-on:
-  pull_request:
-    branches: [main]
-
-permissions:
-  contents: read
-
-jobs:
-  lint:
-    name: "Differential Shell Check"
-    runs-on: ubuntu-latest
-
-    steps:
-    - name: "Clone Repository"
-      uses: actions/checkout@v3
-      with:
-        fetch-depth: 0
-
-    - name: "Differential ShellCheck"
-      uses: redhat-plumbers-in-action/differential-shellcheck@v3
-      with:
-        severity: warning
-        token: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/generate.yml

@@ -1,10 +1,10 @@
-name: Checks
+name: Generate
 
 on: [pull_request, push]
 
 jobs:
-  documentation:
+  generate_documentation:
-    name: "📚 Documentation"
+    name: "Documentation"
     runs-on: ubuntu-latest
     container:
       image: docker.io/library/python:3.7
@@ -33,8 +33,8 @@ jobs:
           test -d docs
           test -f docs/osbuild.1
 
-  test_data:
+  generate_test_data:
-    name: "Regenerate Test Data"
+    name: "Test Data"
     runs-on: ubuntu-latest
     steps:
     - name: "Clone Repository"
@@ -42,17 +42,7 @@ jobs:
     - name: "Regenerate Test Data"
       uses: osbuild/containers/src/actions/privdocker@552e30cf1b4ed19c6ddaa57f96c342b3dff4227b
       with:
-        image: ghcr.io/osbuild/osbuild-ci:latest-202304110753
+        image: ghcr.io/osbuild/osbuild-ci:latest-202304251412
         run: |
           make test-data
           git diff --exit-code -- ./test/data
-
-  codespell:
-    name: "Spell check"
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-      - uses: codespell-project/actions-codespell@master
-        with:
-          ignore_words_list: msdos, pullrequest
-          skip: ./.git,coverity,rpmbuild,samples

.github/workflows/test.yml

@@ -4,7 +4,7 @@ on: [pull_request, push]
 
 jobs:
   test_suite:
-    name: "Test Suite"
+    name: "Unittest"
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
@@ -19,17 +19,21 @@ jobs:
         - "test.run.test_noop"
         - "test.run.test_sources"
         - "test.run.test_stages"
-        - "test.src"
+        environment:
+        - "py36"
+        - "py37"
+        - "py38"
+        - "py39"
+        - "py310"
+        - "py311"
+        - "py312"
     steps:
     - name: "Clone Repository"
       uses: actions/checkout@v3
-    - name: "Run Tests"
+    - name: "Run"
       uses: osbuild/containers/src/actions/privdocker@552e30cf1b4ed19c6ddaa57f96c342b3dff4227b
       with:
-        image: ghcr.io/osbuild/osbuild-ci:latest-202304110753
+        image: ghcr.io/osbuild/osbuild-ci:latest-202304251412
         run: |
-          python3 -m pytest \
+          TEST_CATEGORY="${{ matrix.test }}" \
-            --pyargs "${{ matrix.test }}" \
+          tox -e "${{ matrix.environment }}"
-            --rootdir=. \
-            --cov-report=xml --cov=osbuild \
-            -v

.gitignore

@@ -20,4 +20,6 @@ cov-int/
 /docs/osbuild-manifest.5
 
 venv
-.venv
+.venv
+
+/.tox

.ruff.toml

@@ -0,0 +1,6 @@
+line-length = 120
+
+ignore = [
+    "E741",  # ambiguous variable names
+    "E501",  # line too long
+]

assemblers/org.osbuild.oci-archive

@@ -228,7 +228,7 @@ def create_oci_dir(tree, output_dir, options):
     blobs = os.path.join(output_dir, "blobs", "sha256")
     os.makedirs(blobs)
 
-    ## layers / rootfs
+    # layers / rootfs
 
     digest, info = blobs_add_layer(blobs, tree)
 

assemblers/org.osbuild.qemu

@@ -502,7 +502,7 @@ def grub2_partition_id(pt: PartitionTable):
     return label2grub[pt.label]
 
 
-#pylint: disable=too-many-branches
+# pylint: disable=too-many-branches
 def install_grub2(image: str, pt: PartitionTable, options):
     """Install grub2 to image"""
     platform = options.get("platform", "i386-pc")
@@ -637,7 +637,7 @@ def install_zipl(root: str, device: str, pt: PartitionTable):
                    check=True)
 
 
-#pylint: disable=too-many-branches
+# pylint: disable=too-many-branches
 def main(tree, output_dir, options, loop_client):
     fmt = options["format"]
     filename = options["filename"]

osbuild/buildroot.py

@@ -15,7 +15,7 @@ import stat
 import subprocess
 import tempfile
 import time
-from typing import Optional, Set
+from typing import Set
 
 from osbuild.api import BaseAPI
 from osbuild.util import linux
@@ -101,7 +101,7 @@ class BuildRoot(contextlib.AbstractContextManager):
         self.proc = None
         self.tmp = None
         self.mount_boot = True
-        self.caps: Optional[set] = None
+        self.caps = None
 
     @staticmethod
     def _mknod(path, name, mode, major, minor):

osbuild/formats/v2.py

@@ -2,7 +2,7 @@
 
 Second, and current, version of the manifest description
 """
-from typing import Any, Dict
+from typing import Any, Dict, Optional
 
 from osbuild.meta import Index, ModuleInfo, ValidationResult
 
@@ -197,7 +197,7 @@ def sort_devices(devices: Dict) -> Dict:
             desc = devices[name]
 
             parent = desc.get("parent")
-            if parent and not parent in result:
+            if parent and parent not in result:
                 # if the parent is not in the `result` list, it must
                 # be in `todo`; otherwise it is missing
                 if parent not in todo:
@@ -391,8 +391,8 @@ def load(description: Dict, index: Index) -> Manifest:
     return manifest
 
 
-#pylint: disable=too-many-branches
+# pylint: disable=too-many-branches
-def output(manifest: Manifest, res: Dict, store: ObjectStore = None) -> Dict:
+def output(manifest: Manifest, res: Dict, store: Optional[ObjectStore] = None) -> Dict:
     """Convert a result into the v2 format"""
 
     def collect_metadata(p: Pipeline) -> Dict[str, Any]:

osbuild/host.py

@@ -272,7 +272,7 @@ class Service(abc.ABC):
                 # an exception in `sock.send` later.
                 self._check_fds(reply_fds)
 
-            except:  # pylint: disable=bare-except
+            except Exception:  # pylint: disable=broad-exception-caught
                 reply_fds = self._close_all(reply_fds)
                 _, val, tb = sys.exc_info()
                 reply = self.protocol.encode_exception(val, tb)
@@ -351,7 +351,7 @@ class ServiceClient:
     def call_with_fds(self, method: str,
                       args: Optional[Union[List[str], Dict[str, Any]]] = None,
                       fds: Optional[List[int]] = None,
-                      on_signal: Callable[[Any, Optional[Iterable[int]]], None] = None
+                      on_signal: Optional[Callable[[Any, Optional[Iterable[int]]], None]] = None
                       ) -> Tuple[Any, Optional[Iterable[int]]]:
         """
         Remotely call a method and return the result, including file

osbuild/loop.py

@@ -119,7 +119,7 @@ class Loop:
 
         self.devname = f"loop{minor}"
         self.minor = minor
-        self.on_close: Optional[Callable[["Loop"], None]] = None
+        self.on_close = None
 
         with contextlib.ExitStack() as stack:
             if not dir_fd:

osbuild/objectstore.py

@@ -338,7 +338,7 @@ class ObjectStore(contextlib.AbstractContextManager):
 
     @property
     def active(self) -> bool:
-        #pylint: disable=protected-access
+        # pylint: disable=protected-access
         return self.cache._is_active()
 
     @property

osbuild/pipeline.py

@@ -383,7 +383,7 @@ class Manifest:
 
     def __init__(self):
         self.pipelines = collections.OrderedDict()
-        self.sources: List[Source] = []
+        self.sources = []
 
     def add_pipeline(
         self,

osbuild/util/ostree.py

@@ -6,7 +6,8 @@ import subprocess
 import sys
 import tempfile
 import typing
-from typing import Any, List
+# pylint doesn't understand the string-annotation below
+from typing import Any, List  # pylint: disable=unused-import
 
 from .types import PathLike
 
@@ -222,7 +223,7 @@ class SubIdsDB:
     """
 
     def __init__(self) -> None:
-        self.db: collections.OrderedDict[str, Any] = collections.OrderedDict()
+        self.db: 'collections.OrderedDict[str, Any]' = collections.OrderedDict()
 
     def read(self, fp) -> int:
         idx = 0

setup.cfg

@@ -9,7 +9,8 @@ disable=missing-docstring,
         consider-using-with,
         consider-using-from-import,
         line-too-long,
-        useless-option-value
+        useless-option-value,
+        import-error
 
 [pylint.TYPECHECK]
 ignored-classes=osbuild.loop.LoopInfo

stages/org.osbuild.dracut

@@ -129,7 +129,7 @@ def yesno(name: str, value: bool) -> str:
     return f"--{prefix}{name}"
 
 
-#pylint: disable=too-many-branches
+# pylint: disable=too-many-branches
 def main(tree, options):
     kernels = options["kernel"]
     compress = options.get("compress")

stages/org.osbuild.first-boot

@@ -50,7 +50,7 @@ After=network-online.target"""
 
     execs = "\n"
     for command in commands:
-        execs += f"ExecStart={command}\n"
+        execs += f"ExecStart={command}\n"  # pylint: disable=consider-using-join
 
     service = f"""[Unit]
 Description=OSBuild First Boot Service

stages/org.osbuild.grub2

@@ -484,7 +484,7 @@ class GrubConfig:
         return data
 
 
-#pylint: disable=too-many-statements,too-many-branches
+# pylint: disable=too-many-statements,too-many-branches
 def main(tree, options):
     root_fs = options.get("rootfs")
     boot_fs = options.get("bootfs")

stages/org.osbuild.oci-archive

@@ -309,7 +309,7 @@ def create_oci_dir(inputs, output_dir, options, create_time):
     blobs = os.path.join(output_dir, "blobs", "sha256")
     os.makedirs(blobs)
 
-    ## layers / rootfs
+    # layers / rootfs
     for ip in sorted(inputs.keys()):
         tree = inputs[ip]["path"]
         digest, info = blobs_add_layer(blobs, tree)

stages/org.osbuild.ovf

@@ -36,7 +36,7 @@ SCHEMA_2 = """
 OVF_TEMPLATE = """<?xml version="1.0"?>
 <Envelope xmlns="http://schemas.dmtf.org/ovf/envelope/1" xmlns:cim="http://schemas.dmtf.org/wbem/wscim/1/common" xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1" xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData" xmlns:vmw="http://www.vmware.com/schema/ovf" xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
   <References>
-	  <File ovf:href="{image_name}" ovf:id="file1" ovf:size="{vmdk_size}"/>
+    <File ovf:href="{image_name}" ovf:id="file1" ovf:size="{vmdk_size}"/>
   </References>
   <DiskSection>
     <Info>Virtual disk information</Info>
@@ -57,7 +57,7 @@ OVF_TEMPLATE = """<?xml version="1.0"?>
         <vssd:ElementName>Virtual Hardware Family</vssd:ElementName>
         <vssd:InstanceID>0</vssd:InstanceID>
         <vssd:VirtualSystemIdentifier>image</vssd:VirtualSystemIdentifier>
-	<vssd:VirtualSystemType>vmx-15</vssd:VirtualSystemType>
+        <vssd:VirtualSystemType>vmx-15</vssd:VirtualSystemType>
       </System>
       <Item>
         <rasd:AllocationUnits>hertz * 10^6</rasd:AllocationUnits>
@@ -88,7 +88,7 @@ OVF_TEMPLATE = """<?xml version="1.0"?>
         <rasd:Description>SCSI Controller</rasd:Description>
         <rasd:ElementName>SCSI Controller 0</rasd:ElementName>
         <rasd:InstanceID>3</rasd:InstanceID>
-	<rasd:ResourceSubType>VirtualSCSI</rasd:ResourceSubType>
+        <rasd:ResourceSubType>VirtualSCSI</rasd:ResourceSubType>
         <rasd:ResourceType>6</rasd:ResourceType>
       </Item>
       <Item>

test/src/test_pylint.py

@@ -1,110 +0,0 @@
-#
-# Run `pylint` on all python sources.
-#
-
-import os
-import subprocess
-
-import pytest
-
-from .. import test
-
-
-@pytest.fixture(name="source_files")
-def discover_source_files():
-
-    # Evaluate whether `path` is a python file. This assumes that the file is
-    # opened by the caller, anyway, hence it might invoke `file` to detect the
-    # file type if in doubt.
-    def is_python_script(path):
-        if path.endswith(".py"):
-            return True
-
-        mime_valid = [
-            "text/x-python",
-            "text/x-script.python",
-        ]
-
-        mime_file = subprocess.check_output(
-            [
-                "file",
-                "-bi",
-                "--",
-                path,
-            ],
-            encoding="utf-8",
-        )
-
-        return any(mime_file.startswith(v) for v in mime_valid)
-
-    # Enumerate all repository files. This will invoke `git ls-tree` to list
-    # all files. This also requires a temporary change of directory, since git
-    # operates on the current directory and does not allow path arguments.
-    def list_files(path):
-        cwd = os.getcwd()
-        os.chdir(path)
-        files = subprocess.check_output(
-            [
-                "git",
-                "ls-tree",
-                "-rz",
-                "--full-tree",
-                "--name-only",
-                "HEAD",
-            ],
-            encoding="utf-8",
-        )
-        os.chdir(cwd)
-
-        files = files.split('\x00')
-        return (os.path.join(path, f) for f in files)
-
-    if not test.TestBase.have_test_checkout():
-        pytest.skip("no test-checkout access")
-
-    files = list_files(test.TestBase.locate_test_checkout())
-    files = filter(is_python_script, files)
-    return list(files)
-
-
-def test_pylint(source_files):
-    #
-    # Run `pylint` on all python sources. We simply use `find` to locate
-    # all `*.py` files, and then manually select the reverse-domain named
-    # modules we have.
-    #
-
-    r = subprocess.run(["pylint"] + source_files, check=False)
-    if r.returncode != 0:
-        pytest.fail("pylint issues detected")
-
-
-@pytest.mark.skipif(not test.TestBase.have_mypy(), reason="mypy not available")
-def test_mypy():
-    #
-    # Run `mypy` on osbuild sources.
-    #
-
-    r = subprocess.run(["mypy", "osbuild/"], check=False)
-    if r.returncode != 0:
-        pytest.fail("mypy issues detected")
-
-
-@pytest.mark.skipif(not test.TestBase.have_isort(), reason="isort not available")
-def test_isort(source_files):
-    #
-    # Run `isort` on all python sources. We simply use `find` to locate
-    # all `*.py` files, and then manually select the reverse-domain named
-    # modules we have.
-    #
-
-    r = subprocess.run(["isort", "--check", "--diff"] + source_files, check=False)
-    if r.returncode != 0:
-        pytest.fail("isort issues detected")
-
-
-@pytest.mark.skipif(not test.TestBase.have_autopep8(), reason="autopep8 not available")
-def test_autopep8(source_files):
-    r = subprocess.run(["autopep8-3", "--diff", "--exit-code"] + source_files, check=False)
-    if r.returncode != 0:
-        pytest.fail("autopep8 has detected changes (see diff)")

tox.ini

@@ -0,0 +1,67 @@
+[tox]
+env_list =
+    py{36,37,38,39,310,311}
+    lint
+    type
+
+labels =
+    test = py{36,37,38,39,310,311}
+    lint = ruff, isort, autopep8, pylint
+    type = mypy
+
+[testenv]
+description = "run osbuild unit tests"
+deps =
+    pytest
+    jsonschema
+    mako
+    iniparse
+    pyyaml
+
+setenv =
+    LINTABLES = osbuild/ assemblers/* devices/* inputs/* mounts/* runners/* sources/* stages/*
+    TYPEABLES = osbuild
+
+passenv =
+    TEST_CATEGORY
+
+commands =
+    bash -c 'python -m pytest --pyargs --rootdir=. {env:TEST_CATEGORY}'
+
+allowlist_externals =
+    bash
+
+[testenv:ruff]
+deps =
+    ruff==0.0.263
+
+commands =
+    bash -c 'python -m ruff {env:LINTABLES}'
+
+[testenv:isort]
+deps =
+    isort==5.12.0
+
+commands =
+    bash -c 'python -m isort --check --diff {env:LINTABLES}'
+
+[testenv:autopep8]
+deps =
+    autopep8==2.0.2
+
+commands =
+    bash -c 'python -m autopep8 -r --diff --exit-code {env:LINTABLES}'
+
+[testenv:pylint]
+deps =
+    pylint==2.17.3
+
+commands =
+    bash -c 'python -m pylint {env:LINTABLES}'
+
+[testenv:mypy]
+deps =
+    mypy==1.2.0
+
+commands =
+    bash -c 'python -m mypy {env:TYPEABLES}'