refactor config file finding - #8358
Conversation
fe3b062 to
cbee643
Compare
|
The discussion in #3523 is interesting, seems this will bring a lot of pain to our users that still keep their configuration in What would be the downsides of keeping the existing support, even if for some options it doesn't work so well ( |
|
@nicoddemus in that case lets turn in future we might want to try using config-parser to load based on recent developments of config-parser it may even be possible o drop inconfig and use different config-parser configurations for different config-file setups as part of a breaking release |
I would be fine with that. 👍 We can remove examples of
I'm not keen on the details, but the overall idea seems good. 😁 |
cbee643 to
015fcf4
Compare
|
reviewed the docs and decided the warnings @nicoddemus did put in way earlier are good enough |
ed2ac09 to
7129605
Compare
8ebc068 to
c6f6e59
Compare
its already taken care of earlier
c6f6e59 to
4ac06da
Compare
nicoddemus
left a comment
There was a problem hiding this comment.
Overall the changes look good, please take a look at my comments. 👍
|
|
||
| # tox.ini | ||
| [pytest] | ||
| [tool:pytest] |
There was a problem hiding this comment.
This doesn't seem right, AFAIK for tox.ini files we use the [pytest] section.
| 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.
| ``setup.cfg`` file usage for pytest has been deprecated, its recommended to use ``tox.ini`` or ``pyproject.toml`` | |
| ``setup.cfg`` file usage for pytest is supported but is not recommended for new projects. Instead, use ``tox.ini`` or ``pyproject.toml`` files. |
It is not really deprecated in the sense that we will remove it in the future.
| ) -> 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.
| """Parse the legacy pytest.ini and return the contents of the pytest section | |
| """Parse the pytest.ini file and return the contents of the pytest section |
I don't think we should consider pytest.ini files "legacy" in any way. 😁
|
|
||
| def locate_config( | ||
| args: Iterable[Path], | ||
| args: List[Path], |
There was a problem hiding this comment.
Minor, but for non-mutable parameters I think Sequence is more adequate.
| "section, name", | ||
| [ | ||
| ("tool:pytest", "setup.cfg"), | ||
| pytest.param("tool:pytest", "setup.cfg"), |
| """Parse the legacy pytest.ini and return the contents of the pytest section | ||
|
|
||
| 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.
| if the file exists and lacks a pytest section, consider it empty""" | |
| if the file exists and lacks a pytest section, consider it empty.""" |
|
|
||
|
|
||
| def _parse_ini_file(path: Path) -> PARSE_RESULT: | ||
| """Parses .ini files with expected pytest.ini sections |
There was a problem hiding this comment.
| """Parses .ini files with expected pytest.ini sections | |
| """Parses .ini files with expected pytest.ini sections. |
| def _parse_ini_file(path: Path) -> PARSE_RESULT: | ||
| """Parses .ini files with expected pytest.ini sections | ||
|
|
||
| todo: investigate if tool:pytest should be added |
There was a problem hiding this comment.
| todo: investigate if tool:pytest should be added | |
| TODO: Investigate if tool:pytest should be added. |
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?).
|
Superseded by #14807. This PR bundled two things: the structural refactor of config file finding, and the #14807 redoes only the structural half against current It could not be rebased: this branch predates Closing this one to keep the queue honest. |
Knowledge about config files was spread across three places that had to be kept in sync by hand: load_config_dict_from_file dispatched on suffix with filename checks nested inside those branches, locate_config re-declared the discovery order in its own hardcoded list, and adding a format meant editing both in the right order. Introduce two tables instead. CONFIG_LOADERS maps a config file *name* to its loader and doubles as the discovery order used by locate_config, so order and parsing can no longer drift apart. CONFIG_SUFFIXES maps a *suffix* to a loader for files passed explicitly via -c/--config-file, which may be named anything. load_config_dict_from_file now looks up the name first and falls back to the suffix. The per-format rules move out of nested conditionals into named functions -- _parse_pytest_ini, _parse_ini_file, _parse_cfg_file, _parse_pytest_toml and _parse_pyproject_toml -- each documenting the rule it implements. This is a pure refactor: no test needed changing, and running load_config_dict_from_file over a matrix of every supported name crossed with present/absent/empty/malformed sections (plus unsupported and extension-less files) yields identical values, modes, origins and exceptions before and after. Two pre-existing warts are deliberately preserved rather than fixed here: the CFG_PYTEST_SECTION message still names setup.cfg for any .cfg file, and a scalar `pytest` key still raises AttributeError. This redoes the structural half of pytest-dev#8358 on top of current main; that PR bundled it with the abandoned setup.cfg deprecation from pytest-dev#3523. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
addresses #3523
setup.cfg
also a structural refactor and cleanup of the config file finding/loading