update workflows and unit tests (#84)

2bndy5 · web-flow · commit ed7a981f0a21 · 2024-02-22T21:46:23.000-08:00
now uses the following reusable workflows:
- pre-commit.yml
- py-coverage.yml

## Adjusting Unit tests
- codecov-action v4 now requires a CODECOV token (which I added to repo secrets)
- `test_download_file` was passing unexpectedly (should have expectedly failed for `"latest"` release_tag).
- `test_path_warning` wouldn't pass when run in a venv (which is how I run tests locally).
- `test_get_sha` was using outdated sha512sum for Windows.
- CI now runs unit tests on all 3 OS and the reusable workflow will combine coverage data for the coverage reports.

* update pre-commit hooks; switch to ruff for linting and formatting in a pre-commit hook
* improve test runtime and coverage
  - now includes test code in coverage
  - does not download every supported version just to trigger lines in coverage data; only 1 version will suffice.
  - only download clang-format when testing `install_tool()` because it gets same coverage and downloads faster on all supported platforms.
diff --git a/.github/workflows/bump-version.py b/.github/workflows/bump-version.py
@@ -1,12 +1,15 @@
 import requests
 import sys
-sys.path.append('../../')
-from clang_tools import release_tag
+
+sys.path.append("../../")
+from clang_tools import release_tag  # noqa E402
 
 
 def get_latest_tag() -> str:
-    response = requests.get("https://api.github.com/repos/cpp-linter/clang-tools-static-binaries/releases/latest")
-    return response.json()['tag_name']
+    response = requests.get(
+        "https://api.github.com/repos/cpp-linter/clang-tools-static-binaries/releases/latest"
+    )
+    return response.json()["tag_name"]
 
 
 def update_tag(current_tag, latest_tag) -> None:
@@ -16,7 +19,7 @@ def update_tag(current_tag, latest_tag) -> None:
 
     updated_content = file_content.replace(current_tag, latest_tag)
 
-    with open(file_path, 'w') as file:
+    with open(file_path, "w") as file:
         file.write(updated_content)
 
 
diff --git a/.github/workflows/python-publish.yml b/.github/workflows/python-publish.yml
@@ -10,7 +10,6 @@ name: Upload Python Package
 
 on:
   release:
-    branches: [main]
     types: [published]
   workflow_dispatch:
 
diff --git a/.github/workflows/python-test.yml b/.github/workflows/python-test.yml
@@ -17,8 +17,12 @@ on:
 
 
 jobs:
-  build:
-    runs-on: ubuntu-latest
+  test:
+    strategy:
+      matrix:
+        os: [ ubuntu-latest, macos-latest, windows-latest ]
+      fail-fast: false
+    runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v4
 
@@ -32,20 +36,29 @@ jobs:
           python3 -m pip install --upgrade pip
           python3 -m pip install . -r requirements-dev.txt
 
-      - name: Run pre-commit
-        run: pre-commit run --all-files
+      - name: Collect Coverage
+        run: coverage run -m pytest -vv
 
-      - name: Collect coverage
-        run: |
-          coverage run -m pytest -vv
-          coverage report -m
-          coverage xml
+      - name: Upload coverage data
+        uses: actions/upload-artifact@v4
+        with:
+          name: coverage-data-${{ runner.os }}
+          path: .coverage*
 
-      - name: Upload coverage reports to Codecov
-        uses: codecov/codecov-action@v4
+  coverage-report:
+    needs: [test]
+    uses: cpp-linter/.github/.github/workflows/py-coverage.yml@main
+    secrets: inherit
+
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up Python 3.10
+        uses: actions/setup-python@v5
         with:
-          files: ./coverage.xml
-          verbose: true # optional (default = false)
+          python-version: "3.10"
 
       - name: Build wheel
         run: python -m pip wheel -w dist .
diff --git a/.github/workflows/run-pre-commit.yml b/.github/workflows/run-pre-commit.yml
@@ -0,0 +1,10 @@
+name: Run pre-commit
+
+on:
+  push:
+  pull_request:
+    types: opened
+
+jobs:
+  pre-commit:
+    uses: cpp-linter/.github/.github/workflows/pre-commit.yml@main
diff --git a/.gitignore b/.gitignore
@@ -4,7 +4,7 @@ clang_tools.egg-info
 clang_tools/__pycache__
 dist
 clang_tools/llvm-project*
-.coverage
+.coverage*
 coverage.xml
 htmlcov/
 .vscode/
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -12,15 +12,14 @@ repos:
       - id: debug-statements
       - id: requirements-txt-fixer
   - repo: https://github.com/asottile/pyupgrade
-    rev: v3.15.0
+    rev: v3.15.1
     hooks:
       - id: pyupgrade
-  - repo: https://github.com/pycqa/flake8
-    rev: '7.0.0'
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.2.2
     hooks:
-      - id: flake8
-        args: [--max-line-length=120]
-        exclude: ^(.github/workflows/bump-version.py)
+      - id: ruff
+      - id: ruff-format
   # - repo: local
   #   hooks:
   #     - id: pytest
diff --git a/clang_tools/install.py b/clang_tools/install.py
@@ -39,33 +39,32 @@ def is_installed(tool_name: str, version: str) -> Optional[Path]:
     )
     try:
         result = subprocess.run(
-            [exe_name, "--version"], stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True
+            [exe_name, "--version"],
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            check=True,
         )
     except (FileNotFoundError, subprocess.CalledProcessError):
         return None  # tool is not installed
     ver_num = RE_PARSE_VERSION.search(result.stdout)
     print(
         f"Found a installed version of {tool_name}:",
         ver_num.groups(0)[0].decode(encoding="utf-8"),
-        end=" "
+        end=" ",
     )
     path = shutil.which(exe_name)  # find the installed binary
     if path is None:
         print()  # print end-of-line
         return None  # failed to locate the binary
     path = Path(path).resolve()
     print("at", str(path))
