Skip to content

Commit

Permalink
Forbid use of os.path, point user to tmt._compat.Path
Browse files Browse the repository at this point in the history
  • Loading branch information
happz committed Sep 12, 2024
1 parent a85dde5 commit 5717bea
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 27 deletions.
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ builtins-ignorelist = ["help", "format", "input", "filter", "copyright", "max"]
"pathlib.Path".msg = "Use tmt._compat.pathlib.Path instead."
"pathlib.PosixPath".msg = "Use tmt._compat.pathlib.Path instead."
"warnings.deprecated".msg = "Use tmt._compat.warnings.deprecated instead."
"os.path.join".msg = "Use tmt._compat.pathlib.Path and pathlib instead."
"os.path".msg = "Use tmt._compat.pathlib.Path and pathlib instead."

[tool.ruff.lint.isort]
known-first-party = ["tmt"]
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ def validate_node(tree, node, schema, label, name):
pytest.fail(f"{label} {name} fails validation")


def _tree_path(tree):
return os.path.relpath(os.path.abspath(tree._path))
def _tree_path(tree: tmt.Tree):
return tree._path.absolute().relative_to(Path.cwd())


def extract_testcase_id(arg):
Expand Down
39 changes: 21 additions & 18 deletions tmt/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1598,9 +1598,12 @@ def expand_node_data(data: T, fmf_context: FmfContext) -> T:
# don't expand it at all, let's put the original value back
# into the stream.
# See https://github.com/teemtee/tmt/issues/2654.
#
# TID251: `pathlib` does not provide `os.patch.expandvars`, its
# use is allowed.
expanded_env: list[tuple[bool, str, str]] = [
(False, item[1:], item) if item.startswith('@')
else (True, os.path.expandvars(f'${item}'), item)
else (True, os.path.expandvars(f'${item}'), item) # noqa: TID251
for item in split_data]

# Expand unexpanded items, this time with fmf context providing
Expand All @@ -1613,8 +1616,9 @@ def expand_node_data(data: T, fmf_context: FmfContext) -> T:
if was_expanded:
expanded_ctx.append(item)
continue

expanded = os.path.expandvars(f'${item}')
# TID251: `pathlib` does not provide `os.patch.expandvars`, its
# use is allowed.
expanded = os.path.expandvars(f'${item}') # noqa: TID251
expanded_ctx.append(f'${original_item}' if expanded.startswith('$') else expanded)

