Skip to content

Commit

Permalink
Code review improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
nicoddemus committed Oct 10, 2020
1 parent 432916a commit a8b07ce
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 73 deletions.
2 changes: 1 addition & 1 deletion changelog/7425.feature.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ New :fixture:`pytester` fixture, which is identical to :fixture:`testdir` but it

This is part of the movement to use :class:`pathlib.Path` objects internally, in order to remove the dependency to ``py`` in the future.

Internally, the old :class:`Testdir` is now a thin wrapper around :class:`PyTester`, preserving the old interface.
Internally, the old :class:`Testdir` is now a thin wrapper around :class:`Pytester`, preserving the old interface.
4 changes: 2 additions & 2 deletions doc/en/reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ pytester

.. currentmodule:: _pytest.pytester

Provides a :class:`PyTester` instance that can be used to run and test pytest itself.
Provides a :class:`Pytester` instance that can be used to run and test pytest itself.

It provides an empty directory where pytest can be executed in isolation, and contains facilities
to write test, configuration files, and match against expected output.
Expand All @@ -521,7 +521,7 @@ To use it, include in your topmost ``conftest.py`` file:
.. autoclass:: PyTester()
.. autoclass:: Pytester()
:members:

.. autoclass:: RunResult()
Expand Down
47 changes: 26 additions & 21 deletions src/_pytest/pytester.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@

import pexpect

PathLike = os.PathLike[str]
else:
PathLike = os.PathLike


IGNORE_PAM = [ # filenames added when obtaining details about the current user
"/var/lib/sss/mc/passwd"
Expand Down Expand Up @@ -434,22 +438,22 @@ def LineMatcher_fixture(request: FixtureRequest) -> Type["LineMatcher"]:


@pytest.fixture
def pytester(request: FixtureRequest, tmp_path_factory: TempPathFactory) -> "PyTester":
def pytester(request: FixtureRequest, tmp_path_factory: TempPathFactory) -> "Pytester":
"""
Facilities to write tests/configuration files, execute pytest in isolation, and match
against expected output, perfect for black-box testing of pytest plugins.
It attempts to isolate the test run from external factors as much as possible, modifying
the current working directory to ``path`` and environment variables during initialization.
It attempts to isolate the test run from external factors as much as possible, modifying
the current working directory to ``tmp_path`` and environment variables during initialization.
It is particularly useful for testing plugins. It is similar to the :fixture:`tmp_path`
fixture but provides methods which aid in testing pytest itself.
"""
return PyTester(request, tmp_path_factory)
return Pytester(request, tmp_path_factory)


@pytest.fixture
def testdir(pytester: "PyTester") -> "Testdir":
def testdir(pytester: "Pytester") -> "Testdir":
"""
Identical to :fixture:`test_path`, and provides an instance whose methods return
legacy ``py.path.local`` objects instead when applicable.
Expand Down Expand Up @@ -619,7 +623,7 @@ def restore(self) -> None:


@final
class PyTester:
class Pytester:
"""
Facilities to write tests/configuration files, execute pytest in isolation, and match
against expected output, perfect for black-box testing of pytest plugins.
Expand Down Expand Up @@ -685,7 +689,7 @@ def path(self) -> Path:
return self._path

def __repr__(self) -> str:
return f"<PyTester {self.path!r}>"
return f"<Pytester {self.path!r}>"

def _finalize(self) -> None:
"""
Expand Down Expand Up @@ -888,9 +892,7 @@ def copy_example(self, name=None) -> Path:
example_path = maybe_file
else:
raise LookupError(
"{} cant be found as module or package in {}".format(
func_name, example_dir.bestrelpath(self._request.config.rootdir)
)
f"{func_name} cant be found as module or package in {example_dir}"
)
else:
example_path = example_dir.joinpath(name)
Expand All @@ -912,7 +914,9 @@ def copy_example(self, name=None) -> Path:

Session = Session

def getnode(self, config: Config, arg) -> Optional[Union[Collector, Item]]:
def getnode(
self, config: Config, arg: Union[PathLike, str]
) -> Optional[Union[Collector, Item]]:
"""Return the collection node of a file.
:param _pytest.config.Config config:
Expand All @@ -929,14 +933,15 @@ def getnode(self, config: Config, arg) -> Optional[Union[Collector, Item]]:
config.hook.pytest_sessionfinish(session=session, exitstatus=ExitCode.OK)
return res

def getpathnode(self, path):
def getpathnode(self, path: Union[PathLike, str]):
"""Return the collection node of a file.
This is like :py:meth:`getnode` but uses :py:meth:`parseconfigure` to
create the (configured) pytest Config instance.
:param py.path.local path: Path to the file.
"""
path = py.path.local(path)
config = self.parseconfigure(path)
session = Session.from_config(config)
x = session.fspath.bestrelpath(path)
Expand Down Expand Up @@ -1279,7 +1284,7 @@ def popen(

def run(
self,
*cmdargs: Union[str, py.path.local, Path],
*cmdargs: Union[str, PathLike],
timeout: Optional[float] = None,
stdin=CLOSE_STDIN,
) -> RunResult:
Expand All @@ -1290,10 +1295,10 @@ def run(
:param cmdargs:
The sequence of arguments to pass to `subprocess.Popen()`, with ``Path``
and ``py.path.local`` objects being converted to ``str`` automatically.
:kwarg timeout:
:param timeout:
The period in seconds after which to timeout and raise
:py:class:`Testdir.TimeoutExpired`
:kwarg stdin:
:param stdin:
Optional standard input. Bytes are being send, closing
the pipe, otherwise it is passed through to ``popen``.
Defaults to ``CLOSE_STDIN``, which translates to using a pipe
Expand Down Expand Up @@ -1461,19 +1466,19 @@ def assert_contains_lines(self, lines2: Sequence[str]) -> None:
@attr.s(repr=False, str=False)
class Testdir:
"""
Similar to :class:`PyTester`, but this class works with legacy py.path.local objects instead.
Similar to :class:`Pytester`, but this class works with legacy py.path.local objects instead.
All methods just forward to an internal :class:`PyTester` instance, converting results
All methods just forward to an internal :class:`Pytester` instance, converting results
to `py.path.local` objects as necessary.
"""

__test__ = False

CLOSE_STDIN = PyTester.CLOSE_STDIN
TimeoutExpired = PyTester.TimeoutExpired
Session = PyTester.Session
CLOSE_STDIN = Pytester.CLOSE_STDIN
TimeoutExpired = Pytester.TimeoutExpired
Session = Pytester.Session

_pytester = attr.ib(type=PyTester)
_pytester = attr.ib(type=Pytester)

@property
def tmpdir(self) -> py.path.local:
Expand Down
4 changes: 2 additions & 2 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from _pytest.compat import importlib_metadata
from _pytest.config import ExitCode
from _pytest.pathlib import symlink_or_skip
from _pytest.pytester import PyTester
from _pytest.pytester import Pytester


def prepend_pythonpath(*dirs):
Expand Down Expand Up @@ -1276,7 +1276,7 @@ def test_simple():
sys.platform == "win32",
reason="Windows raises `OSError: [Errno 22] Invalid argument` instead",
)
def test_no_brokenpipeerror_message(pytester: PyTester) -> None:
def test_no_brokenpipeerror_message(pytester: Pytester) -> None:
"""Ensure that the broken pipe error message is supressed.
In some Python versions, it reaches sys.unraisablehook, in others
Expand Down

0 comments on commit a8b07ce

Please sign in to comment.