Add test coverage, fix circular reference config

Signed-off-by: Bernát Gábor <[email protected]>

Author: gaborbernat

src/tox/config/cli/ini.py

@@ -51,7 +51,7 @@ def get(self, key: str, of_type: Type[Any]) -> Any:
                     result = None
                 else:
                     source = "file"
-                    value = self.ini.load(key, of_type=of_type, conf=None, env_name="tox")
+                    value = self.ini.load(key, of_type=of_type, conf=None, env_name="tox", chain=[key])
                     result = value, source
             except KeyError:  # just not found
                 result = None

src/tox/config/loader/api.py

@@ -71,7 +71,9 @@ def found_keys(self) -> Set[str]:
     def __repr__(self) -> str:
         return f"{type(self).__name__}"
 
-    def load(self, key: str, of_type: Type[V], conf: Optional["Config"], env_name: Optional[str]) -> V:
+    def load(
+        self, key: str, of_type: Type[V], conf: Optional["Config"], env_name: Optional[str], chain: List[str]
+    ) -> V:
         """
         Load a value.
 
@@ -85,14 +87,21 @@ def load(self, key: str, of_type: Type[V], conf: Optional["Config"], env_name: O
             return _STR_CONVERT.to(self.overrides[key].value, of_type)
         raw = self.load_raw(key, conf, env_name)
         future: "Future[V]" = Future()
-        with self.build(future, key, of_type, conf, env_name, raw) as prepared:
+        with self.build(future, key, of_type, conf, env_name, raw, chain) as prepared:
             converted = self.to(prepared, of_type)
             future.set_result(converted)
         return converted
 
     @contextmanager
     def build(
-        self, future: "Future[V]", key: str, of_type: Type[V], conf: Optional["Config"], env_name: Optional[str], raw: T
+        self,
+        future: "Future[V]",
+        key: str,
+        of_type: Type[V],
+        conf: Optional["Config"],
+        env_name: Optional[str],
+        raw: T,
+        chain: List[str],
     ) -> Generator[T, None, None]:
         yield raw
 

src/tox/config/loader/ini/__init__.py

@@ -46,26 +46,27 @@ def build(
         conf: Optional["Config"],
         env_name: Optional[str],
         raw: str,
+        chain: List[str],
     ) -> Generator[str, None, None]:
         delay_replace = inspect.isclass(of_type) and issubclass(of_type, SetEnv)
 
-        def replacer(raw_: str, chain: List[str]) -> str:
+        def replacer(raw_: str, chain_: List[str]) -> str:
             if conf is None:
                 replaced = raw_  # no replacement supported in the core section
             else:
                 try:
-                    replaced = replace(conf, env_name, self, raw_, chain)  # do replacements
+                    replaced = replace(conf, env_name, self, raw_, chain_)  # do replacements
                 except Exception as exception:
                     msg = f"replace failed in {'tox' if env_name is None else env_name}.{key} with {exception!r}"
                     raise HandledError(msg) from exception
             return replaced
 
         if not delay_replace:
-            raw = replacer(raw, [key])
+            raw = replacer(raw, chain)
         yield raw
         if delay_replace:
             converted = future.result()
-            if hasattr(converted, "replacer"):
+            if hasattr(converted, "replacer"):  # pragma: no branch
                 converted.replacer = replacer  # type: ignore[attr-defined]
 
     def found_keys(self) -> Set[str]:

src/tox/config/loader/ini/replace.py

@@ -83,10 +83,7 @@ def _replace_match(
     elif of_type == "posargs":
         replace_value = replace_pos_args(args, conf.pos_args)
     else:
-        if of_type in chain:
-            raise ValueError(f"circular chain detected {', '.join(chain[chain.index(of_type):])}")
-        chain.append(of_type)
-        replace_value = replace_reference(conf, current_env, loader, value)
+        replace_value = replace_reference(conf, current_env, loader, value, chain)
     return replace_value
 
 
@@ -105,6 +102,7 @@ def replace_reference(
     current_env: Optional[str],
     loader: "IniLoader",
     value: str,
+    chain: List[str],
 ) -> Optional[str]:
     # a return value of None indicates could not replace
     match = _REPLACE_REF.match(value)
@@ -121,7 +119,7 @@ def replace_reference(
                 try:
                     if isinstance(src, SectionProxy):
                         return src[key]
-                    value = src[key]
+                    value = src.load(key, chain)
                     as_str, _ = stringify(value)
                     return as_str
                 except KeyError as exc:  # if fails, keep trying maybe another source can satisfy
@@ -185,12 +183,15 @@ def replace_env(conf: Config, env_name: Optional[str], args: List[str], chain: L
     key = args[0]
     new_key = f"env:{key}"
 
-    if env_name is not None and new_key not in chain:  # check if set env
-        chain.append(new_key)
-        env_conf = conf.get_env(env_name)
-        set_env: SetEnv = env_conf["set_env"]
-        if key in set_env:
-            return set_env.load(key, chain)
+    if env_name is not None:  # on core no set env support # pragma: no branch
+        if new_key not in chain:  # check if set env
+            chain.append(new_key)
+            env_conf = conf.get_env(env_name)
+            set_env: SetEnv = env_conf["set_env"]
+            if key in set_env:
+                return set_env.load(key, chain)
+        elif chain[-1] != new_key:  # if there's a chain but only self-refers than use os.environ
+            raise ValueError(f"circular chain between set env {', '.join(i[4:] for i in chain[chain.index(new_key):])}")
 
     if key in os.environ:
         return os.environ[key]

src/tox/config/loader/str_convert.py

@@ -53,7 +53,7 @@ def to_command(value: str) -> Command:
         is_win = sys.platform == "win32"
         splitter = shlex.shlex(value, posix=not is_win)
         splitter.whitespace_split = True
-        if is_win:
+        if is_win:  # pragma: win32 cover
             args: List[str] = []
             for arg in splitter:
                 # on Windows quoted arguments will remain quoted, strip it

src/tox/config/loader/stringify.py

@@ -28,8 +28,7 @@ def stringify(value: Any) -> Tuple[str, bool]:
     if isinstance(value, Command):
         return value.shell, True
     if isinstance(value, SetEnv):
-        return "\n".join(f"{stringify(k)[0]}={stringify(value.load(k))[0]}" for k in sorted(list(value))), True
-
+        return stringify({k: value.load(k) for k in sorted(list(value))})
     return str(value), False
 
 

src/tox/config/of_type.py

@@ -23,7 +23,7 @@ def __init__(self, keys: Iterable[str], desc: str, env_name: Optional[str]) -> N
         self.env_name = env_name
 
     @abstractmethod
-    def __call__(self, conf: "Config", key: Optional[str], loaders: List[Loader[T]]) -> T:
+    def __call__(self, conf: "Config", key: Optional[str], loaders: List[Loader[T]], chain: List[str]) -> T:
         raise NotImplementedError
 
     def __eq__(self, o: Any) -> bool:
@@ -46,7 +46,7 @@ def __init__(
         super().__init__(keys, desc, env_name)
         self.value = value
 
-    def __call__(self, conf: "Config", name: Optional[str], loaders: List[Loader[T]]) -> T:
+    def __call__(self, conf: "Config", name: Optional[str], loaders: List[Loader[T]], chain: List[str]) -> T:
         if callable(self.value):
             value = self.value()
         else:
@@ -78,13 +78,13 @@ def __init__(
         self.post_process = post_process
         self._cache: Union[object, T] = _PLACE_HOLDER
 
-    def __call__(self, conf: "Config", name: Optional[str], loaders: List[Loader[T]]) -> T:
+    def __call__(self, conf: "Config", name: Optional[str], loaders: List[Loader[T]], chain: List[str]) -> T:
         if self._cache is _PLACE_HOLDER:
             found = False
             for key in self.keys:
                 for loader in loaders:
                     try:
-                        value = loader.load(key, self.of_type, conf, self.env_name)
+                        value = loader.load(key, self.of_type, conf, self.env_name, chain)
                         found = True
                     except KeyError:
                         continue

src/tox/config/set_env.py

@@ -1,10 +1,11 @@
 from typing import Callable, Dict, Iterator, List, Mapping, Optional, Tuple
 
+Replacer = Callable[[str, List[str]], str]
+
 
 class SetEnv:
     def __init__(self, raw: str) -> None:
-        self.replacer: Callable[[str, List[str]], str] = lambda s, c: s
-        # resolve tty, posargs, etc?
+        self.replacer: Replacer = lambda s, c: s
         lines = raw.splitlines()
         self._later: List[str] = []
         self._raw: Dict[str, str] = {}
@@ -15,7 +16,7 @@ def __init__(self, raw: str) -> None:
                 try:
                     key, value = self._extract_key_value(line)
                     if "{" in key:
-                        raise ValueError
+                        raise ValueError(f"invalid line {line!r} in set_env")
                 except ValueError:
                     _, __, match = find_replace_part(line, 0, 0)
                     if match:
@@ -28,9 +29,10 @@ def __init__(self, raw: str) -> None:
 
     @staticmethod
     def _extract_key_value(line: str) -> Tuple[str, str]:
-        at = line.index("=")
-        if at == -1:
-            raise ValueError(line)
+        try:
+            at = line.index("=")
+        except ValueError:
+            raise ValueError(f"invalid line {line!r} in set_env")
         key, value = line[:at], line[at + 1 :]
         return key.strip(), value.strip()
 

src/tox/config/sets.py

@@ -81,8 +81,16 @@ def _add_conf(self, keys: Sequence[str], definition: ConfigDefinition[V]) -> Con
         return definition
 
     def __getitem__(self, item: str) -> Any:
+        return self.load(item)
+
+    def load(self, item: str, chain: Optional[List[str]] = None) -> Any:
         config_definition = self._defined[item]
-        return config_definition(self._conf, item, self.loaders)
+        if chain is None:
+            chain = []
+        if item in chain:
+            raise ValueError(f"circular chain detected {', '.join(chain[chain.index(item):])}")
+        chain.append(item)
+        return config_definition(self._conf, item, self.loaders, chain)
 
     def __repr__(self) -> str:
         values = (v for v in (f"name={self.name!r}" if self.name else "", f"loaders={self.loaders!r}") if v)

src/tox/provision.py

@@ -73,7 +73,7 @@ def provision(state: State) -> Union[int, bool]:
     for package in requires:
         package_name = canonicalize_name(package.name)
         try:
-            dist = distribution(package_name)
+            dist = distribution(package_name)  # type: ignore[no-untyped-call]
             if not package.specifier.contains(dist.version, prereleases=True):
                 missing.append((package, dist.version))
         except PackageNotFoundError:

src/tox/pytest.py

@@ -32,6 +32,7 @@
 from virtualenv.info import IS_WIN, fs_supports_symlink
 
 import tox.run
+from tox.config.sets import EnvConfigSet
 from tox.execute.api import Execute, ExecuteInstance, ExecuteStatus, Outcome
 from tox.execute.request import ExecuteRequest, shell_cmd
 from tox.execute.stream import SyncWrite
@@ -299,6 +300,9 @@ def state(self) -> State:
             raise RuntimeError("no state")
         return self._state
 
+    def env_conf(self, name: str) -> EnvConfigSet:
+        return self.state.conf.get_env(name)
+
     @property
     def success(self) -> bool:
         return self.code == Outcome.OK

src/tox/tox_env/python/virtual_env/package/api.py

@@ -122,7 +122,7 @@ def _ensure_meta_present(self) -> None:
             return  # pragma: no cover
         self.ensure_setup()
         dist_info = self.prepare_metadata_for_build_wheel(self.meta_folder).metadata
-        self._distribution_meta = Distribution.at(str(dist_info))
+        self._distribution_meta = Distribution.at(str(dist_info))  # type: ignore[no-untyped-call]
 
     @abstractmethod
     def _build_artifact(self) -> Path:
@@ -163,18 +163,16 @@ def discover_package_dependencies(  # type: ignore[no-any-unimported]
             ):
                 if marker_key.value == "extra" and op.value == "==":  # pragma: no branch
                     extra = marker_value.value
+                    del markers[_at]
+                    _at -= 1
+                    if _at > 0 and (isinstance(markers[_at], str) and markers[_at] in ("and", "or")):
+                        del markers[_at]
+                    if len(markers) == 0:
+                        req.marker = None
                     break
             # continue only if this extra should be included
             if not (extra is None or extra in extras):
                 continue
-            # delete the extra marker if present
-            if _at is not None:
-                del markers[_at]
-                _at -= 1
-                if _at > 0 and (isinstance(markers[_at], str) and markers[_at] in ("and", "or")):
-                    del markers[_at]
-                if len(markers) == 0:
-                    req.marker = None
             result.append(req)
         return result
 

tests/config/loader/ini/replace/conftest.py

@@ -27,6 +27,6 @@ def example(conf: str, pos_args: Optional[List[str]] = None) -> str:
         tox_ini = ToxIni(tox_ini_file)
         config = Config(tox_ini, overrides=[], root=tmp_path, pos_args=pos_args, work_dir=tmp_path)
         loader = config.get_env("py").loaders[0]
-        return loader.load(key="env", of_type=str, conf=config, env_name="a")
+        return loader.load(key="env", of_type=str, conf=config, env_name="a", chain=[])
 
     return example  # noqa

tests/config/loader/ini/replace/test_replace_os_sep.py

@@ -0,0 +1,8 @@
+import os
+
+from tests.config.loader.ini.replace.conftest import ReplaceOne
+
+
+def test_replace_os_sep(replace_one: ReplaceOne) -> None:
+    result = replace_one("{/}")
+    assert result == os.sep

tests/config/loader/ini/replace/test_replace_tox_env.py

@@ -119,6 +119,15 @@ def test_replace_from_tox_section_missing_section(tox_ini_conf: ToxIniCreator) -
     assert conf_a["x"] == "{[magic]a}"
 
 
+def test_replace_circular(tox_ini_conf: ToxIniCreator) -> None:
+    conf_a = tox_ini_conf("[testenv:a]\nx = {y}\ny = {x}").get_env("a")
+    conf_a.add_config(keys="x", of_type=str, default="o", desc="o")
+    conf_a.add_config(keys="y", of_type=str, default="n", desc="n")
+    with pytest.raises(HandledError) as exc:
+        assert conf_a["x"]
+    assert "circular chain detected x, y" in str(exc.value)
+
+
 def test_replace_from_tox_section_missing_value(tox_ini_conf: ToxIniCreator) -> None:
     conf_a = tox_ini_conf("[testenv:e]\nx = {[m]a}\n[m]").get_env("e")
     conf_a.add_config(keys="x", of_type=str, default="o", desc="d")

tests/config/loader/ini/test_ini_loader.py

@@ -29,12 +29,12 @@ def test_ini_loader_has_no_section(mk_ini_conf: Callable[[str], ConfigParser]) -
 
 def test_ini_loader_raw(mk_ini_conf: Callable[[str], ConfigParser]) -> None:
     loader = IniLoader("tox", mk_ini_conf("[tox]\na=b"), [])
-    result = loader.load(key="a", of_type=str, conf=None, env_name=None)
+    result = loader.load(key="a", of_type=str, conf=None, env_name=None, chain=[])
     assert result == "b"
 
 
 @pytest.mark.parametrize("sep", ["\n", "\r\n"])
 def test_ini_loader_raw_strip_escaped_newline(mk_ini_conf: Callable[[str], ConfigParser], sep: str) -> None:
     loader = IniLoader("tox", mk_ini_conf(f"[tox]{sep}a=b\\{sep} c"), [])
-    result = loader.load(key="a", of_type=str, conf=None, env_name=None)
+    result = loader.load(key="a", of_type=str, conf=None, env_name=None, chain=[])
     assert result == "bc"

tests/config/loader/test_memory_loader.py

@@ -16,7 +16,7 @@ def test_memory_loader_repr() -> None:
 def test_memory_loader_override() -> None:
     loader = MemoryLoader(a=1)
     loader.overrides["a"] = Override("a=2")
-    loaded = loader.load("a", of_type=int, conf=None, env_name=None)
+    loaded = loader.load("a", of_type=int, conf=None, env_name=None, chain=[])
     assert loaded == 2
 
 
@@ -38,7 +38,7 @@ def test_memory_loader_override() -> None:
 )
 def test_memory_loader(value: Any, of_type: Type[Any]) -> None:
     loader = MemoryLoader(**{"a": value})
-    loaded = loader.load("a", of_type=of_type, conf=None, env_name=None)  # noqa
+    loaded = loader.load("a", of_type=of_type, conf=None, env_name=None, chain=[])  # noqa
     assert loaded == value