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.

Sign up for GitHub

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

argparsing: support parser.addini(type="paths") which returns pathlib.Paths #8594

Merged
merged 1 commit into from
May 7, 2021
Merged

argparsing: support parser.addini(type="paths") which returns pathlib.Paths #8594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/7259.feature.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
Added :meth:`cache.mkdir() <pytest.Cache.mkdir>`, which is similar to the existing :meth:`cache.makedir() <pytest.Cache.makedir>`,
but returns a :class:`pathlib.Path` instead of a legacy ``py.path.local``.

Added a ``paths`` type to :meth:`parser.addini() <_pytest.config.argparsing.Parser.addini>`,
as in ``parser.addini("mypaths", "my paths", type="paths")``,
which is similar to the existing ``pathlist``,
but returns a list of :class:`pathlib.Path` instead of legacy ``py.path.local``.
6 changes: 6 additions & 0 deletions src/_pytest/config/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1427,6 +1427,12 @@ def _getini(self, name: str):
dp = self.inipath.parent
input_values = shlex.split(value) if isinstance(value, str) else value
return [legacy_path(str(dp / x)) for x in input_values]
elif type == "paths":
# TODO: This assert is probably not valid in all cases.
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: this is copy/paste from branch above

assert self.inipath is not None
dp = self.inipath.parent
input_values = shlex.split(value) if isinstance(value, str) else value
return [dp / x for x in input_values]
elif type == "args":
return shlex.split(value) if isinstance(value, str) else value
elif type == "linelist":
Expand Down
27 changes: 20 additions & 7 deletions src/_pytest/config/argparsing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,22 +163,35 @@ def addini(
name: str,
help: str,
type: Optional[
"Literal['string', 'pathlist', 'args', 'linelist', 'bool']"
"Literal['string', 'paths', 'pathlist', 'args', 'linelist', 'bool']"
] = None,
default=None,
) -> None:
"""Register an ini-file option.

:name: Name of the ini-variable.
:type: Type of the variable, can be ``string``, ``pathlist``, ``args``,
``linelist`` or ``bool``. Defaults to ``string`` if ``None`` or
not passed.
:default: Default value if no ini-file option exists but is queried.
:name:
Name of the ini-variable.
:type:
Type of the variable. Can be:

* ``string``: a string
* ``bool``: a boolean
* ``args``: a list of strings, separated as in a shell
* ``linelist``: a list of strings, separated by line breaks
* ``paths``: a list of :class:`pathlib.Path`, separated as in a shell
* ``pathlist``: a list of ``py.path``, separated as in a shell

.. versionadded:: 6.3
The ``paths`` variable type.

Defaults to ``string`` if ``None`` or not passed.
:default:
Default value if no ini-file option exists but is queried.

The value of ini-variables can be retrieved via a call to
:py:func:`config.getini(name) <_pytest.config.Config.getini>`.
"""
assert type in (None, "string", "pathlist", "args", "linelist", "bool")
assert type in (None, "string", "paths", "pathlist", "args", "linelist", "bool")
Copy link
Member

Choose a reason for hiding this comment

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

lets put in a DeprecationWarning while at it

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should handle all of the py.path deprecations as one, otherwise we will need at least a dozen different warning types which would be a mess for the users.

Copy link
Member

Choose a reason for hiding this comment

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

we need different warnings for different cases, so im thinking its a little better to add them while we integrate that

plus it gives everyone the opportunity to start their own porting/support for the new apis as soon as they are available (instead of later) (all the things going to warn at once seems daunting to me personally)

Copy link
Member Author

Choose a reason for hiding this comment

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

we need different warnings for different cases, so im thinking its a little better to add them while we integrate that

I was thinking we collect them all together in one place https://docs.pytest.org/en/stable/deprecations.html.

plus it gives everyone the opportunity to start their own porting/support for the new apis as soon as they are available (instead of later) (all the things going to warn at once seems daunting to me personally)

Interesting, to me it's the exact opposite - I'd rather be able to do all the work at once, at a definite version point (especially as a plugin), then to do it in drips across multiple versions.

Copy link
Member

Choose a reason for hiding this comment

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

