Add tests for the package dependency marker filters

Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>

6 files changed, 202 insertions, 111 deletions

diff --git a/src/tox/tox_env/package.py b/src/tox/tox_env/package.py
index 86e822e6..a282e24f 100644
--- a/src/tox/tox_env/package.py
+++ b/src/tox/tox_env/package.py
@@ -4,7 +4,7 @@ A tox environment that can build packages.
 from abc import ABC, abstractmethod
 from argparse import ArgumentParser
 from pathlib import Path
-from typing import TYPE_CHECKING, List, Optional, Set
+from typing import TYPE_CHECKING, List, Set
 
 from packaging.requirements import Requirement
 
@@ -30,7 +30,7 @@ class PackageToxEnv(ToxEnv, ABC):
         self.ref_count = AtomicCounter()
 
     @abstractmethod
-    def get_package_dependencies(self, extras: Optional[Set[str]] = None) -> List[Requirement]:
+    def get_package_dependencies(self, extras: Set[str]) -> List[Requirement]:
         raise NotImplementedError
 
     @abstractmethod
diff --git a/src/tox/tox_env/python/virtual_env/package/api.py b/src/tox/tox_env/python/virtual_env/package/api.py
index 377dd176..03ea06e8 100644
--- a/src/tox/tox_env/python/virtual_env/package/api.py
+++ b/src/tox/tox_env/python/virtual_env/package/api.py
@@ -136,40 +136,46 @@ class Pep517VirtualEnvPackage(VirtualEnv, PythonPackage, Frontend, ABC):
                 self._package = self._build_artifact()
         return [self._package]
 
-    def get_package_dependencies(self, extras: Optional[Set[str]] = None) -> List[Requirement]:
+    def get_package_dependencies(self, extras: Set[str]) -> List[Requirement]:
         with self._lock:
-            if self._package_dependencies is None:
-                self._package_dependencies = self._load_package_dependencies(extras)
+            if self._package_dependencies is None:  # pragma: no branch
+                self._ensure_meta_present()
+                self._package_dependencies = self.discover_package_dependencies(self._distribution_meta, extras)
         return self._package_dependencies
 
-    def _load_package_dependencies(self, extras: Optional[Set[str]]) -> List[Requirement]:
-        self._ensure_meta_present()
-        if extras is None:
-            extras = set()
+    @staticmethod
+    def discover_package_dependencies(  # type: ignore[no-any-unimported]
+        meta: PathDistribution, extras: Set[str]
+    ) -> List[Requirement]:
         result: List[Requirement] = []
-        if self._distribution_meta is None:
-            raise RuntimeError
-        requires = self._distribution_meta.requires or []
-        for v in requires:
-            req = Requirement(v)
+        requires = meta.requires or []
+        for req_str in requires:
+            req = Requirement(req_str)
             markers: List[Union[str, Tuple[Variable, Variable, Variable]]] = getattr(req.marker, "_markers", []) or []
-            extra: Optional[str] = None
+
+            # find the extra marker (if has)
             _at: Optional[int] = None
-            for _at, (m_key, op, m_val) in (
-                (j, i) for j, i in enumerate(markers) if isinstance(i, tuple) and len(i) == 3
+            extra: Optional[str] = None
+            for _at, (marker_key, op, marker_value) in (
+                (_at_marker, marker)
+                for _at_marker, marker in enumerate(markers)
+                if isinstance(marker, tuple) and len(marker) == 3
             ):
-                if m_key.value == "extra" and op.value == "==":
-                    extra = m_val.value
+                if marker_key.value == "extra" and op.value == "==":  # pragma: no branch
+                    extra = marker_value.value
                     break
-            if extra is None or extra in extras:
-                if _at is not None:
+            # 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]
-                    _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)
+                if len(markers) == 0:
+                    req.marker = None
+            result.append(req)
         return result
 
     @property
@@ -211,7 +217,7 @@ class Pep517VirtualEnvPackage(VirtualEnv, PythonPackage, Frontend, ABC):
                 self._backend_executor.close()
 
     @contextmanager
