From 44fbd3b02ad22326767dc37fe3b94aa93b36e8a3 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Thu, 27 Oct 2022 11:48:07 -0400 Subject: [PATCH] fix: in toml config, only apply environment substitution to coverage settings. #1481 --- CHANGES.rst | 10 ++++-- coverage/tomlconfig.py | 69 ++++++++++++++++++++++++++++-------------- doc/config.rst | 12 +++++--- tests/test_config.py | 19 +++++++----- 4 files changed, 75 insertions(+), 35 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 24f50907f..372c639dd 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -30,11 +30,17 @@ Unreleased - A ``[paths]`` setting like ``*/foo`` will now match ``foo/bar.py`` so that relative file paths can be combined more easily. -- Fix internal logic that prevented coverage.py from running on implementations - other than CPython or PyPy (`issue 1474`_). +- Fixed environment variable expansion in pyproject.toml files. It was overly + broad, causing errors outside of coverage.py settings, as described in `issue + 1481`_. This is now fixed, but in rare cases will require changing your + pyproject.toml to quote non-string values using environment substitution. + +- Fixed internal logic that prevented coverage.py from running on + implementations other than CPython or PyPy (`issue 1474`_). .. _issue 991: https://github.com/nedbat/coveragepy/issues/991 .. _issue 1474: https://github.com/nedbat/coveragepy/issues/1474 +.. _issue 1481: https://github.com/nedbat/coveragepy/issues/1481 .. _changes_6-5-0: diff --git a/coverage/tomlconfig.py b/coverage/tomlconfig.py index 148c34f89..0b5052b4c 100644 --- a/coverage/tomlconfig.py +++ b/coverage/tomlconfig.py @@ -52,7 +52,6 @@ def read(self, filenames): except OSError: return [] if tomllib is not None: - toml_text = substitute_variables(toml_text, os.environ) try: self.data = tomllib.loads(toml_text) except tomllib.TOMLDecodeError as err: @@ -101,9 +100,21 @@ def _get(self, section, option): if data is None: raise configparser.NoSectionError(section) try: - return name, data[option] + value = data[option] except KeyError as exc: raise configparser.NoOptionError(option, name) from exc + return name, value + + def _get_single(self, section, option): + """Get a single-valued option. + + Performs environment substitution if the value is a string. Other types + will be converted later as needed. + """ + name, value = self._get(section, option) + if isinstance(value, str): + value = substitute_variables(value, os.environ) + return name, value def has_option(self, section, option): _, data = self._get_section(section) @@ -126,29 +137,45 @@ def get_section(self, section): return data def get(self, section, option): - _, value = self._get(section, option) + _, value = self._get_single(section, option) return value - def _check_type(self, section, option, value, type_, type_desc): - if not isinstance(value, type_): - raise ValueError( - 'Option {!r} in section {!r} is not {}: {!r}' - .format(option, section, type_desc, value) - ) + def _check_type(self, section, option, value, type_, converter, type_desc): + """Check that `value` has the type we want, converting if needed. + + Returns the resulting value of the desired type. + """ + if isinstance(value, type_): + return value + if isinstance(value, str) and converter is not None: + try: + return converter(value) + except Exception as e: + raise ValueError( + f"Option [{section}]{option} couldn't convert to {type_desc}: {value!r}" + ) from e + raise ValueError( + f"Option [{section}]{option} is not {type_desc}: {value!r}" + ) def getboolean(self, section, option): - name, value = self._get(section, option) - self._check_type(name, option, value, bool, "a boolean") - return value + name, value = self._get_single(section, option) + bool_strings = {"true": True, "false": False} + return self._check_type(name, option, value, bool, bool_strings.__getitem__, "a boolean") - def getlist(self, section, option): + def _get_list(self, section, option): + """Get a list of strings, substituting environment variables in the elements.""" name, values = self._get(section, option) - self._check_type(name, option, values, list, "a list") + values = self._check_type(name, option, values, list, None, "a list") + values = [substitute_variables(value, os.environ) for value in values] + return name, values + + def getlist(self, section, option): + _, values = self._get_list(section, option) return values def getregexlist(self, section, option): - name, values = self._get(section, option) - self._check_type(name, option, values, list, "a list") + name, values = self._get_list(section, option) for value in values: value = value.strip() try: @@ -158,13 +185,11 @@ def getregexlist(self, section, option): return values def getint(self, section, option): - name, value = self._get(section, option) - self._check_type(name, option, value, int, "an integer") - return value + name, value = self._get_single(section, option) + return self._check_type(name, option, value, int, int, "an integer") def getfloat(self, section, option): - name, value = self._get(section, option) + name, value = self._get_single(section, option) if isinstance(value, int): value = float(value) - self._check_type(name, option, value, float, "a float") - return value + return self._check_type(name, option, value, float, float, "a float") diff --git a/doc/config.rst b/doc/config.rst index 0cb2cfa6c..6b7535795 100644 --- a/doc/config.rst +++ b/doc/config.rst @@ -31,10 +31,14 @@ Coverage.py will read settings from other usual configuration files if no other configuration file is used. It will automatically read from "setup.cfg" or "tox.ini" if they exist. In this case, the section names have "coverage:" prefixed, so the ``[run]`` options described below will be found in the -``[coverage:run]`` section of the file. If coverage.py is installed with the -``toml`` extra (``pip install coverage[toml]``), it will automatically read -from "pyproject.toml". Configuration must be within the ``[tool.coverage]`` -section, for example, ``[tool.coverage.run]``. +``[coverage:run]`` section of the file. + +Coverage.py will read from "pyproject.toml" if TOML support is available, +either because you are running on Python 3.11 or later, or because you +installed with the ``toml`` extra (``pip install coverage[toml]``). +Configuration must be within the ``[tool.coverage]`` section, for example, +``[tool.coverage.run]``. Environment variable expansion in values is +available, but only within quoted strings, even for non-string values. Syntax diff --git a/tests/test_config.py b/tests/test_config.py index 8db781b03..cb3edadb4 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -3,7 +3,6 @@ """Test the config file handling for coverage.py""" -import math import sys from collections import OrderedDict @@ -89,7 +88,7 @@ def test_toml_config_file(self): assert cov.config.plugins == ["plugins.a_plugin"] assert cov.config.precision == 3 assert cov.config.html_title == "tabblo & «ταБЬℓσ»" - assert math.isclose(cov.config.fail_under, 90.5) + assert cov.config.fail_under == 90.5 assert cov.config.get_plugin_options("plugins.a_plugin") == {"hello": "world"} # Test that our class doesn't reject integers when loading floats @@ -99,7 +98,7 @@ def test_toml_config_file(self): fail_under = 90 """) cov = coverage.Coverage(config_file="pyproject.toml") - assert math.isclose(cov.config.fail_under, 90) + assert cov.config.fail_under == 90 assert isinstance(cov.config.fail_under, float) def test_ignored_config_file(self): @@ -200,7 +199,7 @@ def test_parse_errors(self, bad_config, msg): r"multiple repeat"), ('[tool.coverage.run]\nconcurrency="foo"', "not a list"), ("[tool.coverage.report]\nprecision=1.23", "not an integer"), - ('[tool.coverage.report]\nfail_under="s"', "not a float"), + ('[tool.coverage.report]\nfail_under="s"', "couldn't convert to a float"), ]) def test_toml_parse_errors(self, bad_config, msg): # Im-parsable values raise ConfigError, with details. @@ -230,14 +229,15 @@ def test_environment_vars_in_config(self): assert cov.config.branch is True assert cov.config.exclude_list == ["the_$one", "anotherZZZ", "xZZZy", "xy", "huh${X}what"] - @pytest.mark.xfail(reason="updated to demonstrate bug #1481") def test_environment_vars_in_toml_config(self): # Config files can have $envvars in them. self.make_file("pyproject.toml", """\ [tool.coverage.run] data_file = "$DATA_FILE.fooey" - branch = $BRANCH + branch = "$BRANCH" [tool.coverage.report] + precision = "$DIGITS" + fail_under = "$FAIL_UNDER" exclude_lines = [ "the_$$one", "another${THING}", @@ -246,15 +246,20 @@ def test_environment_vars_in_toml_config(self): "huh$${X}what", ] [othersection] + # This reproduces the failure from https://github.com/nedbat/coveragepy/issues/1481 + # When OTHER has a backslash that isn't a valid escape, like \\z (see below). something = "if [ $OTHER ]; then printf '%s\\n' 'Hi'; fi" """) self.set_environ("BRANCH", "true") + self.set_environ("DIGITS", "3") + self.set_environ("FAIL_UNDER", "90.5") self.set_environ("DATA_FILE", "hello-world") self.set_environ("THING", "ZZZ") self.set_environ("OTHER", "hi\\zebra") cov = coverage.Coverage() - assert cov.config.data_file == "hello-world.fooey" assert cov.config.branch is True + assert cov.config.precision == 3 + assert cov.config.data_file == "hello-world.fooey" assert cov.config.exclude_list == ["the_$one", "anotherZZZ", "xZZZy", "xy", "huh${X}what"] def test_tilde_in_config(self):