test-cache: Fix XDG tests, make tests more consistent (#880)

hseg · woodruffw · web-flow · commit 3f2c8fcff94f · 2025-06-18T10:38:56.000-04:00
Co-authored-by: William Woodruff &lt;william@trailofbits.com&gt;
Co-authored-by: William Woodruff &lt;william@yossarian.net&gt;
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -35,11 +35,83 @@ jobs:
       - name: test
         run: make test PIP_AUDIT_EXTRA=test
 
+      - name: Upload coverage data
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        with:
+          name: coverage-data-${{ matrix.python }}
+          path: .coverage.*
+          include-hidden-files: true
+          if-no-files-found: ignore
+
+  test-windows:
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        with:
+          persist-credentials: false
+
+      - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0
+        with:
+          # Always test with latest Python on Windows.
+          python-version: "3.x"
+          cache: "pip"
+          cache-dependency-path: pyproject.toml
+
+      - name: test
+        run: make test PIP_AUDIT_EXTRA=test
+
+      - name: Upload coverage data
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        with:
+          name: coverage-data-windows
+          path: .coverage.*
+          include-hidden-files: true
+          if-no-files-found: ignore
+
+  coverage:
+    name: Combine & check coverage
+    if: always()
+    needs: [test, test-windows]
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        with:
+          persist-credentials: false
+
+      - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
+        with:
+          python-version: "3.x"
+
+      - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0
+        with:
+          pattern: coverage-data-*
+          merge-multiple: true
+
+      - name: Combine coverage & fail if it's <100%
+        run: |
+          make dev PIP_AUDIT_EXTRA=cov
+
+          ./env/bin/python -Im coverage combine
+          ./env/bin/python -Im coverage html --skip-covered --skip-empty
+
+          # Report and write to summary.
+          ./env/bin/python -Im coverage report --format=markdown >> "${GITHUB_STEP_SUMMARY}"
+
+          # Report again and fail if under 100%.
+          ./env/bin/python -Im coverage report --fail-under=100
+
+      - name: Upload HTML report if check failed
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        with:
+          name: html-report
+          path: htmlcov
+        if: ${{ failure() }}
+
   all-tests-pass:
     if: always()
 
-    needs:
-    - test
+    needs: [coverage]
 
     runs-on: ubuntu-latest
 
diff --git a/Makefile b/Makefile
@@ -23,14 +23,10 @@ PIP_AUDIT_EXTRA := dev
 
 # If the user selects a specific test pattern to run, set `pytest` to fail fast
 # and only run tests that match the pattern.
-# Otherwise, run all tests and enable coverage assertions, since we expect
-# complete test coverage.
 ifneq ($(TESTS),)
 	TEST_ARGS := -x -k $(TESTS)
-	COV_ARGS :=
 else
 	TEST_ARGS :=
-	COV_ARGS := --fail-under 100
 endif
 
 .PHONY: all
@@ -67,8 +63,7 @@ reformat:
 .PHONY: test tests
 test tests: $(VENV)/pyvenv.cfg
 	. $(VENV_BIN)/activate && \
-		pytest --cov=$(PY_MODULE) $(T) $(TEST_ARGS) && \
-		python -m coverage report -m $(COV_ARGS)
+	    coverage run -m pytest $(T) $(TEST_ARGS)
 
 .PHONY: doc
 doc: $(VENV)/pyvenv.cfg
diff --git a/pip_audit/_cli.py b/pip_audit/_cli.py
@@ -34,7 +34,13 @@
 )
 from pip_audit._service import EcosystemsService, OsvService, PyPIService
 from pip_audit._service.interface import ConnectionError as VulnServiceConnectionError
-from pip_audit._service.interface import ResolvedDependency, SkippedDependency, VulnerabilityService
+from pip_audit._service.interface import (
+    Dependency,
+    ResolvedDependency,
+    SkippedDependency,
+    VulnerabilityResult,
+    VulnerabilityService,
+)
 from pip_audit._state import AuditSpinner, AuditState
 from pip_audit._util import assert_never
 
@@ -538,7 +544,7 @@ def audit() -> None:  # pragma: no cover
         # wants to dry-run the "fix" step instead of the "audit" step
         auditor = Auditor(service, options=AuditOptions(dry_run=args.dry_run and not args.fix))
 
