Skip to content

Commit

Permalink
Add a single tmpdir fixture for all Python versions
Browse files Browse the repository at this point in the history
See comments in `conftest.py`, but in general:

* because of our code, we would like to use `tmp_path` and `Path`
  it returns,
* but we have also our custom `tmt.utils.Path` we should be using
  instead of the original `pathlib.Path`,
* because of RHEL8, we also get pytest without `tmp_path` fixture,
* and `tmpdir` fixture returns `py.path.local`.

The patch adds our custom fixture which returns `tmt.utils.Path`
no matter what pytest we get. It's achieved by a test import,
trivial wrapper class, and two fixtures hiding the actual pytest
fixture involved.
  • Loading branch information
happz authored and psss committed Jun 29, 2023
1 parent ccfb0d0 commit 4e05f95
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 38 deletions.
78 changes: 73 additions & 5 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,88 @@
from pathlib import Path
import pathlib
from typing import Any

import _pytest.logging
import _pytest.tmpdir
import py.path
import pytest

from tmt.log import Logger
from tmt.utils import Path


@pytest.fixture(name='root_logger')
def fixture_root_logger(caplog: _pytest.logging.LogCaptureFixture) -> Logger:
return Logger.create(verbose=0, debug=0, quiet=False)


# Temporary directories and paths
#
# * the recommended way is to use `tmp_path` and `tmp_path_factory` fixtures
# * `tmp_path*` fixtures are not available in RHEL-8, `tmpdir` (and `tmpdir_factory`)
# fixtures are - but these return `py.path.local` instead of a lovely `pathlib.Path`
# * `pathlib.Path` is also not good enough, as it may lack some methods in older
# Python versions, that's why we have our own `tmt.utils.Path`.
#
# So, what we need:
#
# * a single name, we can't switch between `tmp_path` and `tmpdir` in every test
# * `tmt.utils.Path`, no strings, no `py.path.local`
# * works across all supported Python versions, from 3.6 in RHEL8 till 3.11 or so
#
# To solve this, we add here:
#
# * a wrapper class, representing the "tmp path factory". It's initialized with an
# actual factory, has the same public API, but returns our `tmt.utils.Path`
# * two new fixtures, `tmppath` and `tmppath_factory` that consume available fixtures
# and return corresponding `tmt.utils.Path`.
# * tests using `tmppath*` instead of pytest's own `tmp_path*` and `tmpdir*` fixtures
#
class TempPathFactory:
def __init__(self, actual_factory: Any) -> None:
self._actual_factory = actual_factory

def getbasetemp(self) -> Path:
return Path(str(self._actual_factory.getbasetemp()))

def mktemp(self, basename: str, numbered: bool = True) -> Path:
return Path(str(self._actual_factory.mktemp(basename, numbered=numbered)))


try:
# If the import succeeds, we're about to wrap `Path` by `tmp_path`...
from _pytest.tmpdir import tmp_path_factory # noqa: F401

@pytest.fixture(scope='session')
def tmppath_factory(
tmp_path_factory: '_pytest.tmpdir.TempPathFactory') -> TempPathFactory: # noqa: F811
return TempPathFactory(tmp_path_factory)

@pytest.fixture()
def tmppath(tmp_path: pathlib.Path) -> Path:
return Path(str(tmp_path))

except ImportError:
# ... and if the import fails, we're wrapping `py.path.local` from `tmpdir` family.

# ignore[name-defined]: when inspected with our daily Python 3.9 or something,
# the pytest is probably way newer than the one in RHEL8, and therefore the
# name indeed would not exist. But this whole branch applies only with the old
# pytest, therefore things are safe.
@pytest.fixture(scope='session')
def tmppath_factory(
tmpdir_factory: '_pytest.tmpdir.TempdirFactory' # type: ignore[name-defined]
) -> TempPathFactory:
return TempPathFactory(tmpdir_factory)

@pytest.fixture()
def tmppath(tmpdir: py.path.local) -> Path:
return Path(str(tmpdir))


@pytest.fixture(scope='module')
def source_dir(tmpdir_factory):
def source_dir(tmppath_factory: TempPathFactory) -> Path:
""" Create dummy directory structure and remove it after tests """
source_location = Path(tmpdir_factory.mktemp('source'))
source_location = tmppath_factory.mktemp('source')
(source_location / 'library').mkdir(parents=True)
(source_location / 'lib_folder').mkdir()
(source_location / 'tests').mkdir()
Expand All @@ -26,6 +94,6 @@ def source_dir(tmpdir_factory):


