Extend coverage (#374)

* Add coverage to schema

* Omit types.py from coverage

We explicitly ignore TYPE_CHECKING, so it will never get looked at.

* Add tests for load_collections

* Update ruff setting locations

* Cover collection_path cases

* I don't think this break is ever reachable

Which means the else is not necessary because anything that escapes the
loop will have done so naturally

* Add test for ignore_errors

* Test that both offline messages are emitted

* Actually check required_collections use

* Add test for passing through a valid symlink

* Reduce size of test_runtime

* Disable too-many-lines for test_runtime.py

.github/workflows/tox.yml

@@ -50,7 +50,7 @@ jobs:
       matrix: ${{ fromJson(needs.prepare.outputs.matrix) }}
     env:
       FORCE_COLOR: 1
-      PYTEST_REQPASS: 94
+      PYTEST_REQPASS: 106
     steps:
       - uses: actions/checkout@v4
         with:

.pre-commit-config.yaml

@@ -26,10 +26,11 @@ repos:
     hooks:
       - id: ruff
         args: [--fix, --exit-non-zero-on-fix]
-  - repo: https://github.com/pappasam/toml-sort
-    rev: v0.23.1
-    hooks:
-      - id: toml-sort-fix
+  # https://github.com/pappasam/toml-sort/issues/69
+  # - repo: https://github.com/pappasam/toml-sort
+  #   rev: v0.23.1
+  #   hooks:
+  #     - id: toml-sort-fix
   - repo: https://github.com/pre-commit/mirrors-prettier
     # keep it before yamllint
     rev: "v4.0.0-alpha.8"

pyproject.toml

@@ -51,6 +51,7 @@ show_missing = true
 
 [tool.coverage.run]
 source = ["src"]
+omit = ["src/ansible_compat/types.py"]
 # Do not use branch until bug is fixes:
 # https://github.com/nedbat/coveragepy/issues/605
 branch = false
@@ -116,8 +117,9 @@ filterwarnings = [
 testpaths = ["test"]
 
 [tool.ruff]
-select = ["ALL"]
-ignore = [
+target-version = "py39"
+lint.select = ["ALL"]
+lint.ignore = [
   # Disabled on purpose:
   "ANN101", # Missing type annotation for `self` in method
   "D203", # incompatible with D211
@@ -132,19 +134,18 @@ ignore = [
   "RUF012",
   "PERF203"
 ]
-target-version = "py39"
 
-[tool.ruff.flake8-pytest-style]
+[tool.ruff.lint.flake8-pytest-style]
 parametrize-values-type = "tuple"
 
-[tool.ruff.isort]
+[tool.ruff.lint.isort]
 known-first-party = ["ansible_compat"]
 known-third-party = ["packaging"]
 
-[tool.ruff.per-file-ignores]
+[tool.ruff.lint.per-file-ignores]
 "test/**/*.py" = ["SLF001", "S101", "FBT001"]
 
-[tool.ruff.pydocstyle]
+[tool.ruff.lint.pydocstyle]
 convention = "pep257"
 
 [tool.setuptools.dynamic]

src/ansible_compat/runtime.py

@@ -285,15 +285,19 @@ def load_collections(self) -> None:
             _logger.error(proc)
             msg = f"Unable to list collections: {proc}"
             raise RuntimeError(msg)
-        data = json.loads(proc.stdout)
+        try:
+            data = json.loads(proc.stdout)
+        except json.decoder.JSONDecodeError as exc:
+            msg = f"Unable to parse galaxy output as JSON: {proc.stdout}"
+            raise RuntimeError(msg) from exc
         if not isinstance(data, dict):
             msg = f"Unexpected collection data, {data}"
             raise TypeError(msg)
         for path in data:
+            if not isinstance(data[path], dict):
+                msg = f"Unexpected collection data, {data[path]}"
+                raise TypeError(msg)
             for collection, collection_info in data[path].items():
-                if not isinstance(collection, str):
-                    msg = f"Unexpected collection data, {collection}"
-                    raise TypeError(msg)
                 if not isinstance(collection_info, dict):
                     msg = f"Unexpected collection data, {collection_info}"
                     raise TypeError(msg)
@@ -772,18 +776,16 @@ def require_collection(
                             _logger.fatal(msg)
                             raise InvalidPrerequisiteError(msg)
                     return found_version, collpath.resolve()
-                break
-        else:
-            if install:
-                self.install_collection(f"{name}:>={version}" if version else name)
-                return self.require_collection(
-                    name=name,
-                    version=version,
-                    install=False,
-                )
-            msg = f"Collection '{name}' not found in '{paths}'"
-            _logger.fatal(msg)
-            raise InvalidPrerequisiteError(msg)
+        if install:
+            self.install_collection(f"{name}:>={version}" if version else name)
+            return self.require_collection(
+                name=name,
+                version=version,
+                install=False,
+            )
+        msg = f"Collection '{name}' not found in '{paths}'"
+        _logger.fatal(msg)
+        raise InvalidPrerequisiteError(msg)
 
     def _prepare_ansible_paths(self) -> None:
         """Configure Ansible environment variables."""

test/roles/acme.missing_deps/requirements.yml

@@ -1,2 +1,4 @@
 collections:
   - foo.bar # collection that does not exist, so we can test offline mode
+roles:
+  - this_role_does_not_exist # and also role that does not exist

test/test_config.py

@@ -7,7 +7,12 @@
 from _pytest.monkeypatch import MonkeyPatch
 from packaging.version import Version
 
-from ansible_compat.config import AnsibleConfig, ansible_version, parse_ansible_version
+from ansible_compat.config import (
+    AnsibleConfig,
+    ansible_collections_path,
+    ansible_version,
+    parse_ansible_version,
+)
 from ansible_compat.errors import InvalidPrerequisiteError, MissingAnsibleError
 
 
@@ -85,3 +90,17 @@ def test_ansible_version() -> None:
 def test_ansible_version_arg() -> None:
     """Validate ansible_version behavior."""
     assert ansible_version("2.0") >= Version("1.0")
+
+
+@pytest.mark.parametrize(
+    "var",
+    ("", "ANSIBLE_COLLECTIONS_PATH", "ANSIBLE_COLLECTIONS_PATHS"),
+    ids=["blank", "singular", "plural"],
+)
+def test_ansible_collections_path_env(var: str, monkeypatch: MonkeyPatch) -> None:
+    """Test that ansible_collections_path returns the appropriate env var."""
+    # Set the variable
+    if var:
+        monkeypatch.setenv(var, "")
+
+    assert ansible_collections_path() == (var or "ANSIBLE_COLLECTIONS_PATH")

test/test_runtime.py

@@ -1,6 +1,6 @@
 """Tests for Runtime class."""
 
-# pylint: disable=protected-access
+# pylint: disable=protected-access,too-many-lines
 from __future__ import annotations
 
 import logging
@@ -182,10 +182,13 @@ def test_runtime_install_role(
     runtime.cache_dir = tmp_dir
 
 
-def test_prepare_environment_with_collections(tmp_path: pathlib.Path) -> None:
+def test_prepare_environment_with_collections(runtime_tmp: Runtime) -> None:
     """Check that collections are correctly installed."""
-    runtime = Runtime(isolated=True, project_dir=tmp_path)
-    runtime.prepare_environment(required_collections={"community.molecule": "0.1.0"})
+    runtime_tmp.prepare_environment(
+        required_collections={"community.molecule": "0.1.0"},
+        install_local=True,
+    )
+    assert "community.molecule" in runtime_tmp.collections
 
 
 def test_runtime_install_requirements_missing_file() -> None:
@@ -438,14 +441,13 @@ def test_require_collection_invalid_collections_path(runtime: Runtime) -> None:
         runtime.require_collection("community.molecule")
 
 
-def test_require_collection_preexisting_broken(tmp_path: pathlib.Path) -> None:
+def test_require_collection_preexisting_broken(runtime_tmp: Runtime) -> None:
     """Check that require_collection raise with broken pre-existing collection."""
-    runtime = Runtime(isolated=True, project_dir=tmp_path)
-    dest_path: str = runtime.config.collections_paths[0]
+    dest_path: str = runtime_tmp.config.collections_paths[0]
     dest = pathlib.Path(dest_path) / "ansible_collections" / "foo" / "bar"
     dest.mkdir(parents=True, exist_ok=True)
     with pytest.raises(InvalidPrerequisiteError, match="missing MANIFEST.json"):
-        runtime.require_collection("foo.bar")
+        runtime_tmp.require_collection("foo.bar")
 
 
 def test_require_collection(runtime_tmp: Runtime) -> None:
@@ -567,6 +569,18 @@ def test_install_galaxy_role_bad_namespace(runtime_tmp: Runtime) -> None:
         runtime_tmp._install_galaxy_role(runtime_tmp.project_dir, role_name_check=1)
 
 
+def test_install_galaxy_role_no_meta(runtime_tmp: Runtime) -> None:
+    """Check install role with missing meta/main.yml."""
+    # This should fail because meta/main.yml is missing
+    with pytest.raises(
+        FileNotFoundError,
+        match=f"No such file or directory: '{runtime_tmp.project_dir.absolute()}/meta/main.yaml'",
+    ):
+        runtime_tmp._install_galaxy_role(runtime_tmp.project_dir)
+    # But ignore_errors will return without doing anything
+    runtime_tmp._install_galaxy_role(runtime_tmp.project_dir, ignore_errors=True)
+
+
 @pytest.mark.parametrize(
     "galaxy_info",
     (
@@ -741,11 +755,83 @@ def test_install_collection_from_disk_fail() -> None:
         )
 
 
-def test_prepare_environment_offline_role() -> None:
+def test_load_collections_failure(mocker: MockerFixture) -> None:
+    """Tests for ansible-galaxy erroring."""
+    mocker.patch(
+        "ansible_compat.runtime.Runtime.run",
+        return_value=CompletedProcess(
+            ["x"],
+            returncode=1,
+            stdout="There was an error",
+            stderr="This is the error",
+        ),
+        autospec=True,
+    )
+    runtime = Runtime()
+    with pytest.raises(RuntimeError, match="Unable to list collections: "):
+        runtime.load_collections()
+
+
+@pytest.mark.parametrize(
+    "value",
+    ("[]", '{"path": "bad data"}', '{"path": {"ansible.posix": 123}}'),
+    ids=["list", "malformed_collection", "bad_collection_data"],
+)
+def test_load_collections_garbage(value: str, mocker: MockerFixture) -> None:
+    """Tests for ansible-galaxy returning bad data."""
+    mocker.patch(
+        "ansible_compat.runtime.Runtime.run",
+        return_value=CompletedProcess(
+            ["x"],
+            returncode=0,
+            stdout=value,
+            stderr="",
+        ),
+        autospec=True,
+    )
+    runtime = Runtime()
+    with pytest.raises(TypeError, match="Unexpected collection data, "):
+        runtime.load_collections()
+
+
+@pytest.mark.parametrize(
+    "value",
+    ("", '{"path": {123: 456}}'),
+    ids=["nothing", "bad_collection_name"],
+)
+def test_load_collections_invalid_json(value: str, mocker: MockerFixture) -> None:
+    """Tests for ansible-galaxy returning bad data."""
+    mocker.patch(
+        "ansible_compat.runtime.Runtime.run",
+        return_value=CompletedProcess(
+            ["x"],
+            returncode=0,
+            stdout=value,
+            stderr="",
+        ),
+        autospec=True,
+    )
+    runtime = Runtime()
+    with pytest.raises(
+        RuntimeError,
+        match=f"Unable to parse galaxy output as JSON: {value}",
+    ):
+        runtime.load_collections()
+
+
+def test_prepare_environment_offline_role(caplog: pytest.LogCaptureFixture) -> None:
     """Ensure that we can make use of offline roles."""
     with cwd(Path("test/roles/acme.missing_deps")):
         runtime = Runtime(isolated=True)
         runtime.prepare_environment(install_local=True, offline=True)
+        assert (
+            "Skipped installing old role dependencies due to running in offline mode."
+            in caplog.text
+        )
+        assert (
+            "Skipped installing collection dependencies due to running in offline mode."
+            in caplog.text
+        )
 
 
 def test_runtime_run(runtime: Runtime) -> None:
@@ -870,11 +956,20 @@ def test_is_url(name: str, result: bool) -> None:
     assert is_url(name) == result
 
 
-def test_prepare_environment_repair_broken_symlink(
+@pytest.mark.parametrize(
+    ("dest", "message"),
+    (
+        ("/invalid/destination", "Collection is symlinked, but not pointing to"),
+        (Path.cwd(), "Found symlinked collection, skipping its installation."),
+    ),
+    ids=["broken", "valid"],
+)
+def test_prepare_environment_symlink(
+    dest: str | Path,
+    message: str,
     caplog: pytest.LogCaptureFixture,
 ) -> None:
-    """Ensure we can deal with broken symlinks in collections."""
-    caplog.set_level(logging.INFO)
+    """Ensure avalid symlinks to collections are properly detected."""
     project_dir = Path(__file__).parent / "collections" / "acme.minimal"
     runtime = Runtime(isolated=True, project_dir=project_dir)
     assert runtime.cache_dir
@@ -883,12 +978,9 @@ def test_prepare_environment_repair_broken_symlink(
     goodies = acme / "minimal"
     rmtree(goodies, ignore_errors=True)
     goodies.unlink(missing_ok=True)
-    goodies.symlink_to("/invalid/destination")
+    goodies.symlink_to(dest)
     runtime.prepare_environment(install_local=True)
-    assert any(
-        msg.startswith("Collection is symlinked, but not pointing to")
-        for msg in caplog.messages
-    )
+    assert message in caplog.text
 
 
 def test_get_galaxy_role_name_invalid() -> None:

test/test_schema.py

@@ -72,3 +72,16 @@ def test_schema(index: int) -> None:
 def test_json_path() -> None:
     """Test json_path function."""
     assert json_path(["a", 1, "b"]) == "$.a[1].b"
+
+
+def test_validate_invalid_schema() -> None:
+    """Test validate function error handling."""
+    schema = "[]"
+    data = json_from_asset("assets/validate0_data.json")
+    errors = validate(schema, data)
+
+    assert len(errors) == 1
+    assert (
+        errors[0].to_friendly()
+        == "In 'schema sanity check': Invalid schema, must be a mapping."
+    )