-        result = {}
+        result: dict[Dependency, list[VulnerabilityResult]] = {}
         pkg_count = 0
         vuln_count = 0
         skip_count = 0
diff --git a/pyproject.toml b/pyproject.toml
@@ -40,12 +40,10 @@ dependencies = [
 requires-python = ">=3.9"
 
 [project.optional-dependencies]
-test = [
+cov = [
     "coverage[toml] ~= 7.0, != 7.3.3", # https://github.com/nedbat/coveragepy/issues/1713
-    "pretend",
-    "pytest",
-    "pytest-cov",
 ]
+test = ["pretend", "pytest", "pip-audit[cov]"]
 lint = [
     "ruff >= 0.11",
     "interrogate ~= 1.6",
@@ -64,6 +62,19 @@ Homepage = "https://pypi.org/project/pip-audit/"
 Issues = "https://github.com/pypa/pip-audit/issues"
 Source = "https://github.com/pypa/pip-audit"
 
+[tool.coverage.paths]
+# This is used for path mapping when combining coverage data
+# from multiple machines. The first entry is the local path,
+# and subsequent entries are the remote paths that get remapped
+# to the local path.
+# See: https://coverage.readthedocs.io/en/latest/config.html#paths
+source = ["pip_audit", "*/pip_audit", "*\\pip_audit"]
+
+[tool.coverage.run]
+source = ["pip_audit"]
+parallel = true
+relative_files = true
+
 [tool.interrogate]
 # don't enforce documentation coverage for packaging, testing, the virtual
 # environment, or the CLI (which is documented separately).
diff --git a/test/dependency_source/test_pip.py b/test/dependency_source/test_pip.py
@@ -54,12 +54,13 @@ def test_pip_source_warns_about_old_pip(monkeypatch):
     monkeypatch.setattr(pip, "logger", logger)
 
     pip.PipSource()
-    assert logger.warning.calls == [
+    assert (
         pretend.call(
             "pip 1.0.0 is very old, and may not provide reliable dependency information! "
             "You are STRONGLY encouraged to upgrade to a newer version of pip."
         )
-    ]
+        in logger.warning.calls
+    )
 
 
 def test_pip_source_pip_api_failure(monkeypatch):
@@ -75,7 +76,9 @@ def explode():
 
 
 def test_pip_source_invalid_version(monkeypatch):
-    logger = pretend.stub(debug=pretend.call_recorder(lambda s: None))
+    logger = pretend.stub(
+        debug=pretend.call_recorder(lambda s: None), warning=pretend.call_recorder(lambda s: None)
+    )
     monkeypatch.setattr(pip, "logger", logger)
 
     source = pip.PipSource()
diff --git a/test/test_cache.py b/test/test_cache.py
@@ -1,40 +1,24 @@
-import importlib
-import sys
 from pathlib import Path
 
-import platformdirs
 import pretend  # type: ignore
-import pytest
 from packaging.version import Version
-from pytest import MonkeyPatch
+from platformdirs import user_cache_path
 
 import pip_audit._cache as cache
 from pip_audit._cache import _get_cache_dir, _get_pip_cache
 
 
-def _patch_platformdirs(monkeypatch: MonkeyPatch, sys_platform: str) -> None:
-    """Utility function to patch `platformdirs` in order to test cross-platforms."""
-    # Mocking OS host
-    monkeypatch.setattr(sys, "platform", sys_platform)
-    # We are forced to reload `platformdirs` to get the correct cache directory
-    # as cache definition is stored in the top level `__init__.py` file of the
-    # `platformdirs` package
-    importlib.reload(platformdirs)
-    if sys_platform == "win32":
-        monkeypatch.setenv("LOCALAPPDATA", "/tmp/AppData/Local")
-
-
 def test_get_cache_dir(monkeypatch):
     # When we supply a cache directory, always use that
-    cache_dir = _get_cache_dir(Path("/tmp/foo/cache_dir"))
-    assert cache_dir.as_posix() == "/tmp/foo/cache_dir"
+    cache_dir = Path("/tmp/foo/cache_dir")
+    assert _get_cache_dir(cache_dir) == cache_dir
 
-    get_pip_cache = pretend.call_recorder(lambda: Path("/fake/pip/cache/dir"))
+    cache_dir = Path("/fake/pip/cache/dir")
+    get_pip_cache = pretend.call_recorder(lambda: cache_dir)
     monkeypatch.setattr(cache, "_get_pip_cache", get_pip_cache)
 
     # When `pip cache dir` works, we use it. In this case, it's mocked.
-    cache_dir = _get_cache_dir(None, use_pip=True)
-    assert cache_dir.as_posix() == "/fake/pip/cache/dir"
+    assert _get_cache_dir(None, use_pip=True) == cache_dir
 
 
 def test_get_pip_cache():
@@ -43,96 +27,30 @@ def test_get_pip_cache():
     assert cache_dir.stem == "http"
 
 
-@pytest.mark.parametrize(
-    "sys_platform,expected",
-    [
-        pytest.param(
-            "linux",
-            Path.home() / ".cache" / "pip-audit",
-            id="on Linux",
-        ),
-        pytest.param(
-            "win32",
-            Path("/tmp") / "AppData" / "Local" / "pip-audit" / "Cache",
-            id="on Windows",
-        ),
-        pytest.param(
-            "darwin",
-            Path.home() / "Library" / "Caches" / "pip-audit",
-            id="on MacOS",
-        ),
-    ],
-)
-def test_get_cache_dir_do_not_use_pip(monkeypatch, sys_platform, expected):
-    # Check cross-platforms
-    _patch_platformdirs(monkeypatch, sys_platform)
+def test_get_cache_dir_do_not_use_pip():
+    expected = user_cache_path("pip-audit", appauthor=False)
+
     # Even with None, we never use the pip cache if we're told not to.
-    cache_dir = _get_cache_dir(None, use_pip=False)
-    assert cache_dir == expected
+    assert _get_cache_dir(None, use_pip=False) == expected
 
 
-@pytest.mark.parametrize(
-    "sys_platform,expected",
-    [
-        pytest.param(
-            "linux",
-            Path.home() / ".cache" / "pip-audit",
-            id="on Linux",
-        ),
-        pytest.param(
-            "win32",
-            Path("/tmp") / "AppData" / "Local" / "pip-audit" / "Cache",
-            id="on Windows",
-        ),
-        pytest.param(
-            "darwin",
-            Path.home() / "Library" / "Caches" / "pip-audit",
-            id="on MacOS",
-        ),
-    ],
-)
-def test_get_cache_dir_pip_disabled_in_environment(monkeypatch, sys_platform, expected):
+def test_get_cache_dir_pip_disabled_in_environment(monkeypatch):
     monkeypatch.setenv("PIP_NO_CACHE_DIR", "1")
-    # Check cross-platforms
-    _patch_platformdirs(monkeypatch, sys_platform)
+
+    expected = user_cache_path("pip-audit", appauthor=False)
 
     # Even with use_pip=True, we avoid pip's cache if the environment tells us to.
     assert _get_cache_dir(None, use_pip=True) == expected
 
 
-@pytest.mark.parametrize(
-    "sys_platform,expected",
-    [
-        pytest.param(
-            "linux",
-            Path.home() / ".cache" / "pip-audit",
-            id="on Linux",
-        ),
-        pytest.param(
-            "win32",
-            Path("/tmp") / "AppData" / "Local" / "pip-audit" / "Cache",
-            id="on Windows",
-        ),
-        pytest.param(
-            "darwin",
-            Path.home() / "Library" / "Caches" / "pip-audit",
-            id="on MacOS",
-        ),
-    ],
-)
-def test_get_cache_dir_old_pip(monkeypatch, sys_platform, expected):
+def test_get_cache_dir_old_pip(monkeypatch):
     # Check the case where we have an old `pip`
     monkeypatch.setattr(cache, "_PIP_VERSION", Version("1.0.0"))
-    # Check cross-platforms
-    _patch_platformdirs(monkeypatch, sys_platform)
-
-    # When we supply a cache directory, always use that
-    cache_dir = _get_cache_dir(Path("/tmp/foo/cache_dir"))
-    assert cache_dir.as_posix() == "/tmp/foo/cache_dir"
 
     # In this case, we can't query `pip` to figure out where its HTTP cache is
     # Instead, we use `~/.pip-audit-cache`
     cache_dir = _get_cache_dir(None)
+    expected = user_cache_path("pip-audit", appauthor=False)
     assert cache_dir == expected
 
 