@pytest.fixture()
def target_dir(tmpdir_factory):
def target_dir(tmppath_factory: TempPathFactory) -> Path:
""" Return target directory path and clean up after tests """
return Path(tmpdir_factory.mktemp('target'))
return tmppath_factory.mktemp('target')
8 changes: 4 additions & 4 deletions tests/unit/test_report_junit.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@


@pytest.fixture()
def report_fix(tmpdir, root_logger):
def report_fix(tmppath: Path, root_logger):
# need to provide genuine workdir paths - mock would break os.path.* calls
step_mock = MagicMock(workdir=str(tmpdir))
step_mock = MagicMock(workdir=tmppath)
plan_mock = MagicMock()
name_property = PropertyMock(return_value='name')

type(plan_mock).name = name_property
type(step_mock).plan = plan_mock

out_file_path = str(tmpdir.join("out.xml"))
out_file_path = str(tmppath / "out.xml")

def get(key, default=None):
if key == "file":
Expand All @@ -31,7 +31,7 @@ def get(key, default=None):
logger=root_logger,
step=step_mock,
data=ReportJUnitData(name='x', how='junit'),
workdir=Path(str(tmpdir.join('junit'))))
workdir=tmppath / 'junit')
report.get = get
report.info = MagicMock()

Expand Down
60 changes: 31 additions & 29 deletions tests/unit/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from datetime import timedelta
from typing import Any, List, Tuple

import py
import pytest

import tmt
Expand Down Expand Up @@ -40,8 +39,8 @@


@pytest.fixture()
def local_git_repo(tmpdir: py.path.local) -> Path:
origin = Path(str(tmpdir)) / 'origin'
def local_git_repo(tmppath: Path) -> Path:
origin = tmppath / 'origin'
origin.mkdir()

run(Command('git', 'init'), cwd=origin)
Expand Down Expand Up @@ -136,9 +135,11 @@ def test_config():
assert config2.last_run.resolve() == run.resolve()


def test_last_run_race(tmpdir, monkeypatch):
def test_last_run_race(tmppath: Path, monkeypatch):
""" Race in last run symlink should't be fatal """
monkeypatch.setattr(tmt.utils, 'CONFIG_PATH', Path(str(tmpdir.mkdir('config'))))
config_path = tmppath / 'config'
config_path.mkdir()
monkeypatch.setattr(tmt.utils, 'CONFIG_PATH', config_path)
mock_logger = unittest.mock.MagicMock()
monkeypatch.setattr(tmt.utils.log, 'warning', mock_logger)
config = tmt.utils.Config()
Expand All @@ -147,7 +148,9 @@ def test_last_run_race(tmpdir, monkeypatch):

def create_last_run(config, counter):
try:
val = config.last_run = Path(str(tmpdir.mkdir(f"run-{counter}")))
last_run_path = tmppath / f"run-{counter}"
last_run_path.mkdir()
val = config.last_run = last_run_path
results.put(val)
except Exception as err:
results.put(err)
Expand All @@ -173,21 +176,21 @@ def create_last_run(config, counter):
assert config.last_run, "Some run was stored as last run"


def test_workdir_env_var(tmpdir, monkeypatch, root_logger):
def test_workdir_env_var(tmppath: Path, monkeypatch, root_logger):
""" Test TMT_WORKDIR_ROOT environment variable """
# Cannot use monkeypatch.context() as it is not present for CentOS Stream 8
monkeypatch.setenv('TMT_WORKDIR_ROOT', str(tmpdir))
monkeypatch.setenv('TMT_WORKDIR_ROOT', str(tmppath))
common = Common(logger=root_logger)
common._workdir_init()
monkeypatch.delenv('TMT_WORKDIR_ROOT')
assert common.workdir == Path(f'{tmpdir}/run-001')
assert common.workdir == tmppath / 'run-001'


