Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend coverage #374

Merged
merged 12 commits into from
May 20, 2024
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 5 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
15 changes: 8 additions & 7 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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]
Expand Down
34 changes: 18 additions & 16 deletions src/ansible_compat/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,15 +285,19 @@
_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):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is removed as I don't think it's possible to hit. Non-string keys won't get this far, as they won't be recognized as valid JSON and therefore hit the DecodeError above.

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)
Expand Down Expand Up @@ -772,18 +776,16 @@
_logger.fatal(msg)
raise InvalidPrerequisiteError(msg)
return found_version, collpath.resolve()
break
else:
Comment on lines -775 to -776
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like this break can ever be reached, which means the else is also superfluous as anything that exits the loop does so by exhausting paths

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(

Check warning on line 781 in src/ansible_compat/runtime.py

View check run for this annotation

Codecov / codecov/patch

src/ansible_compat/runtime.py#L781

Added line #L781 was not covered by tests
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."""
Expand Down
2 changes: 2 additions & 0 deletions test/roles/acme.missing_deps/requirements.yml
Original file line number Diff line number Diff line change
@@ -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
21 changes: 20 additions & 1 deletion test/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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")
126 changes: 109 additions & 17 deletions test/test_runtime.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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",
(
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
13 changes: 13 additions & 0 deletions test/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."
)
Loading