@bluetech makes me think i want to start a extra addon to enable determining when a warning should start to appear (so we can coordinate warning start and feature end more detailed, but thats for later, we can decide on the exact mechanism later

but yeah, the perspectives are indeed different, i prefer steady small changes over all at once changes,
but i also see the drag that can be triggered by having changes belonging to a larger scheme tickle in

im happy to leave the particular warnings out and defer the warning topic for later

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, thanks.

I'll try to work on the coordinated py.path deprecation today or tomorrow. We still have the reportinfo headache, but I'm think just deferring that specific one is OK.

Copy link
Member

Choose a reason for hiding this comment

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

I missed this review completely, but wanted to comment on this:

Interesting, to me it's the exact opposite - I'd rather be able to do all the work at once, at a definite version point (especially as a plugin), then to do it in drips across multiple versions.

Same for me, it would be much more annoying to have to deal with warnings for the same issues across multiple versions than just sitting down and fixing it once.

self._inidict[name] = (help, type, default)
self._ininames.append(name)

Expand Down
67 changes: 34 additions & 33 deletions testing/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,14 +595,14 @@ def test_getoption(self, pytester: Pytester) -> None:
def test_getconftest_pathlist(self, pytester: Pytester, tmp_path: Path) -> None:
somepath = tmp_path.joinpath("x", "y", "z")
p = tmp_path.joinpath("conftest.py")
p.write_text(f"pathlist = ['.', {str(somepath)!r}]")
p.write_text(f"mylist = {['.', os.fspath(somepath)]}")
config = pytester.parseconfigure(p)
assert (
config._getconftest_pathlist("notexist", path=tmp_path, rootpath=tmp_path)
is None
)
pl = (
config._getconftest_pathlist("pathlist", path=tmp_path, rootpath=tmp_path)
config._getconftest_pathlist("mylist", path=tmp_path, rootpath=tmp_path)
or []
)
print(pl)
Expand Down Expand Up @@ -634,41 +634,37 @@ def pytest_addoption(parser):
assert val == "hello"
pytest.raises(ValueError, config.getini, "other")

def make_conftest_for_pathlist(self, pytester: Pytester) -> None:
@pytest.mark.parametrize("config_type", ["ini", "pyproject"])
@pytest.mark.parametrize("ini_type", ["paths", "pathlist"])
def test_addini_paths(
self, pytester: Pytester, config_type: str, ini_type: str
) -> None:
pytester.makeconftest(
"""
f"""
def pytest_addoption(parser):
parser.addini("paths", "my new ini value", type="pathlist")
parser.addini("paths", "my new ini value", type="{ini_type}")
parser.addini("abc", "abc value")
"""
)

def test_addini_pathlist_ini_files(self, pytester: Pytester) -> None:
self.make_conftest_for_pathlist(pytester)
p = pytester.makeini(
if config_type == "ini":
inipath = pytester.makeini(
"""
[pytest]
paths=hello world/sub.py
"""
[pytest]
paths=hello world/sub.py
"""
)
self.check_config_pathlist(pytester, p)

def test_addini_pathlist_pyproject_toml(self, pytester: Pytester) -> None:
self.make_conftest_for_pathlist(pytester)
p = pytester.makepyprojecttoml(
)
elif config_type == "pyproject":
inipath = pytester.makepyprojecttoml(
"""
[tool.pytest.ini_options]
paths=["hello", "world/sub.py"]
"""
[tool.pytest.ini_options]
paths=["hello", "world/sub.py"]
"""
)
self.check_config_pathlist(pytester, p)

def check_config_pathlist(self, pytester: Pytester, config_path: Path) -> None:
)
config = pytester.parseconfig()
values = config.getini("paths")
assert len(values) == 2
assert values[0] == config_path.parent.joinpath("hello")
assert values[1] == config_path.parent.joinpath("world/sub.py")
assert values[0] == inipath.parent.joinpath("hello")
assert values[1] == inipath.parent.joinpath("world/sub.py")
pytest.raises(ValueError, config.getini, "other")

def make_conftest_for_args(self, pytester: Pytester) -> None:
Expand Down Expand Up @@ -1519,24 +1515,29 @@ def test_pass(pytestconfig):
assert result.ret == 0
result.stdout.fnmatch_lines(["custom_option:3.0"])

def test_override_ini_pathlist(self, pytester: Pytester) -> None:
@pytest.mark.parametrize("ini_type", ["paths", "pathlist"])
def test_override_ini_paths(self, pytester: Pytester, ini_type: str) -> None:
pytester.makeconftest(
"""
f"""
def pytest_addoption(parser):
parser.addini("paths", "my new ini value", type="pathlist")"""
parser.addini("paths", "my new ini value", type="{ini_type}")"""
)
pytester.makeini(
"""
[pytest]
paths=blah.py"""
)
pytester.makepyfile(
"""
def test_pathlist(pytestconfig):
rf"""
def test_overriden(pytestconfig):
config_paths = pytestconfig.getini("paths")
print(config_paths)
for cpf in config_paths:
print('\\nuser_path:%s' % cpf.basename)"""
if "{ini_type}" == "pathlist":
print('\nuser_path:%s' % cpf.basename)
else:
print('\nuser_path:%s' % cpf.name)
"""
)
result = pytester.runpytest(
"--override-ini", "paths=foo/bar1.py foo/bar2.py", "-s"
Expand Down