Skip to content

Commit

Permalink
Update util.abs_path to return Path for Paths (#4310)
Browse files Browse the repository at this point in the history
Also return empty string instead of None where it would have before.

This allows us to always consume the output, without doing type
shenanigans after, and gradually move to Path where it's convenient.
  • Loading branch information
Qalthos authored Oct 29, 2024
1 parent f58ac30 commit c20dc08
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 25 deletions.
2 changes: 0 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ repos:
rev: v3.1.0
hooks:
- id: add-trailing-comma
args:
- --py36-plus

- repo: https://github.com/Lucas-C/pre-commit-hooks.git
rev: v1.5.5
Expand Down
6 changes: 3 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -356,14 +356,14 @@ verbosity_assertions = 2
[tool.ruff]
builtins = ["__"]
cache-dir = "./.cache/.ruff"
external = [
"DOC" # pydoclint
]
fix = true
line-length = 100
target-version = "py310"

[tool.ruff.lint]
external = [
"DOC" # pydoclint
]
select = ["ALL"]

[tool.ruff.lint.flake8-pytest-style]
Expand Down
2 changes: 1 addition & 1 deletion src/molecule/command/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ def get_configs(
scenario_paths = filter_ignored_scenarios(scenario_paths)
configs = [
config.Config(
molecule_file=util.abs_path(c), # type: ignore[arg-type]
molecule_file=util.abs_path(c),
args=args,
command_args=command_args,
ansible_args=ansible_args,
Expand Down
17 changes: 7 additions & 10 deletions src/molecule/provisioner/ansible.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import os
import shutil

from pathlib import Path
from typing import Any

from ansible_compat.ports import cached_property
Expand Down Expand Up @@ -520,8 +521,8 @@ def default_env(self): # type: ignore[no-untyped-def] # noqa: ANN201, D102
os.environ,
{
"ANSIBLE_CONFIG": self._config.provisioner.config_file,
"ANSIBLE_ROLES_PATH": ":".join(roles_path_list), # type: ignore[arg-type]
self._config.ansible_collections_path: ":".join(collections_path_list), # type: ignore[arg-type]
"ANSIBLE_ROLES_PATH": ":".join(roles_path_list),
self._config.ansible_collections_path: ":".join(collections_path_list),
"ANSIBLE_LIBRARY": ":".join(self._get_modules_directories()),
"ANSIBLE_FILTER_PLUGINS": ":".join(
self._get_filter_plugins_directories(),
Expand Down Expand Up @@ -804,17 +805,13 @@ def _add_or_update_vars(self): # type: ignore[no-untyped-def] # noqa: ANN202
vars_target = self.group_vars

if vars_target:
target_vars_directory = os.path.join( # noqa: PTH118
self.inventory_directory,
target,
)

if not os.path.isdir(util.abs_path(target_vars_directory)): # type: ignore[arg-type] # noqa: PTH112
os.mkdir(util.abs_path(target_vars_directory)) # type: ignore[arg-type] # noqa: PTH102
target_vars_directory = util.abs_path(Path(self.inventory_directory) / target)
if not target_vars_directory.is_dir():
target_vars_directory.mkdir()

for target in vars_target: # noqa: PLW2901
target_var_content = vars_target[target]
path = os.path.join(util.abs_path(target_vars_directory), target) # type: ignore[arg-type] # noqa: PTH118
path = target_vars_directory / target
util.write_file(path, util.safe_dump(target_var_content))

def _write_inventory(self): # type: ignore[no-untyped-def] # noqa: ANN202
Expand Down
25 changes: 18 additions & 7 deletions src/molecule/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

from pathlib import Path
from subprocess import CalledProcessError, CompletedProcess
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, overload

import jinja2
import yaml
Expand Down Expand Up @@ -394,7 +394,15 @@ def filter_verbose_permutation(options: dict[str, Any]) -> dict[str, Any]:
return {k: options[k] for k in options if not re.match("^[v]+$", k)}


def abs_path(path: str | Path | None) -> str | None:
@overload
def abs_path(path: Path) -> Path: ...


@overload
def abs_path(path: str | None) -> str: ...


def abs_path(path: str | Path | None) -> str | Path:
"""Return absolute path.
Args:
Expand All @@ -403,12 +411,15 @@ def abs_path(path: str | Path | None) -> str | None:
Returns:
Absolute path of path.
"""
if path:
if isinstance(path, str):
path = Path(path)
if not path:
return ""

return str(path.resolve())
return None
output_type = type(path)
if isinstance(path, str):
path = Path(path)
path = path.resolve()

return output_type(path)


def merge_dicts(a: _T, b: _T) -> _T:
Expand Down
10 changes: 8 additions & 2 deletions tests/unit/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import os
import warnings

from pathlib import Path
from typing import TYPE_CHECKING

import pytest
Expand All @@ -36,7 +37,6 @@

if TYPE_CHECKING:
from collections.abc import MutableMapping
from pathlib import Path
from typing import Any
from unittest.mock import Mock

Expand Down Expand Up @@ -327,9 +327,15 @@ def test_abs_path() -> None:
assert util.abs_path(test_dir) == "/foo"


def test_abs_path_with_path() -> None:
"""Test the `abs_path` function."""
test_dir = Path("/foo/../foo")
assert util.abs_path(test_dir) == Path("/foo")


def test_abs_path_with_empty_path() -> None:
"""Test the `abs_path` function with an empty path."""
assert util.abs_path("") is None
assert util.abs_path("") == ""


@pytest.mark.parametrize(
Expand Down

0 comments on commit c20dc08

Please sign in to comment.