# This cast is tricky: we get a string, and we return a
Expand Down Expand Up @@ -1991,15 +1995,15 @@ def create(
# Override template with data provided on command line
plan_content = Plan.edit_template(plan_content)

for name in names:
(directory, plan) = os.path.split(name)
directory_path = path / directory.lstrip('/')
has_fmf_ext = os.path.splitext(plan)[1] == '.fmf'
plan_path = directory_path / (plan + ('' if has_fmf_ext else '.fmf'))
for plan_name in names:
plan_path = Path(plan_name)

if plan_path.suffix != '.fmf':
plan_path = plan_path.parent / f'{plan_path.name}.fmf'

# Create directory & plan
tmt.utils.create_directory(
path=directory_path,
path=plan_path.parent,
name='plan directory',
dry=dry,
logger=logger)
Expand Down Expand Up @@ -2697,22 +2701,21 @@ def create(
except KeyError:
raise tmt.utils.GeneralError(f"Invalid template '{template}'.")

for name in names:
# Prepare paths
(directory, story) = os.path.split(name)
directory_path = path / directory.lstrip('/')
has_fmf_ext = os.path.splitext(story)[1] == '.fmf'
story_path = directory_path / (story + ('' if has_fmf_ext else '.fmf'))
for story_name in names:
story_path = Path(story_name)

if story_path.suffix != '.fmf':
story_path = story_path.parent / f'{story_path.name}.fmf'

# Create directory & story
tmt.utils.create_directory(
path=directory_path,
path=path / story_path.parent,
name='story directory',
dry=dry,
logger=logger)

tmt.utils.create_file(
path=story_path,
path=path / story_path,
name='story',
content=story_content,
dry=dry,
Expand Down Expand Up @@ -3948,7 +3951,7 @@ def runs(self, id_: tuple[str, ...]) -> bool:
if keep is not None:
# Sort by modify time of the workdirs and keep the newest workdirs
all_workdirs.sort(
key=lambda workdir: os.path.getmtime(workdir / 'run.yaml'), reverse=True)
key=lambda workdir: (workdir / 'run.yaml').stat().st_mtime, reverse=True)
all_workdirs = all_workdirs[keep:]

successful = True
Expand Down
5 changes: 4 additions & 1 deletion tmt/export/nitrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import os
import re
import types
import urllib.parse
from collections.abc import Iterator
from contextlib import suppress
from functools import cache
Expand Down Expand Up @@ -339,7 +340,9 @@ def add_to_nitrate_runs(
def prepare_extra_summary(test: 'tmt.Test', append_summary: bool) -> str:
""" extra-summary for export --create test """
assert test.fmf_id.url is not None # narrow type
remote_dirname = re.sub('.git$', '', os.path.basename(test.fmf_id.url))

parsed_url = urllib.parse.urlparse(test.fmf_id.url)
remote_dirname = re.sub('.git$', '', Path(parsed_url.path).name)
if not remote_dirname:
raise ConvertError("Unable to find git remote url.")
generated = f"{remote_dirname} {test.name}"
Expand Down
5 changes: 4 additions & 1 deletion tmt/plugins/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,11 @@ def _explore_custom_directories(logger: Logger) -> None:
return

for _path in os.environ[ENVIRONMENT_NAME].split(os.pathsep):
# TID251: `pathlib` does not provide `os.patch.expandvars`, its
# use is allowed. For the simplicity, keeping `expanduser` as well,
# to avoid str -> Path -> str -> Path conversion.
_explore_directory(
Path(os.path.expandvars(os.path.expanduser(_path))).resolve(),
Path(os.path.expandvars(os.path.expanduser(_path))).resolve(), # noqa: TID251
logger)


Expand Down
5 changes: 3 additions & 2 deletions tmt/steps/discover/fmf.py
Original file line number Diff line number Diff line change
Expand Up @@ -544,8 +544,9 @@ def do_the_discovery(self, path: Optional[Path] = None) -> None:
'git', 'log', '--format=', '--stat', '--name-only', f"{modified_ref}..HEAD"
), cwd=self.testdir)
if output.stdout:
directories = [os.path.dirname(name) for name in output.stdout.split('\n')]
modified = {f"^/{re.escape(name)}" for name in directories if name}
directories = [Path(name).parent for name in output.stdout.split('\n')]
modified = {f"^/{re.escape(str(directory))}"
for directory in directories if directory}
if not modified:
# Nothing was modified, do not select anything
return
Expand Down
2 changes: 1 addition & 1 deletion tmt/steps/provision/testcloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ def is_ready(self) -> bool:
@functools.cached_property
def is_kvm(self) -> bool:
# Is the combination of host-requested architecture kvm capable?
return bool(self.arch == platform.machine() and os.path.exists("/dev/kvm"))
return bool(self.arch == platform.machine() and Path('/dev/kvm').exists())

@functools.cached_property
def is_legacy_os(self) -> bool:
Expand Down
2 changes: 1 addition & 1 deletion tmt/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ def from_file(
content = environment_filepath.read_text()

# Parse yaml file
if os.path.splitext(filename)[1].lower() in ('.yaml', '.yml'):
if filename.rsplit('.')[-1].lower() in ('yaml', 'yml'):
environment = cls.from_yaml(content)

else:
Expand Down

0 comments on commit 5717bea

Please sign in to comment.