-    ver_num = ver_num.groups(0)[0].decode(encoding="utf-8").split(".")  # pragma: no cover
-    if (
-        ver_num is None or ver_num[0] != ver_major
-    ):
+    ver_num = ver_num.groups(0)[0].decode(encoding="utf-8").split(".")
+    if ver_num is None or ver_num[0] != ver_major:
         return None  # version is unknown or not the desired major release
     return path
 
 
-def clang_tools_binary_url(
-    tool: str, version: str, tag: str = release_tag
-) -> str:
+def clang_tools_binary_url(tool: str, version: str, tag: str = release_tag) -> str:
     """Assemble the URL to the binary.
 
     :param tool: The name of the tool to download.
diff --git a/clang_tools/main.py b/clang_tools/main.py
@@ -23,8 +23,8 @@ def get_parser() -> argparse.ArgumentParser:
     parser.add_argument(
         "-t",
         "--tool",
-        nargs='+',
-        default=['clang-format', 'clang-tidy'],
+        nargs="+",
+        default=["clang-format", "clang-tidy"],
         metavar="TOOL",
         help="Specify which tool(s) to install.",
     )
@@ -67,12 +67,16 @@ def main():
         uninstall_clang_tools(args.uninstall, args.directory)
     if args.install:
         install_clang_tools(
-            args.install, args.tool, args.directory, args.overwrite, args.no_progress_bar
+            args.install,
+            args.tool,
+            args.directory,
+            args.overwrite,
+            args.no_progress_bar,
         )
     else:
         print(
             f"{YELLOW}Nothing to do because `--install` and `--uninstall`",
-            f"was not specified.{RESET_COLOR}"
+            f"was not specified.{RESET_COLOR}",
         )
         parser.print_help()
 
diff --git a/clang_tools/util.py b/clang_tools/util.py
@@ -48,7 +48,7 @@ def download_file(url: str, file_name: str, no_progress_bar: bool) -> Optional[s
         return None
     assert response.length is not None
     length = response.length
-    buffer = b''
+    buffer = b""
     progress_bar = "=" if check_install_os() == "windows" else "█"
     while len(buffer) < length:
         block_size = int(length / 20)
diff --git a/docs/conf.py b/docs/conf.py
@@ -37,7 +37,7 @@
     "sphinx.ext.intersphinx",
 ]
 
-intersphinx_mapping = {'python': ('https://docs.python.org/3', None)}
+intersphinx_mapping = {"python": ("https://docs.python.org/3", None)}
 
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ["_templates"]
@@ -78,7 +78,7 @@
             "toggle": {
                 "icon": "material/lightbulb-outline",
                 "name": "Switch to dark mode",
-            }
+            },
         },
         {
             "media": "(prefers-color-scheme: dark)",
@@ -88,7 +88,7 @@
             "toggle": {
                 "icon": "material/lightbulb",
                 "name": "Switch to light mode",
-            }
+            },
         },
     ],
     "features": [
@@ -98,7 +98,7 @@
         "toc.sticky",
         "toc.follow",
         "search.share",
-    ]
+    ],
 }
 
 object_description_options = [
@@ -122,9 +122,7 @@ def setup(app: Sphinx):
         if arg.default != "==SUPPRESS==":
             doc += f"    :Default: ``{repr(arg.default)}``\n\n"
         description = (
-            ""
-            if arg.help is None
-            else "    %s\n" % (arg.help.replace('\n', '\n    '))
+            "" if arg.help is None else "    %s\n" % (arg.help.replace("\n", "\n    "))
         )
         doc += description
     cli_doc = Path(app.srcdir, "cli_args.rst")
diff --git a/pyproject.toml b/pyproject.toml
@@ -61,9 +61,11 @@ show_column_numbers = true
 [tool.coverage]
 [tool.coverage.run]
 dynamic_context = "test_function"
+parallel = true
+relative_files = true
 omit = [
     # don't include tests in coverage
-    "tests/*",
+    # "tests/*",
 ]
 
 [tool.coverage.json]
@@ -79,7 +81,5 @@ exclude_lines = [
     "pragma: no cover",
     # ignore any branch that makes the module executable
     'if __name__ == "__main__"',
-    # we test parts of these algorithms separately
-    "def uninstall_clang_tools",
     "def main",
 ]
diff --git a/tests/clang-format-12_windows-amd64.sha512sum b/tests/clang-format-12_windows-amd64.sha512sum
@@ -1 +1 @@
-4b96e93eedcbceafc0dea62cf46a002ba3d2a165215a83fbfe1e2a73d21888a3b36bc7da092fcad38fa56e615d7cb9d375a34eddc642ee08a4a29b836f938762 *clang-format-12_windows-amd64
+8c03bcc6f4166cd05039a6217f0c0834ed2a321e0a2a64a16b87923e94ee33a980c09b1e78e1aba5e9ea045ac843d8d51b32fcfe44fc5d98d840efb16b359bb6 *clang-format-12_windows-amd64
diff --git a/tests/test_install.py b/tests/test_install.py
@@ -9,12 +9,16 @@
     create_sym_link,
     install_tool,
     install_clang_tools,
-    uninstall_tool,
+    is_installed,
+    uninstall_clang_tools,
 )
 
 
 @pytest.mark.parametrize("version", [str(v) for v in range(7, 17)] + ["12.0.1"])
-@pytest.mark.parametrize("tool_name", ["clang-format", "clang-tidy", "clang-query", "clang-apply-replacements"])
+@pytest.mark.parametrize(
+    "tool_name",
+    ["clang-format", "clang-tidy", "clang-query", "clang-apply-replacements"],
+)
 def test_clang_tools_binary_url(tool_name: str, version: str):
     """Test `clang_tools_binary_url()`"""
     url = clang_tools_binary_url(tool_name, version)
@@ -50,19 +54,27 @@ def test_create_symlink(monkeypatch: pytest.MonkeyPatch, tmp_path: Path):
     assert not create_sym_link(tool_name, version, str(tmp_path), True)
 
 
-@pytest.mark.parametrize("version", [str(v) for v in range(10, 17)] + ["12.0.1"])
+@pytest.mark.parametrize("version", ["12"])
 def test_install_tools(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, version: str):
     """Test install tools to a temp directory."""
     monkeypatch.chdir(tmp_path)
-    for tool_name in ("clang-format", "clang-tidy"):
-        assert install_tool(tool_name, version, str(tmp_path), False)
-        # invoking again should return False
-        assert not install_tool(tool_name, version, str(tmp_path), False)
-        # uninstall the tool deliberately
-        uninstall_tool(tool_name, version, str(tmp_path))
-        assert f"{tool_name}-{version}{suffix}" not in [
-            fd.name for fd in tmp_path.iterdir()
-        ]
+    tool_name = "clang-format"
+
+    assert install_tool(tool_name, version, str(tmp_path), False)
+    # invoking again should return False
+    assert not install_tool(tool_name, version, str(tmp_path), False)
+    # uninstall the tool deliberately
+    uninstall_clang_tools(version, str(tmp_path))
+    assert f"{tool_name}-{version}{suffix}" not in [
+        fd.name for fd in tmp_path.iterdir()
+    ]
+
+
+@pytest.mark.parametrize("version", ["0"])
+def test_is_installed(version: str):
+    """Test if installed version matches specified ``version``"""
+    tool_path = is_installed("clang-format", version=version)
+    assert tool_path is None
 
 
 def test_path_warning(capsys: pytest.CaptureFixture):
@@ -74,6 +86,8 @@ def test_path_warning(capsys: pytest.CaptureFixture):
     try:
         install_clang_tools("x", "x", ".", False, False)
     except OSError as exc:
-        result = capsys.readouterr()
-        assert "directory is not in your environment variable PATH" in result.out
+        if install_dir_name(".") not in os.environ.get("PATH"):  # pragma: no cover
+            # this warning does not happen in an activated venv
+            result = capsys.readouterr()
+            assert "directory is not in your environment variable PATH" in result.out
         assert "Failed to download" in exc.args[0]
diff --git a/tests/test_main.py b/tests/test_main.py
@@ -1,5 +1,5 @@
 """Tests that relate to the main.py module."""
-from typing import Optional
+from typing import Optional, List
 from argparse import ArgumentParser
 import pytest
 from clang_tools.main import get_parser
@@ -14,6 +14,7 @@ class Args:
     overwrite: bool = False
     no_progress_bar: bool = False
     uninstall: Optional[str] = None
+    tool: List[str] = ["clang-format", "clang-tidy"]
 
 
 @pytest.fixture
@@ -37,15 +38,8 @@ def test_cli_switch(switch_name: str, parser: ArgumentParser):
     assert getattr(args, switch_name.replace("-", "_"))
 
 
-@pytest.mark.parametrize("name, default_value", [
-    ("install", None),
-    ("uninstall", None),
-    ("overwrite", False),
-    ("no_progress_bar", False),
-    ("directory", ""),
-    ("tool", ['clang-format', 'clang-tidy'])
-])
-def test_default_args(parser: ArgumentParser, name, default_value):
+def test_default_args(parser: ArgumentParser):
     """Test the default values of CLI args"""
     args = parser.parse_args([])
-    assert getattr(args, name) == default_value
+    for name, value in args.__dict__.items():
+        assert getattr(Args, name) == value
diff --git a/tests/test_util.py b/tests/test_util.py
@@ -19,7 +19,7 @@ def test_check_install_os():
 def test_download_file(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, tag: str):
     """Test that deliberately fails to download a file."""
     monkeypatch.chdir(str(tmp_path))
-    url = clang_tools_binary_url("clang-format", "12", tag=release_tag)
+    url = clang_tools_binary_url("clang-format", "12", tag=tag)
     file_name = download_file(url, "file.tar.gz", True)
     assert file_name is not None
 
