-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
refactor config file finding #8358
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
8db673c
4c3d797
17514c1
5c3fea0
dd7f52b
7012fe3
3e01de5
4ac06da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Internal Refactoring for finding/loading config files. |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -79,7 +79,7 @@ and can also be used to hold pytest configuration if they have a ``[pytest]`` se | |||||
.. code-block:: ini | ||||||
|
||||||
# tox.ini | ||||||
[pytest] | ||||||
[tool:pytest] | ||||||
minversion = 6.0 | ||||||
addopts = -ra -q | ||||||
testpaths = | ||||||
|
@@ -90,26 +90,7 @@ and can also be used to hold pytest configuration if they have a ``[pytest]`` se | |||||
setup.cfg | ||||||
~~~~~~~~~ | ||||||
|
||||||
``setup.cfg`` files are general purpose configuration files, used originally by :doc:`distutils <distutils/configfile>`, and can also be used to hold pytest configuration | ||||||
if they have a ``[tool:pytest]`` section. | ||||||
|
||||||
.. code-block:: ini | ||||||
|
||||||
# setup.cfg | ||||||
[tool:pytest] | ||||||
minversion = 6.0 | ||||||
addopts = -ra -q | ||||||
testpaths = | ||||||
tests | ||||||
integration | ||||||
|
||||||
.. warning:: | ||||||
|
||||||
Usage of ``setup.cfg`` is not recommended unless for very simple use cases. ``.cfg`` | ||||||
files use a different parser than ``pytest.ini`` and ``tox.ini`` which might cause hard to track | ||||||
down problems. | ||||||
When possible, it is recommended to use the latter files, or ``pyproject.toml``, to hold your | ||||||
pytest configuration. | ||||||
``setup.cfg`` file usage for pytest has been deprecated, its recommended to use ``tox.ini`` or ``pyproject.toml`` | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
It is not really deprecated in the sense that we will remove it in the future. |
||||||
|
||||||
|
||||||
.. _rootdir: | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -10,109 +10,148 @@ | |||||
from typing import TYPE_CHECKING | ||||||
from typing import Union | ||||||
|
||||||
import iniconfig | ||||||
|
||||||
from .exceptions import UsageError | ||||||
from _pytest.outcomes import fail | ||||||
from _pytest.pathlib import absolutepath | ||||||
from _pytest.pathlib import commonpath | ||||||
|
||||||
if TYPE_CHECKING: | ||||||
from . import Config | ||||||
from iniconfig import IniConfig # NOQA: F401 | ||||||
|
||||||
PARSE_RESULT = Optional[Dict[str, Union[str, List[str]]]] | ||||||
|
||||||
|
||||||
def _parse_ini_config(path: Path) -> iniconfig.IniConfig: | ||||||
def _parse_ini_config(path: Path) -> "IniConfig": | ||||||
"""Parse the given generic '.ini' file using legacy IniConfig parser, returning | ||||||
the parsed object. | ||||||
|
||||||
Raise UsageError if the file cannot be parsed. | ||||||
""" | ||||||
from iniconfig import IniConfig, ParseError # NOQA: F811 | ||||||
|
||||||
try: | ||||||
return iniconfig.IniConfig(str(path)) | ||||||
except iniconfig.ParseError as exc: | ||||||
return IniConfig(os.fspath(path), data=path.read_text()) | ||||||
except ParseError as exc: | ||||||
raise UsageError(str(exc)) from exc | ||||||
|
||||||
|
||||||
def load_config_dict_from_file( | ||||||
filepath: Path, | ||||||
) -> Optional[Dict[str, Union[str, List[str]]]]: | ||||||
"""Load pytest configuration from the given file path, if supported. | ||||||
def _parse_pytest_ini(path: Path) -> PARSE_RESULT: | ||||||
"""Parse the legacy pytest.ini and return the contents of the pytest section | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I don't think we should consider |
||||||
|
||||||
Return None if the file does not contain valid pytest configuration. | ||||||
if the file exists and lacks a pytest section, consider it empty""" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
iniconfig = _parse_ini_config(path) | ||||||
|
||||||
if "pytest" in iniconfig: | ||||||
return dict(iniconfig["pytest"].items()) | ||||||
else: | ||||||
# "pytest.ini" files are always the source of configuration, even if empty. | ||||||
return {} | ||||||
|
||||||
|
||||||
def _parse_ini_file(path: Path) -> PARSE_RESULT: | ||||||
"""Parses .ini files with expected pytest.ini sections | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
todo: investigate if tool:pytest should be added | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
However I think it would be best to either create a new issue, or write more details about what you mean with this "TODO" (or is this something you still plan to work on this PR?). |
||||||
""" | ||||||
iniconfig = _parse_ini_config(path) | ||||||
|
||||||
# Configuration from ini files are obtained from the [pytest] section, if present. | ||||||
if filepath.suffix == ".ini": | ||||||
iniconfig = _parse_ini_config(filepath) | ||||||
if "pytest" in iniconfig: | ||||||
return dict(iniconfig["pytest"].items()) | ||||||
return None | ||||||
|
||||||
if "pytest" in iniconfig: | ||||||
return dict(iniconfig["pytest"].items()) | ||||||
else: | ||||||
# "pytest.ini" files are always the source of configuration, even if empty. | ||||||
if filepath.name == "pytest.ini": | ||||||
return {} | ||||||
|
||||||
# '.cfg' files are considered if they contain a "[tool:pytest]" section. | ||||||
elif filepath.suffix == ".cfg": | ||||||
iniconfig = _parse_ini_config(filepath) | ||||||
|
||||||
if "tool:pytest" in iniconfig.sections: | ||||||
return dict(iniconfig["tool:pytest"].items()) | ||||||
elif "pytest" in iniconfig.sections: | ||||||
# If a setup.cfg contains a "[pytest]" section, we raise a failure to indicate users that | ||||||
# plain "[pytest]" sections in setup.cfg files is no longer supported (#3086). | ||||||
fail(CFG_PYTEST_SECTION.format(filename="setup.cfg"), pytrace=False) | ||||||
|
||||||
# '.toml' files are considered if they contain a [tool.pytest.ini_options] table. | ||||||
elif filepath.suffix == ".toml": | ||||||
if sys.version_info >= (3, 11): | ||||||
import tomllib | ||||||
else: | ||||||
import tomli as tomllib | ||||||
|
||||||
toml_text = filepath.read_text(encoding="utf-8") | ||||||
try: | ||||||
config = tomllib.loads(toml_text) | ||||||
except tomllib.TOMLDecodeError as exc: | ||||||
raise UsageError(f"{filepath}: {exc}") from exc | ||||||
def _parse_cfg_file(path: Path) -> PARSE_RESULT: | ||||||
"""Parses .cfg files, specifically used for setup.cfg support | ||||||
|
||||||
tool:pytest as section name is required | ||||||
""" | ||||||
|
||||||
iniconfig = _parse_ini_config(path) | ||||||
|
||||||
result = config.get("tool", {}).get("pytest", {}).get("ini_options", None) | ||||||
if result is not None: | ||||||
# TOML supports richer data types than ini files (strings, arrays, floats, ints, etc), | ||||||
# however we need to convert all scalar values to str for compatibility with the rest | ||||||
# of the configuration system, which expects strings only. | ||||||
def make_scalar(v: object) -> Union[str, List[str]]: | ||||||
return v if isinstance(v, list) else str(v) | ||||||
if "tool:pytest" in iniconfig.sections: | ||||||
return dict(iniconfig["tool:pytest"].items()) | ||||||
elif "pytest" in iniconfig.sections: | ||||||
# If a setup.cfg contains a "[pytest]" section, we raise a failure to indicate users that | ||||||
# plain "[pytest]" sections in setup.cfg files is no longer supported (#3086). | ||||||
fail(CFG_PYTEST_SECTION.format(filename="setup.cfg"), pytrace=False) | ||||||
else: | ||||||
return None | ||||||
|
||||||
|
||||||
def _parse_pyproject_ini_options( | ||||||
filepath: Path, | ||||||
) -> PARSE_RESULT: | ||||||
"""Load backward compatible ini options from pyproject.toml""" | ||||||
|
||||||
if sys.version_info >= (3, 11): | ||||||
import tomllib | ||||||
else: | ||||||
import tomli as tomllib | ||||||
|
||||||
toml_text = filepath.read_text(encoding="utf-8") | ||||||
try: | ||||||
config = tomllib.loads(toml_text) | ||||||
except tomllib.TOMLDecodeError as exc: | ||||||
raise UsageError(f"{filepath}: {exc}") from exc | ||||||
|
||||||
result = config.get("tool", {}).get("pytest", {}).get("ini_options", None) | ||||||
if result is not None: | ||||||
# TOML supports richer data types than ini files (strings, arrays, floats, ints, etc), | ||||||
# however we need to convert all scalar values to str for compatibility with the rest | ||||||
# of the configuration system, which expects strings only. | ||||||
def make_scalar(v: object) -> Union[str, List[str]]: | ||||||
return v if isinstance(v, list) else str(v) | ||||||
|
||||||
return {k: make_scalar(v) for k, v in result.items()} | ||||||
else: | ||||||
return None | ||||||
|
||||||
return {k: make_scalar(v) for k, v in result.items()} | ||||||
|
||||||
CONFIG_LOADERS = { | ||||||
"pytest.ini": _parse_pytest_ini, | ||||||
".pytest.ini": _parse_pytest_ini, | ||||||
"pyproject.toml": _parse_pyproject_ini_options, | ||||||
"tox.ini": _parse_ini_file, | ||||||
"setup.cfg": _parse_cfg_file, | ||||||
} | ||||||
|
||||||
CONFIG_SUFFIXES = { | ||||||
".ini": _parse_ini_file, | ||||||
".cfg": _parse_cfg_file, | ||||||
".toml": _parse_pyproject_ini_options, | ||||||
} | ||||||
|
||||||
|
||||||
def load_config_dict_from_file(path: Path) -> PARSE_RESULT: | ||||||
"""Load pytest configuration from the given file path, if supported. | ||||||
|
||||||
Return None if the file does not contain valid pytest configuration. | ||||||
""" | ||||||
if path.name in CONFIG_LOADERS: | ||||||
return CONFIG_LOADERS[path.name](path) | ||||||
if path.suffix in CONFIG_SUFFIXES: | ||||||
return CONFIG_SUFFIXES[path.suffix](path) | ||||||
return None | ||||||
|
||||||
|
||||||
def locate_config( | ||||||
args: Iterable[Path], | ||||||
args: List[Path], | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor, but for non-mutable parameters I think |
||||||
) -> Tuple[Optional[Path], Optional[Path], Dict[str, Union[str, List[str]]]]: | ||||||
"""Search in the list of arguments for a valid ini-file for pytest, | ||||||
and return a tuple of (rootdir, inifile, cfg-dict).""" | ||||||
config_names = [ | ||||||
"pytest.ini", | ||||||
".pytest.ini", | ||||||
"pyproject.toml", | ||||||
"tox.ini", | ||||||
"setup.cfg", | ||||||
] | ||||||
args = [x for x in args if not str(x).startswith("-")] | ||||||
|
||||||
if not args: | ||||||
args = [Path.cwd()] | ||||||
for arg in args: | ||||||
argpath = absolutepath(arg) | ||||||
for base in (argpath, *argpath.parents): | ||||||
for config_name in config_names: | ||||||
for config_name, loader in CONFIG_LOADERS.items(): | ||||||
p = base / config_name | ||||||
if p.is_file(): | ||||||
ini_config = load_config_dict_from_file(p) | ||||||
if ini_config is not None: | ||||||
return base, p, ini_config | ||||||
config = loader(p) | ||||||
if config is not None: | ||||||
return base, p, config | ||||||
return None, None, {} | ||||||
|
||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,7 +113,7 @@ def test_tox_ini_wrong_version(self, pytester: Pytester) -> None: | |
@pytest.mark.parametrize( | ||
"section, name", | ||
[ | ||
("tool:pytest", "setup.cfg"), | ||
pytest.param("tool:pytest", "setup.cfg"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems unnecessary? |
||
("pytest", "tox.ini"), | ||
("pytest", "pytest.ini"), | ||
("pytest", ".pytest.ini"), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem right, AFAIK for
tox.ini
files we use the[pytest]
section.