diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml new file mode 100644 index 00000000..feb36d06 --- /dev/null +++ b/.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 }} diff --git a/.github/workflows/differential-shellcheck.yml b/.github/workflows/differential-shellcheck.yml deleted file mode 100644 index 223984f4..00000000 --- a/.github/workflows/differential-shellcheck.yml +++ /dev/null @@ -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 }} diff --git a/.github/workflows/checks.yml b/.github/workflows/generate.yml similarity index 68% rename from .github/workflows/checks.yml rename to .github/workflows/generate.yml index af1bbb75..4e148b38 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/generate.yml @@ -1,10 +1,10 @@ -name: Checks +name: Generate on: [pull_request, push] jobs: - documentation: - name: "📚 Documentation" + generate_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: - name: "Regenerate Test Data" + generate_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 diff --git a/.github/workflows/tests.yml b/.github/workflows/test.yml similarity index 65% rename from .github/workflows/tests.yml rename to .github/workflows/test.yml index 7d50c190..4d7abdff 100644 --- a/.github/workflows/tests.yml +++ b/.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 \ - --pyargs "${{ matrix.test }}" \ - --rootdir=. \ - --cov-report=xml --cov=osbuild \ - -v + TEST_CATEGORY="${{ matrix.test }}" \ + tox -e "${{ matrix.environment }}" diff --git a/.gitignore b/.gitignore index 3e4f64b9..50a3602f 100644 --- a/.gitignore +++ b/.gitignore @@ -20,4 +20,6 @@ cov-int/ /docs/osbuild-manifest.5 venv -.venv \ No newline at end of file +.venv + +/.tox diff --git a/.ruff.toml b/.ruff.toml new file mode 100644 index 00000000..98c97c6e --- /dev/null +++ b/.ruff.toml @@ -0,0 +1,6 @@ +line-length = 120 + +ignore = [ + "E741", # ambiguous variable names + "E501", # line too long +] diff --git a/assemblers/org.osbuild.oci-archive b/assemblers/org.osbuild.oci-archive index b2db43bf..7f2c5eab 100755 --- a/assemblers/org.osbuild.oci-archive +++ b/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) diff --git a/assemblers/org.osbuild.qemu b/assemblers/org.osbuild.qemu index e7caca73..1d383892 100755 --- a/assemblers/org.osbuild.qemu +++ b/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"] diff --git a/osbuild/buildroot.py b/osbuild/buildroot.py index 0c33c5ee..5b47d70e 100644 --- a/osbuild/buildroot.py +++ b/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): diff --git a/osbuild/formats/v2.py b/osbuild/formats/v2.py index 5640b6ac..c0366d5e 100644 --- a/osbuild/formats/v2.py +++ b/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 -def output(manifest: Manifest, res: Dict, store: ObjectStore = None) -> Dict: +# pylint: disable=too-many-branches +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]: diff --git a/osbuild/host.py b/osbuild/host.py index 4857c2ac..c2dd87cf 100644 --- a/osbuild/host.py +++ b/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 diff --git a/osbuild/loop.py b/osbuild/loop.py index 5e1725f0..a0f5e811 100644 --- a/osbuild/loop.py +++ b/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: diff --git a/osbuild/objectstore.py b/osbuild/objectstore.py index d8f2cba8..4a19ce9f 100644 --- a/osbuild/objectstore.py +++ b/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 diff --git a/osbuild/pipeline.py b/osbuild/pipeline.py index 8013df0d..ab43387f 100644 --- a/osbuild/pipeline.py +++ b/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, diff --git a/osbuild/util/ostree.py b/osbuild/util/ostree.py index 70600397..6c5e4232 100644 --- a/osbuild/util/ostree.py +++ b/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 diff --git a/setup.cfg b/setup.cfg index a9e35cb8..cd92f299 100644 --- a/setup.cfg +++ b/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 diff --git a/stages/org.osbuild.dracut b/stages/org.osbuild.dracut index e98a25dc..8fb43761 100755 --- a/stages/org.osbuild.dracut +++ b/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") diff --git a/stages/org.osbuild.first-boot b/stages/org.osbuild.first-boot index 4b02b626..bd6708f5 100755 --- a/stages/org.osbuild.first-boot +++ b/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 diff --git a/stages/org.osbuild.grub2 b/stages/org.osbuild.grub2 index 91bea21f..93ccbd35 100755 --- a/stages/org.osbuild.grub2 +++ b/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") diff --git a/stages/org.osbuild.oci-archive b/stages/org.osbuild.oci-archive index 8704e50e..64845dee 100755 --- a/stages/org.osbuild.oci-archive +++ b/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) diff --git a/stages/org.osbuild.ovf b/stages/org.osbuild.ovf index 31515e2c..0aadbc33 100755 --- a/stages/org.osbuild.ovf +++ b/stages/org.osbuild.ovf @@ -36,7 +36,7 @@ SCHEMA_2 = """ OVF_TEMPLATE = """ - + Virtual disk information @@ -57,7 +57,7 @@ OVF_TEMPLATE = """ Virtual Hardware Family 0 image - vmx-15 + vmx-15 hertz * 10^6 @@ -88,7 +88,7 @@ OVF_TEMPLATE = """ SCSI Controller SCSI Controller 0 3 - VirtualSCSI + VirtualSCSI 6 diff --git a/test/src/test_pylint.py b/test/src/test_pylint.py deleted file mode 100644 index 1a5aac5f..00000000 --- a/test/src/test_pylint.py +++ /dev/null @@ -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)") diff --git a/tox.ini b/tox.ini new file mode 100644 index 00000000..54d23a19 --- /dev/null +++ b/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}'