conftest: Partially isolate the default LocalPackageResolver

LocalPackageResolver without a given pyenv_path defaults to looking up
packages in sys.path. This is problematic in tests, as we do not fully
control the Python environment in which the tests are run (e.g. consider
a developer running pytest directly in their dev environment where they
happen to have installed some packages that we don't expect).

Until now, we've skirted around the problem by adding a couple of TODO
comments near the relevant tests, but it's now time to solve this in a
better way. However, short of running the tests in a subprocess, we
cannot _fully_ control the sys.path. What we can do, is to use a
combination of our fake_venv fixture and pytest's
monkeypatch.syspath_prepend() function to insert an environment that we
_do_ control at the start of sys.path. This will allow us to place
entries into that fake_venv that we can then expect to be resolved
exactly as we expect in tests.

We introduce a new isolate_default_resolver fixture to apply this
partial isolation for a given dict of package-to-imports mappings.
At the same time we add a default_sys_path_env_for_tests dict that
encodes what we previously considered the minimal/default package
mappings that we expected (but could not guarantee) to be present in
the test environment.

With this, we can now call

  isolate_default_resolver(default_sys_path_env_for_tests)

from tests that previously relied on the surrounding test environment,
and their expectation is now guarateed by this new fake_venv inserted at
the start of sys.path.

Note that for tests that rely on packages NOT being installed into the
test environment (e.g. testing the fallback to the identity mapping), we
must still take care to use package names that we can ~guarantee never
to be present in the environment in which the tests are run. Thus, we
are only _partially_ isolated from the surrounding test environment.

Author: jherland

tests/conftest.py

@@ -4,7 +4,7 @@
 from pathlib import Path
 from tempfile import mkdtemp
 from textwrap import dedent
-from typing import Dict, Iterable, Set, Tuple, Union
+from typing import Callable, Dict, Iterable, Set, Tuple, Union
 
 import pytest
 
@@ -52,6 +52,40 @@ def create_one_fake_venv(fake_packages: Dict[str, Set[str]]) -> Tuple[Path, Path
     return create_one_fake_venv
 
 
+@pytest.fixture
+def isolate_default_resolver(
+    fake_venv: Callable[[Dict[str, Set[str]]], Tuple[Path, Path]], monkeypatch
+):
+    """Put a fake_venv at the start of sys.path to yield predictable Packages.
+
+    Call the returned function to place a fake venv with the specified package
+    mappings at the start of sys.path.
+
+    Rationale:
+    When testing resolve_dependencies() or anything that depends on
+    LocalPackageResolver() with default/empty pyenv, it is important to realize
+    that local packages will be resolved via sys.path. This is hard to fully
+    isolate/mock in tests, but we can do the following to approximate isolation:
+    - Use fake_venv() and pytest.monkeypatch.syspath_prepend(path) to make sure
+      packages that we expect to find in the default environment are always
+      found in this fake venv. This is achieved by using this fixture.
+    - Populate this fake_venv with package that we expect to find in the default
+      environment. These will then be resolved through the fake_venv to yield
+      predictable import names and mapping descriptions.
+    - Tests must make sure packages that they expect NOT to find in the default
+      environment are chosen/spelled in ways to ensure they are indeed never
+      found elsewhere in sys.path, as we are not able to isolate the resolver
+      from sys.path.
+    """
+
+    def inner(fake_packages: Dict[str, Set[str]]) -> Path:
+        _venv_dir, package_dir = fake_venv(fake_packages)
+        monkeypatch.syspath_prepend(package_dir)
+        return package_dir
+
+    return inner
+
+
 @pytest.fixture
 def project_with_requirements(write_tmp_files):
     return write_tmp_files(

tests/test_local_env.py

@@ -105,19 +105,18 @@ def test_local_env__default_venv__contains_pip_and_setuptools(tmp_path):
         assert import_names.issubset(p.mappings[DependenciesMapping.LOCAL_ENV])
 
 
-def test_local_env__current_venv__contains_our_test_dependencies():
+def test_local_env__current_venv__contains_prepared_packages(isolate_default_resolver):
+    isolate_default_resolver(
+        {
+            "pip": {"pip"},
+            "setuptools": {"setuptools", "pkg_resources"},
+            "isort": {"isort"},
+            "pydantic": {"pydantic"},
+            "pytest": {"pytest"},
+        }
+    )
     lpl = LocalPackageResolver()
-    expect_package_names = [
-        # Present in ~all venvs:
-        "pip",
-        "setuptools",
-        # FawltyDeps main deps
-        "isort",
-        "pydantic",
-        # Test dependencies
-        "hypothesis",
-        "pytest",
-    ]
+    expect_package_names = ["pip", "setuptools", "isort", "pydantic", "pytest"]
     for package_name in expect_package_names:
         assert package_name in lpl.packages
 

tests/test_packages.py

@@ -15,7 +15,7 @@
 )
 from fawltydeps.types import UnparseablePathException
 
-from .utils import SAMPLE_PROJECTS_DIR, test_vectors
+from .utils import SAMPLE_PROJECTS_DIR, default_sys_path_env_for_tests, test_vectors
 
 
 def test_package__empty_package__matches_nothing():
@@ -229,14 +229,6 @@ def test_user_defined_mapping__no_input__returns_empty_mapping():
     assert len(udm.packages) == 0
 
 
-# TODO: These tests are not fully isolated, i.e. they do not control the
-# virtualenv in which they run. For now, we assume that we are running in an
-# environment where at least these packages are available:
-# - setuptools (exposes multiple import names, including pkg_resources)
-# - pip (exposes a single import name: pip)
-# - isort (exposes no top_level.txt, but 'isort' import name can be inferred)
-
-
 @pytest.mark.parametrize(
     "dep_name,expect_import_names",
     [
@@ -272,7 +264,10 @@ def test_user_defined_mapping__no_input__returns_empty_mapping():
         ),
     ],
 )
-def test_LocalPackageResolver_lookup_packages(dep_name, expect_import_names):
+def test_LocalPackageResolver_lookup_packages(
+    isolate_default_resolver, dep_name, expect_import_names
+):
+    isolate_default_resolver(default_sys_path_env_for_tests)
     lpl = LocalPackageResolver()
     actual = lpl.lookup_packages({dep_name})
     if expect_import_names is None:
@@ -283,13 +278,17 @@ def test_LocalPackageResolver_lookup_packages(dep_name, expect_import_names):
 
 
 @pytest.mark.parametrize("vector", [pytest.param(v, id=v.id) for v in test_vectors])
-def test_resolve_dependencies(vector):
+def test_resolve_dependencies(vector, isolate_default_resolver):
     dep_names = [dd.name for dd in vector.declared_deps]
+    isolate_default_resolver(default_sys_path_env_for_tests)
     assert resolve_dependencies(dep_names) == vector.expect_resolved_deps
 
 
-def test_resolve_dependencies__informs_once_when_id_mapping_is_used(caplog):
+def test_resolve_dependencies__informs_once_when_id_mapping_is_used(
+    caplog, isolate_default_resolver
+):
     dep_names = ["some-foo", "pip", "some-foo"]
+    isolate_default_resolver(default_sys_path_env_for_tests)
     expect = {
         "pip": Package("pip", {DependenciesMapping.LOCAL_ENV: {"pip"}}),
         "some-foo": Package("some-foo", {DependenciesMapping.IDENTITY: {"some_foo"}}),

tests/test_resolver.py

@@ -1,7 +1,7 @@
 """Verify behavior of packages resolver"""
 
 from pathlib import Path
-from typing import Dict, List, Optional
+from typing import Dict, List, Optional, Set
 
 from hypothesis import HealthCheck, given, settings
 from hypothesis import strategies as st
@@ -13,22 +13,15 @@
     resolve_dependencies,
 )
 
+from .utils import default_sys_path_env_for_tests
+
 # The deps in each category should be disjoint
 other_deps = ["pandas", "numpy", "other"]
-locally_installed_deps = {
-    "setuptools": [
-        "_distutils_hack",
-        "pkg_resources",
-        "setuptools",
-    ],
-    "pip": ["pip"],
-    "isort": ["isort"],
-}
 user_defined_mapping = {"apache-airflow": ["airflow", "foo", "bar"]}
 
 
 @st.composite
-def dict_subset_strategy(draw, input_dict: Dict[str, List[str]]):
+def dict_subset_strategy(draw, input_dict: Dict[str, Set[str]]):
     """Returns a hypothesis strategy to choose items from a dict."""
     if not input_dict:
         return {}
@@ -89,7 +82,7 @@ def generate_expected_resolved_deps(
         ret.update(
             {
                 dep: Package(
-                    dep,
+                    Package.normalize_name(dep),
                     {DependenciesMapping.LOCAL_ENV: set(imports)},
                 )
                 for dep, imports in locally_installed_deps.items()
@@ -118,14 +111,15 @@ def generate_expected_resolved_deps(
 # The test function only reads the file content and filters needed input.
 @given(
     user_mapping=user_mapping_strategy(user_defined_mapping),
-    installed_deps=dict_subset_strategy(locally_installed_deps),
+    installed_deps=dict_subset_strategy(default_sys_path_env_for_tests),
     other_deps=st.lists(st.sampled_from(other_deps), unique=True),
 )
 @settings(suppress_health_check=[HealthCheck.function_scoped_fixture])
 def test_resolve_dependencies__generates_expected_mappings(
     installed_deps,
     other_deps,
     user_mapping,
+    isolate_default_resolver,
     tmp_path,
 ):
 
@@ -163,6 +157,7 @@ def test_resolve_dependencies__generates_expected_mappings(
         other_deps=other_deps,
     )
 
+    isolate_default_resolver(installed_deps)
     obtained = resolve_dependencies(
         dep_names,
         custom_mapping_files=set([custom_mapping_file])

tests/utils.py

@@ -8,12 +8,7 @@
 from typing import Any, Dict, Iterable, List, Optional, Tuple
 
 from fawltydeps.main import main
-from fawltydeps.packages import (
-    DependenciesMapping,
-    IdentityMapping,
-    LocalPackageResolver,
-    Package,
-)
+from fawltydeps.packages import DependenciesMapping, Package
 from fawltydeps.types import (
     DeclaredDependency,
     Location,
@@ -35,16 +30,6 @@ def collect_dep_names(deps: Iterable[DeclaredDependency]) -> Iterable[str]:
     return (dep.name for dep in deps)
 
 
-# TODO: These tests are not fully isolated, i.e. they do not control the
-# virtualenv in which they run. For now, we assume that we are running in an
-# environment where at least these packages are available:
-# - setuptools (exposes multiple import names, including pkg_resources)
-# - pip (exposes a single import name: pip)
-# - isort (exposes no top_level.txt, but 'isort' import name can be inferred)
-
-local_env = LocalPackageResolver()
-
-
 def imports_factory(*imports: str) -> List[ParsedImport]:
     return [ParsedImport(imp, Location("<stdin>")) for imp in imports]
 
@@ -54,15 +39,26 @@ def deps_factory(*deps: str, path: str = "foo") -> List[DeclaredDependency]:
     return [DeclaredDependency(name=dep, source=Location(Path(path))) for dep in deps]
 
 
+# There are the packages/imports we expect to be able to resolve via the default
+# LocalPackageResolver. See the isolate_default_resolver() fixture for how we
+# make this come true.
+default_sys_path_env_for_tests = {
+    "pip": {"pip"},
+    "setuptools": {"setuptools", "pkg_resources", "_distutils_hack"},
+    "isort": {"isort"},
+    "typing-extensions": {"typing_extensions"},
+}
+
+
 def resolved_factory(*deps: str) -> Dict[str, Package]:
-    unresolved = set(deps)
-    resolved = local_env.lookup_packages(unresolved)
-    unresolved -= resolved.keys()
-    id_mapping = IdentityMapping()
-    resolved.update(id_mapping.lookup_packages(unresolved))
-    unresolved -= resolved.keys()
-    assert not unresolved
-    return resolved
+    def make_package(dep: str) -> Package:
+        imports = default_sys_path_env_for_tests.get(dep, None)
+        if imports is not None:  # exists in local env
+            return Package(dep, {DependenciesMapping.LOCAL_ENV: imports})
+        # fall back to identity mapping
+        return Package(dep, {DependenciesMapping.IDENTITY: {dep}})
+
+    return {dep: make_package(dep) for dep in deps}
 
 
 def undeclared_factory(*deps: str) -> List[UndeclaredDependency]: