Add pyproject.toml support by nicoddemus · Pull Request #7247 · pytest-dev/pytest · GitHub

(the code is ready for review I believe, keeping as draft because I need to update the docs)

(py35 failures might be related to #7303)

change looks good otherwise!

do we want to preserve external links to this target?

I don't really think it is necessary... we've changed section titles in the past. Or is there a way to preserve the old link that I'm not aware of?

Left some comments on the code. I've yet to read to issue to try to understand the ini_options approach but I get a sense of why it was done...

Should this be recursive? Otherwise I can write e.g. addopts = [true, 10].

I don't think so, that's invalid in TOML:

>>> import toml
>>> toml.loads('x = [true, 10]')
...
ValueError: Not a homogeneous array
>>> toml.loads('x = [10, 20]')
{'x': [10, 20]}

Recursive arrays are a possibility:

>>> toml.loads('x = [[10, 20], [30, 40]]')
{'x': [[10, 20], [30, 40]]}

But I would rather be safe here and convert them to string anyway, because that's what we would have gotten if we wrote that in an ini file.

Does toml actually give out both list and tuple? Would have assumed it's only one.

What about dicts?

dates and datetimes can also be unexpected, but I guess their str is fine.

You are right, fixed to only test for lists.

Everything else should be converted to strings I think, because that's what we would have gotten if we wrote them in an ini file. The tool.pytest.ini_options is a bridge to configure pytest using pyproject.toml files with the legacy options, hopefully we can in the future thinks of better ways to make advantage of the more powerful features of TOML.

Basically IIUC we want the Any here to be Union[None, str, List[str]].

I've changed to Union[str, List[str]], because I don't think TOML supports None?

Would be great if you're able to type-annotate this one 😁 Usually I don't ask this but in this case I think it can really be beneficial for clarifying things.

No worries, feel free to ask anytime... but I just did it, and yikes!

def getcfg(args: List[Union[str, py.path.local]]) -> Tuple[Optional[py.path.local], Optional[py.path.local], Optional[Dict[str, Union[str, List[str]]]]]:

Feel free to suggest how to improve readability here.

I gave it a go there, let me know if you have any suggestions however. 👍

'

Noticed also -c is not being handled correctly yet. Will need a bit more of time to work out the kinks.

This makes it clear each type of file that it is supported. Also dropped 'config' parameter as it is no longer used.

Fix pytest-dev#1556

While setup.cfg might be considered an "inifile", "pyproject.toml" definitely is not.

Rebased and did further refactorings in findpaths.py to be able to make -c somefile.toml to work.

Great work @nicoddemus, besides adding the new feature, the code in findpaths.py is much cleaner than before.

I agree that the approach taken is a decent way to support pyproject.toml in the midterm without too many consequences for the future or big code changes.

Personally I'd prefer "tool.pytest.ini" over "tool.pytest.ini_options".

I'd also add a short "note" comment in the documentation to explain why it's "pytest.tool.ini_options" and not just "tool.pytest" -- it's seems odd if unexplained.

There's also some type errors that come up when checking with proper py.path.local typing (pytest-dev/py#232), but there are also existing errors so I need to take care of that myself anyway.

This function no longer seems to be necessary

I'd also add a short "note" comment in the documentation to explain why it's "pytest.tool.ini_options" and not just "tool.pytest" -- it's seems odd if unexplained.

Done!

@nicoddemus Re. my comment above

Personally I'd prefer "tool.pytest.ini" over "tool.pytest.ini_options".

Not sure if you missed it or rejected it (which is fine of course).

Just seems to me the _options part is redundant.

Hi @bluetech

Oh sorry, didn't mean to seem dismissive, I thought you had seen this comment: #7247 (comment)

That's what we had agreed on the original issue. 😁

RonnyPfannschmidt reviewed May 23, 2020

View reviewed changes

Comment thread src/_pytest/config/findpaths.py Outdated

aklajnert reviewed Jun 1, 2020

View reviewed changes

Comment thread src/_pytest/config/findpaths.py Outdated

nicoddemus force-pushed the pyproject.toml branch 2 times, most recently from ae2d84a to a746cc4 Compare June 2, 2020 18:54

nicoddemus marked this pull request as ready for review June 2, 2020 19:04

nicoddemus commented Jun 2, 2020

View reviewed changes

Comment thread doc/en/customize.rst

nicoddemus force-pushed the pyproject.toml branch from 9c8e884 to 597b5c4 Compare June 2, 2020 22:45

nicoddemus requested review from RonnyPfannschmidt, asottile and bluetech June 3, 2020 15:45

asottile approved these changes Jun 3, 2020

View reviewed changes

bluetech reviewed Jun 3, 2020

View reviewed changes

RonnyPfannschmidt approved these changes Jun 3, 2020

View reviewed changes

nicoddemus added 12 commits June 4, 2020 18:46

Refactor getcfg function for clarity

2ccda38

This makes it clear each type of file that it is supported. Also dropped 'config' parameter as it is no longer used.

Add support for configuration in pyproject.toml

d37b01f

Fix pytest-dev#1556

Adjust to tool.pytest.ini_options

b5fce14

Change "inifile:" to "configfile:" in the terminal

d99d22c

While setup.cfg might be considered an "inifile", "pyproject.toml" definitely is not.

Use pyproject.toml for configuration in pytest itself

2a16e63

Add docs for pyproject.toml

9aa068c

Add CHANGELOG

d96a9fc

Apply more suggestions

a745f08

Support specifying a pyproject.toml file using -c

62aff27

Rename 'getcfg' to 'locate_cfg' and add type annotations

78fefdd

Fix iniconfig usage

953df45

Fix linting

99ddf36

nicoddemus force-pushed the pyproject.toml branch from 63dd5ba to 99ddf36 Compare June 4, 2020 22:04

bluetech approved these changes Jun 6, 2020

View reviewed changes

Comment thread src/_pytest/config/findpaths.py Outdated

Comment thread src/_pytest/config/findpaths.py Outdated

Comment thread src/_pytest/config/findpaths.py Outdated

Comment thread src/_pytest/config/findpaths.py Outdated

Comment thread src/_pytest/config/findpaths.py Outdated

nicoddemus added 4 commits June 6, 2020 19:45

Remove findpaths.exists()

f211ea6

This function no longer seems to be necessary

Improve coverage

a291796

Ignore local coverage html report

414b022

Apply type changes and note to pyproject.toml (review)

ffd5898

RonnyPfannschmidt approved these changes Jun 7, 2020

View reviewed changes

nicoddemus requested a review from aklajnert June 7, 2020 12:56

aklajnert approved these changes Jun 8, 2020

View reviewed changes

nicoddemus changed the title pyproject.toml support Add pyproject.toml support Jun 8, 2020

nicoddemus merged commit c17d508 into pytest-dev:master Jun 8, 2020

nicoddemus deleted the pyproject.toml branch June 8, 2020 13:03

ffe4 mentioned this pull request Jun 19, 2020

Support for pyproject.toml python/mypy#5205

Closed

+                      # convert all scalar values to strings for compatibility with other ini formats
+                      # conversion to actual useful values is made by Config._getini
+                      def make_scalar(v):
+                          return v if isinstance(v, (list, tuple)) else str(v)

-              Initialization: determining rootdir and inifile
-              -----------------------------------------------
+              Initialization: determining rootdir and configfile

+                  result = config.get("tool", {}).get("pytest", {}).get("ini_options", None)
+                  if result is not None:
+                      # convert all scalar values to strings for compatibility with other ini formats

+              def _get_ini_config_from_pyproject_toml(
+                  path: py.path.local,
+              ) -> Optional[Dict[str, Any]]:

		return None


		def getcfg(args):

Add pyproject.toml support by nicoddemus · Pull Request #7247 · pytest-dev/pytest · GitHub

Uh oh!

Uh oh!

Add pyproject.toml support - #7247

Add pyproject.toml support#7247nicoddemus merged 16 commits intopytest-dev:masterpytest-dev/pytest:masterfrom nicoddemus:pyproject.tomlnicoddemus/pytest:pyproject.tomlCopy head branch name to clipboard

Conversation

nicoddemus commented May 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicoddemus commented May 23, 2020

Uh oh!

Uh oh!

Uh oh!

nicoddemus commented Jun 2, 2020

Uh oh!

Uh oh!

asottile left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicoddemus Jun 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bluetech left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

Uh oh!

nicoddemus commented Jun 4, 2020

Uh oh!

nicoddemus commented Jun 4, 2020

Uh oh!

bluetech left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nicoddemus commented Jun 6, 2020

Uh oh!

bluetech commented Jun 10, 2020

Uh oh!

nicoddemus commented Jun 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Add pyproject.toml support#7247
nicoddemus merged 16 commits into
pytest-dev:masterfrom
nicoddemus:pyproject.toml

nicoddemus commented May 23, 2020 •
edited
Loading

nicoddemus Jun 4, 2020 •
edited
Loading