From f5850c0d3a893cbb6d9de192fe8e6e857573499b Mon Sep 17 00:00:00 2001
From: Arik Mitschang <arik.mitschang@gmail.com>
Date: Wed, 3 Apr 2024 17:58:46 -0400
Subject: [PATCH] Support multiple override appends (#3261)

Currently only the last override supplied is considered, in conflict with now
supported append overrides ("+="). This now enables appending multiple times via
the command line.
---
 docs/changelog/3261.feature.rst            |  1 +
 docs/config.rst                            | 15 +++++-
 src/tox/config/loader/api.py               | 50 ++++++++++---------
 tests/config/loader/ini/test_ini_loader.py |  2 +-
 tests/config/loader/test_loader.py         |  6 +++
 tests/config/loader/test_memory_loader.py  |  2 +-
 tests/config/test_main.py                  | 57 ++++++++++++++++++++++
 7 files changed, 108 insertions(+), 25 deletions(-)
 create mode 100644 docs/changelog/3261.feature.rst

diff --git a/docs/changelog/3261.feature.rst b/docs/changelog/3261.feature.rst
new file mode 100644
index 000000000..f07f3d74b
--- /dev/null
+++ b/docs/changelog/3261.feature.rst
@@ -0,0 +1 @@
+Add support for multiple appending override options (-x, --override) on command line - by :user:`amitschang`.
diff --git a/docs/config.rst b/docs/config.rst
index 840924e58..0d40027d6 100644
--- a/docs/config.rst
+++ b/docs/config.rst
@@ -1037,7 +1037,7 @@ You could add additional dependencies by running:
 
 .. code-block:: bash
 
-   tox --override testenv.deps+=pytest-xdist,pytest-cov
+   tox --override testenv.deps+=pytest-xdist
 
 You could set additional environment variables by running:
 
@@ -1045,6 +1045,19 @@ You could set additional environment variables by running:
 
    tox --override testenv.setenv+=baz=quux
 
+You can specify overrides multiple times on the command line to append multiple items:
+
+.. code-block:: bash
+
+   tox -x testenv.seteenv+=foo=bar -x testenv.setenv+=baz=quux
+   tox -x testenv.deps+=pytest-xdist -x testenv.deps+=pytest-cov
+
+Or reset override and append to that (note the first override is ``=`` and not ``+=``):
+
+.. code-block:: bash
+
+   tox -x testenv.deps=pytest-xdist -x testenv.deps+=pytest-cov
+
 Set CLI flags via environment variables
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 All CLI flags can be set via environment variables too, the naming convention here is ``TOX_<option>``. E.g.
diff --git a/src/tox/config/loader/api.py b/src/tox/config/loader/api.py
index 98a235ed6..a7f35d8f4 100644
--- a/src/tox/config/loader/api.py
+++ b/src/tox/config/loader/api.py
@@ -81,7 +81,9 @@ class Loader(Convert[T]):
 
     def __init__(self, section: Section, overrides: list[Override]) -> None:
         self._section = section
-        self.overrides: dict[str, Override] = {o.key: o for o in overrides}
+        self.overrides: dict[str, list[Override]] = {}
+        for override in overrides:
+            self.overrides.setdefault(override.key, []).append(override)
         self.parent: Loader[Any] | None = None
 
     @property
@@ -130,31 +132,35 @@ def load(  # noqa: PLR0913
         """
         from tox.config.set_env import SetEnv  # noqa: PLC0415
 
-        override = self.overrides.get(key)
-        if override:
-            converted_override = _STR_CONVERT.to(override.value, of_type, factory)
-            if not override.append:
-                return converted_override
+        overrides = self.overrides.get(key, [])
+
         try:
             raw = self.load_raw(key, conf, args.env_name)
         except KeyError:
-            if override:
-                return converted_override
-            raise
-        converted = self.build(key, of_type, factory, conf, raw, args)
-        if override and override.append:
-            if isinstance(converted, list) and isinstance(converted_override, list):
-                converted += converted_override
-            elif isinstance(converted, dict) and isinstance(converted_override, dict):
-                converted.update(converted_override)
-            elif isinstance(converted, SetEnv) and isinstance(converted_override, SetEnv):
-                converted.update(converted_override, override=True)
-            elif isinstance(converted, PythonDeps) and isinstance(converted_override, PythonDeps):
-                converted += converted_override
+            converted = None
+            if not overrides:
+                raise
+        else:
+            converted = self.build(key, of_type, factory, conf, raw, args)
+
+        for override in overrides:
+            converted_override = _STR_CONVERT.to(override.value, of_type, factory)
+            if override.append and converted is not None:
+                if isinstance(converted, list) and isinstance(converted_override, list):
+                    converted += converted_override
+                elif isinstance(converted, dict) and isinstance(converted_override, dict):
+                    converted.update(converted_override)
+                elif isinstance(converted, SetEnv) and isinstance(converted_override, SetEnv):
+                    converted.update(converted_override, override=True)
+                elif isinstance(converted, PythonDeps) and isinstance(converted_override, PythonDeps):
+                    converted += converted_override
+                else:
+                    msg = "Only able to append to lists and dicts"
+                    raise ValueError(msg)
             else:
-                msg = "Only able to append to lists and dicts"
-                raise ValueError(msg)
-        return converted
+                converted = converted_override
+
+        return converted  # type: ignore[return-value]
 
     def build(  # noqa: PLR0913
         self,
diff --git a/tests/config/loader/ini/test_ini_loader.py b/tests/config/loader/ini/test_ini_loader.py
index 4268cfba8..f05f85041 100644
--- a/tests/config/loader/ini/test_ini_loader.py
+++ b/tests/config/loader/ini/test_ini_loader.py
@@ -21,7 +21,7 @@ def test_ini_loader_keys(mk_ini_conf: Callable[[str], ConfigParser]) -> None:
 def test_ini_loader_repr(mk_ini_conf: Callable[[str], ConfigParser]) -> None:
     core = IniSection(None, "tox")
     loader = IniLoader(core, mk_ini_conf("\n[tox]\n\na=b\nc=d\n\n"), [Override("tox.a=1")], core_section=core)
-    assert repr(loader) == "IniLoader(section=tox, overrides={'a': Override('tox.a=1')})"
+    assert repr(loader) == "IniLoader(section=tox, overrides={'a': [Override('tox.a=1')]})"
 
 
 def test_ini_loader_has_section(mk_ini_conf: Callable[[str], ConfigParser]) -> None:
diff --git a/tests/config/loader/test_loader.py b/tests/config/loader/test_loader.py
index b00a5a24c..ab8c7d996 100644
--- a/tests/config/loader/test_loader.py
+++ b/tests/config/loader/test_loader.py
@@ -42,6 +42,12 @@ def test_override_append(flag: str) -> None:
     assert value.append is True
 
 
+@pytest.mark.parametrize("flag", ["-x", "--override"])
+def test_override_multiple(flag: str) -> None:
+    parsed, _, __, ___, ____ = get_options(flag, "magic+=1", flag, "magic+=2")
+    assert len(parsed.override) == 2
+
+
 def test_override_equals() -> None:
     assert Override("a=b") == Override("a=b")
 
diff --git a/tests/config/loader/test_memory_loader.py b/tests/config/loader/test_memory_loader.py
index 8ab7f8f62..508566df1 100644
--- a/tests/config/loader/test_memory_loader.py
+++ b/tests/config/loader/test_memory_loader.py
@@ -18,7 +18,7 @@ def test_memory_loader_repr() -> None:
 
 def test_memory_loader_override() -> None:
     loader = MemoryLoader(a=1)
-    loader.overrides["a"] = Override("a=2")
+    loader.overrides["a"] = [Override("a=2")]
     args = ConfigLoadArgs([], "name", None)
     loaded = loader.load("a", of_type=int, conf=None, factory=None, args=args)
     assert loaded == 2
diff --git a/tests/config/test_main.py b/tests/config/test_main.py
index 264e2e1df..853ef9bb4 100644
--- a/tests/config/test_main.py
+++ b/tests/config/test_main.py
@@ -78,6 +78,17 @@ def test_config_override_appends_to_list(tox_ini_conf: ToxIniCreator) -> None:
     assert conf["passenv"] == ["foo", "bar"]
 
 
+def test_config_override_sequence(tox_ini_conf: ToxIniCreator) -> None:
+    example = """
+    [testenv]
+    passenv = foo
+    """
+    overrides = [Override("testenv.passenv=bar"), Override("testenv.passenv+=baz")]
+    conf = tox_ini_conf(example, override=overrides).get_env("testenv")
+    conf.add_config("passenv", of_type=List[str], default=[], desc="desc")
+    assert conf["passenv"] == ["bar", "baz"]
+
+
 def test_config_override_appends_to_empty_list(tox_ini_conf: ToxIniCreator) -> None:
     conf = tox_ini_conf("[testenv]", override=[Override("testenv.passenv+=bar")]).get_env("testenv")
     conf.add_config("passenv", of_type=List[str], default=[], desc="desc")
@@ -95,6 +106,35 @@ def test_config_override_appends_to_setenv(tox_ini_conf: ToxIniCreator) -> None:
     assert conf["setenv"].load("baz") == "quux"
 
 
+def test_config_override_appends_to_setenv_multiple(tox_ini_conf: ToxIniCreator) -> None:
+    example = """
+    [testenv]
+    setenv =
+      foo = bar
+    """
+    overrides = [Override("testenv.setenv+=baz=quux"), Override("testenv.setenv+=less=more")]
+    conf = tox_ini_conf(example, override=overrides).get_env("testenv")
+    assert conf["setenv"].load("foo") == "bar"
+    assert conf["setenv"].load("baz") == "quux"
+    assert conf["setenv"].load("less") == "more"
+
+
+def test_config_override_sequential_processing(tox_ini_conf: ToxIniCreator) -> None:
+    example = """
+    [testenv]
+    setenv =
+      foo = bar
+    """
+    overrides = [Override("testenv.setenv+=a=b"), Override("testenv.setenv=c=d"), Override("testenv.setenv+=e=f")]
+    conf = tox_ini_conf(example, override=overrides).get_env("testenv")
+    with pytest.raises(KeyError):
+        assert conf["setenv"].load("foo") == "bar"
+    with pytest.raises(KeyError):
+        assert conf["setenv"].load("a") == "b"
+    assert conf["setenv"].load("c") == "d"
+    assert conf["setenv"].load("e") == "f"
+
+
 def test_config_override_appends_to_empty_setenv(tox_ini_conf: ToxIniCreator) -> None:
     conf = tox_ini_conf("[testenv]", override=[Override("testenv.setenv+=foo=bar")]).get_env("testenv")
     assert conf["setenv"].load("foo") == "bar"
@@ -116,6 +156,23 @@ def test_config_override_appends_to_pythondeps(tox_ini_conf: ToxIniCreator, tmp_
     assert conf["deps"].lines() == ["foo", "bar"]
 
 
+def test_config_multiple_overrides(tox_ini_conf: ToxIniCreator, tmp_path: Path) -> None:
+    example = """
+    [testenv]
+    deps = foo
+    """
+    overrides = [Override("testenv.deps+=bar"), Override("testenv.deps+=baz")]
+    conf = tox_ini_conf(example, override=overrides).get_env("testenv")
+    conf.add_config(
+        "deps",
+        of_type=PythonDeps,
+        factory=partial(PythonDeps.factory, tmp_path),
+        default=PythonDeps("", root=tmp_path),
+        desc="desc",
+    )
+    assert conf["deps"].lines() == ["foo", "bar", "baz"]
+
+
 def test_config_override_appends_to_empty_pythondeps(tox_ini_conf: ToxIniCreator, tmp_path: Path) -> None:
     example = """
     [testenv]