fix: in toml config, only apply environment substitution to coverage settings. #1481

Author: nedbat

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,16 @@ 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_simple(self, section, option):
+        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 +132,40 @@ def get_section(self, section):
         return data
 
     def get(self, section, option):
-        _, value = self._get(section, option)
+        _, value = self._get_simple(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):
+        if isinstance(value, type_):
+            return value
+        if isinstance(value, str) and converter is not None:
+            try:
+                return converter(value)
+            except Exception:
+                raise ValueError(
+                    f"Option [{section}]{option} couldn't convert to {type_desc}: {value!r}"
+                ) from None
+        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_simple(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 _getlist(self, section, option):
         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._getlist(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._getlist(section, option)
         for value in values:
             value = value.strip()
             try:
@@ -158,13 +175,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_simple(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_simple(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")

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.
@@ -235,8 +234,10 @@ def test_environment_vars_in_toml_config(self):
         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}",
@@ -245,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):