-    def _send_msg(self, cmd: str, result_file: Path, msg: str) -> Iterator[CmdStatus]:
+    def _send_msg(self, cmd: str, result_file: Path, msg: str) -> Iterator[ToxCmdStatus]:  # type: ignore[override]
         with self.execute_async(
             cmd=self.backend_cmd,
             cwd=self._root,
diff --git a/src/tox/util/pep517/via_fresh_subprocess.py b/src/tox/util/pep517/via_fresh_subprocess.py
new file mode 100644
index 00000000..1f83a557
--- /dev/null
+++ b/src/tox/util/pep517/via_fresh_subprocess.py
@@ -0,0 +1,68 @@
+import os
+import sys
+from contextlib import contextmanager
+from pathlib import Path
+from subprocess import PIPE, Popen
+from threading import Thread
+from typing import IO, Any, Iterator, Optional, Tuple, cast
+
+from packaging.requirements import Requirement
+
+from .frontend import CmdStatus, Frontend
+
+
+class SubprocessCmdStatus(CmdStatus, Thread):
+    def __init__(self, process: "Popen[str]") -> None:
+        super().__init__()
+        self.process = process
+        self._out_err: Optional[Tuple[str, str]] = None
+        self.start()
+
+    def run(self) -> None:
+        self._out_err = self.process.communicate()
+
+    @property
+    def done(self) -> bool:
+        return self.process.returncode is not None
+
+    def out_err(self) -> Tuple[str, str]:
+        return cast(Tuple[str, str], self._out_err)
+
+
+class SubprocessFrontend(Frontend):
+    def __init__(
+        self,
+        root: Path,
+        backend_paths: Tuple[Path, ...],
+        backend_module: str,
+        backend_obj: Optional[str],
+        requires: Tuple[Requirement, ...],
+    ):
+        super().__init__(root, backend_paths, backend_module, backend_obj, requires, reuse_backend=False)
+
+    @contextmanager
+    def _send_msg(  # type: ignore[override]
+        self, cmd: str, result_file: Path, msg: str
+    ) -> Iterator[SubprocessCmdStatus]:
+        env = os.environ.copy()
+        env["PYTHONPATH"] = os.pathsep.join(str(i) for i in self._backend_paths)
+        process = Popen(
+            args=[sys.executable] + self.backend_args,
+            stdout=PIPE,
+            stderr=PIPE,
+            stdin=PIPE,
+            universal_newlines=True,
+            cwd=self._root,
+            env=env,
+        )
+        cast(IO[str], process.stdin).write(f"{os.linesep}{msg}{os.linesep}")
+        yield SubprocessCmdStatus(process)
+
+    def send_cmd(self, cmd: str, **kwargs: Any) -> Tuple[Any, str, str]:
+        return self._send(cmd, **kwargs)
+
+
+__all__ = (
+    "SubprocessCmdStatus",
+    "SubprocessFrontend",
+)
diff --git a/tests/tox_env/python/virtual_env/test_package.py b/tests/tox_env/python/virtual_env/test_package.py
new file mode 100644
index 00000000..2b6e3d20
--- /dev/null
+++ b/tests/tox_env/python/virtual_env/test_package.py
@@ -0,0 +1,97 @@
+import sys
+from itertools import zip_longest
+from textwrap import dedent
+
+import pytest
+from packaging.requirements import Requirement
+
+from tox.pytest import TempPathFactory, ToxProjectCreator
+from tox.tox_env.python.virtual_env.package.api import PackageType, Pep517VirtualEnvPackage
+from tox.util.pep517.via_fresh_subprocess import SubprocessFrontend
+
+if sys.version_info >= (3, 8):  # pragma: no cover (py38+)
+    from importlib.metadata import Distribution, PathDistribution  # type: ignore[attr-defined]
+else:  # pragma: no cover (<py38)
+    from importlib_metadata import Distribution, PathDistribution  # noqa
+
+
+@pytest.mark.parametrize(
+    "pkg_type, of_type",
+    [
+        ("dev", "virtualenv-legacy-dev"),
+        ("sdist", "virtualenv-pep-517-sdist"),
+        ("wheel", "virtualenv-pep-517-wheel"),
+    ],
+)
+def test_tox_ini_package_type_valid(tox_project: ToxProjectCreator, pkg_type: str, of_type: str) -> None:
+    proj = tox_project({"tox.ini": f"[testenv]\npackage={pkg_type}"})
+    result = proj.run("c", "-k", "package_tox_env_type")
+    result.assert_success()
+    res = result.state.tox_env("py").conf["package"]
+    assert res is getattr(PackageType, pkg_type)
+    got_type = result.state.tox_env("py").conf["package_tox_env_type"]
+    assert got_type == of_type
+
+
+def test_tox_ini_package_type_invalid(tox_project: ToxProjectCreator) -> None:
+    proj = tox_project({"tox.ini": "[testenv]\npackage=bad"})
+    result = proj.run("c", "-k", "package_tox_env_type")
+    result.assert_failed()
+    assert " invalid package config type 'bad' requested, must be one of sdist, wheel, dev, skip" in result.out
+
+
+@pytest.fixture(scope="session")
+def pkg_with_extras(tmp_path_factory: TempPathFactory) -> PathDistribution:  # type: ignore[no-any-unimported]
+    py_ver = ".".join(str(i) for i in sys.version_info[0:2])
+    setup_cfg = f"""
+    [metadata]
+    name = demo
+    [options]
+    packages = find:
+    install_requires =
+        appdirs>=1.4.3
+        colorama>=0.4.3
+
+    [options.extras_require]
+    testing =
+        covdefaults>=1.2; python_version == '2.7' or python_version == '{py_ver}'
+        pytest>=5.4.1; python_version == '{py_ver}'
+    docs =
+        sphinx>=3
+        sphinx-rtd-theme>=0.4.3,<1
+    format =
+        black>=3
+        flake8
+    """
+    tmp_path = tmp_path_factory.mktemp("prj")
+    (tmp_path / "setup.cfg").write_text(dedent(setup_cfg))
+    (tmp_path / "setup.py").write_text("from setuptools import setup; setup()")
+    toml = '[build-system]\nrequires=["setuptools", "wheel"]\nbuild-backend = "setuptools.build_meta"'
+    (tmp_path / "pyproject.toml").write_text(toml)
+    frontend = SubprocessFrontend(*SubprocessFrontend.create_args_from_folder(tmp_path)[:-1])
+    meta = tmp_path / "meta"
+    result = frontend.prepare_metadata_for_build_wheel(meta)
+    return Distribution.at(result.metadata)
+
+
+def test_load_dependency_no_extra(pkg_with_extras: PathDistribution) -> None:  # type: ignore[no-any-unimported]
+    result = Pep517VirtualEnvPackage.discover_package_dependencies(pkg_with_extras, set())
+    for left, right in zip_longest(result, (Requirement("appdirs>=1.4.3"), Requirement("colorama>=0.4.3"))):
+        assert isinstance(right, Requirement)
+        assert str(left) == str(right)
+
+
+def test_load_dependency_many_extra(pkg_with_extras: PathDistribution) -> None:  # type: ignore[no-any-unimported]
+    py_ver = ".".join(str(i) for i in sys.version_info[0:2])
+    result = Pep517VirtualEnvPackage.discover_package_dependencies(pkg_with_extras, {"docs", "testing"})
+    exp = [
+        Requirement("appdirs>=1.4.3"),
+        Requirement("colorama>=0.4.3"),
+        Requirement("sphinx>=3"),
+        Requirement("sphinx-rtd-theme<1,>=0.4.3"),
+        Requirement(f'covdefaults>=1.2; python_version == "2.7" or python_version == "{py_ver}"'),
+        Requirement(f'pytest>=5.4.1; python_version == "{py_ver}"'),
+    ]
+    for left, right in zip_longest(result, exp):
+        assert isinstance(right, Requirement)
+        assert str(left) == str(right)
diff --git a/tests/tox_env/python/virtual_env/test_package_types.py b/tests/tox_env/python/virtual_env/test_package_types.py
deleted file mode 100644
index 85fe035e..00000000
--- a/tests/tox_env/python/virtual_env/test_package_types.py
+++ /dev/null
@@ -1,29 +0,0 @@
-import pytest
-
-from tox.pytest import ToxProjectCreator
-from tox.tox_env.python.virtual_env.package.api import PackageType
-
-
-@pytest.mark.parametrize(
-    "pkg_type, of_type",
-    [
-        ("dev", "virtualenv-legacy-dev"),
-        ("sdist", "virtualenv-pep-517-sdist"),
-        ("wheel", "virtualenv-pep-517-wheel"),
-    ],
-)
-def test_tox_ini_package_type_valid(tox_project: ToxProjectCreator, pkg_type: str, of_type: str) -> None:
-    proj = tox_project({"tox.ini": f"[testenv]\npackage={pkg_type}"})
-    result = proj.run("c", "-k", "package_tox_env_type")
-    result.assert_success()
-    res = result.state.tox_env("py").conf["package"]
-    assert res is getattr(PackageType, pkg_type)
-    got_type = result.state.tox_env("py").conf["package_tox_env_type"]
-    assert got_type == of_type
-
-
-def test_tox_ini_package_type_invalid(tox_project: ToxProjectCreator) -> None:
-    proj = tox_project({"tox.ini": "[testenv]\npackage=bad"})
-    result = proj.run("c", "-k", "package_tox_env_type")
-    result.assert_failed()
-    assert " invalid package config type 'bad' requested, must be one of sdist, wheel, dev, skip" in result.out
diff --git a/tests/util/test_pep517.py b/tests/util/test_pep517.py
index 8b520f0e..a060a5ad 100644
--- a/tests/util/test_pep517.py
+++ b/tests/util/test_pep517.py
@@ -1,19 +1,17 @@
-import os
 import sys
 from contextlib import contextmanager
 from pathlib import Path
 from stat import S_IWGRP, S_IWOTH, S_IWUSR
-from subprocess import PIPE, Popen
 from textwrap import dedent
-from threading import Thread
-from typing import IO, Any, Callable, Iterator, NamedTuple, Optional, Tuple, cast
+from typing import Callable, Iterator, NamedTuple
 
 import pytest
 from packaging.requirements import Requirement
 from pytest_mock import MockerFixture
 
 from tox.pytest import TempPathFactory
-from tox.util.pep517.frontend import BackendFailed, CmdStatus, Frontend
+from tox.util.pep517.frontend import BackendFailed
+from tox.util.pep517.via_fresh_subprocess import SubprocessFrontend
 
 if sys.version_info >= (3, 8):  # pragma: no cover (py38+)
     from importlib.metadata import Distribution, EntryPoint  # type: ignore[attr-defined]
@@ -21,55 +19,6 @@ else:  # pragma: no cover (<py38)
     from importlib_metadata import Distribution, EntryPoint  # noqa
 
 
-class SubprocessCmdStatus(CmdStatus, Thread):
-    def __init__(self, process: "Popen[str]") -> None:
-        super().__init__()
-        self.process = process
-        self._out_err: Optional[Tuple[str, str]] = None
-        self.start()
-
-    def run(self) -> None:
-        self._out_err = self.process.communicate()
-
-    @property
-    def done(self) -> bool:
-        return self.process.returncode is not None
-
-    def out_err(self) -> Tuple[str, str]:
-        return cast(Tuple[str, str], self._out_err)
-
-
-class SubprocessFrontend(Frontend):
-    def __init__(
-        self,
-        root: Path,
-        backend_paths: Tuple[Path, ...],
-        backend_module: str,
-        backend_obj: Optional[str],
-        requires: Tuple[Requirement, ...],
-    ):
-        super().__init__(root, backend_paths, backend_module, backend_obj, requires, reuse_backend=False)
-
-    @contextmanager
-    def _send_msg(self, cmd: str, result_file: Path, msg: str) -> Iterator[CmdStatus]:
-        env = os.environ.copy()
-        env["PYTHONPATH"] = os.pathsep.join(str(i) for i in self._backend_paths)
-        process = Popen(
-            args=[sys.executable] + self.backend_args,
-            stdout=PIPE,
-            stderr=PIPE,
-            stdin=PIPE,
-            universal_newlines=True,
-            cwd=self._root,
-            env=env,
-        )
-        cast(IO[str], process.stdin).write(f"{os.linesep}{msg}{os.linesep}")
-        yield SubprocessCmdStatus(process)
-
-    def send_cmd(self, cmd: str, **kwargs: Any) -> Tuple[Any, str, str]:
-        return self._send(cmd, **kwargs)
-
-
 @pytest.fixture(scope="session")
 def frontend_setuptools(tmp_path_factory: TempPathFactory) -> SubprocessFrontend:
     prj = tmp_path_factory.mktemp("proj")