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."
+ )