def test_workdir_root_full(tmpdir, monkeypatch, root_logger):
def test_workdir_root_full(tmppath, monkeypatch, root_logger):
""" Raise if all ids lower than WORKDIR_MAX are exceeded """
monkeypatch.setenv('TMT_WORKDIR_ROOT', str(tmpdir))
monkeypatch.setenv('TMT_WORKDIR_ROOT', str(tmppath))
monkeypatch.setattr(tmt.utils, 'WORKDIR_MAX', 1)
possible_workdir = Path(str(tmpdir)) / 'run-001'
possible_workdir = tmppath / 'run-001'
# First call success
common1 = Common(logger=root_logger)
common1._workdir_init()
Expand All @@ -203,9 +206,9 @@ def test_workdir_root_full(tmpdir, monkeypatch, root_logger):
assert common2.workdir.resolve() == possible_workdir.resolve()


def test_workdir_root_race(tmpdir, monkeypatch, root_logger):
def test_workdir_root_race(tmppath, monkeypatch, root_logger):
""" Avoid race in workdir creation """
monkeypatch.setattr(tmt.utils, 'WORKDIR_ROOT', Path(str(tmpdir)))
monkeypatch.setattr(tmt.utils, 'WORKDIR_ROOT', tmppath)
results = queue.Queue()
threads = []

Expand Down Expand Up @@ -546,23 +549,23 @@ def test_multiple_values(self):
StructuredFieldError, field.get, "section", "key")


def test_run_interactive_not_joined(tmpdir, root_logger):
def test_run_interactive_not_joined(tmppath, root_logger):
output = ShellScript("echo abc; echo def >2").to_shell_command().run(
shell=True,
interactive=True,
cwd=Path(str(tmpdir)),
cwd=tmppath,
env={},
log=None,
logger=root_logger)
assert output.stdout is None
assert output.stderr is None


def test_run_interactive_joined(tmpdir, root_logger):
def test_run_interactive_joined(tmppath, root_logger):
output = ShellScript("echo abc; echo def >2").to_shell_command().run(
shell=True,
interactive=True,
cwd=Path(str(tmpdir)),
cwd=tmppath,
env={},
join=True,
log=None,
Expand Down Expand Up @@ -650,14 +653,13 @@ def test_get_distgit_handler_explicit():
assert instance.__class__.__name__ == 'RedHatGitlab'


def test_fedora_dist_git(tmpdir):
def test_fedora_dist_git(tmppath):
# Fake values, production hash is too long
path = Path(str(tmpdir))
path.joinpath('sources').write_text('SHA512 (fn-1.tar.gz) = 09af\n')
path.joinpath('tmt.spec').write_text('')
(tmppath / 'sources').write_text('SHA512 (fn-1.tar.gz) = 09af\n')
(tmppath / 'tmt.spec').write_text('')
fedora_sources_obj = tmt.utils.FedoraDistGit()
assert [("https://src.fedoraproject.org/repo/pkgs/rpms/tmt/fn-1.tar.gz/sha512/09af/fn-1.tar.gz", # noqa: E501
"fn-1.tar.gz")] == fedora_sources_obj.url_and_name(cwd=path)
"fn-1.tar.gz")] == fedora_sources_obj.url_and_name(cwd=tmppath)


class TestValidateGitStatus:
Expand Down Expand Up @@ -689,15 +691,15 @@ def test_all_good(

@classmethod
def test_no_remote(cls, local_git_repo: Path, root_logger):
tmpdir = local_git_repo
tmt.Tree.init(logger=root_logger, path=tmpdir, template=None, force=None)
tmpdir.joinpath('main.fmf').write_text('test: echo')
tmt.Tree.init(logger=root_logger, path=local_git_repo, template=None, force=None)
with open(local_git_repo / 'main.fmf', 'w') as f:
f.write('test: echo')
run(ShellScript('git add main.fmf .fmf/version').to_shell_command(),
cwd=tmpdir)
cwd=local_git_repo)
run(ShellScript('git commit -m initial_commit').to_shell_command(),
cwd=tmpdir)
cwd=local_git_repo)

test = tmt.Tree(logger=root_logger, path=tmpdir).tests()[0]
test = tmt.Tree(logger=root_logger, path=local_git_repo).tests()[0]
val, msg = validate_git_status(test)
assert not val
assert "Failed to get remote branch" in msg
Expand Down

0 comments on commit 4e05f95

Please sign in to comment.