From 65241d4e414aa3225e7968eeab0e26bf24643036 Mon Sep 17 00:00:00 2001 From: kyleknap Date: Sun, 28 Jan 2024 10:43:00 -0800 Subject: [PATCH] Introduce dependency test suite The plan is to build out this test suite to test against the AWS CLI's dependencies to help facilitate dependency upgrades. To start, this test suite contains the following new test cases: * Assert expected packages in runtime closure. This allows us to be alerted if a dependency introduces a new transitive depenency to the AWS CLI closure. * Assert expected unbounded dependencies in runtime closure. * Assert expected packages in build closure * Assert expected unbounded dependencies in build closure Implemenation notes: * Why it is a separate test directory * Note how to figure it out for runtime dependencies * Note how to figure out for build dependencies --- .github/workflows/run-dep-tests.yml | 27 ++ requirements-dev-lock.txt | 33 ++- requirements-dev.txt | 4 + scripts/ci/run-dep-tests | 34 +++ tests/dependencies/__init__.py | 12 + tests/dependencies/test_closure.py | 390 ++++++++++++++++++++++++++++ 6 files changed, 492 insertions(+), 8 deletions(-) create mode 100644 .github/workflows/run-dep-tests.yml create mode 100755 scripts/ci/run-dep-tests create mode 100644 tests/dependencies/__init__.py create mode 100644 tests/dependencies/test_closure.py diff --git a/.github/workflows/run-dep-tests.yml b/.github/workflows/run-dep-tests.yml new file mode 100644 index 0000000000000..b158f84151cc2 --- /dev/null +++ b/.github/workflows/run-dep-tests.yml @@ -0,0 +1,27 @@ +name: Run dependency tests + +on: + push: + pull_request: + branches-ignore: [ master ] + +jobs: + build: + + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] + os: [ubuntu-latest, macOS-latest, windows-latest] + + steps: + - uses: actions/checkout@v4 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: python scripts/ci/install + - name: Run tests + run: python scripts/ci/run-dep-tests diff --git a/requirements-dev-lock.txt b/requirements-dev-lock.txt index 05bb6ee0baa1e..a8686579aeed2 100644 --- a/requirements-dev-lock.txt +++ b/requirements-dev-lock.txt @@ -7,6 +7,10 @@ atomicwrites==1.4.1 \ --hash=sha256:81b2c9071a49367a7f770170e5eec8cb66567cfbbc8c73d20ce5ca4a8d71cf11 # via -r requirements-dev.txt +build==1.0.3 \ + --hash=sha256:538aab1b64f9828977f84bc63ae570b060a8ed1be419e7870b8b4fc5e6ea553b \ + --hash=sha256:589bf99a67df7c9cf07ec0ac0e5e2ea5d4b37ac63301c4986d1acb126aa83f8f + # via -r requirements-dev.txt colorama==0.4.4 \ --hash=sha256:5941b2b48a20143d2267e95b1c2a7603ce057ee39fd88e7329b0c292aa16869b \ --hash=sha256:9f47eda37229f68eee03b24b9748937c7dc3868f906e8ba69fbcbdd3bc5dc3e2 @@ -79,22 +83,29 @@ exceptiongroup==1.1.3 \ --hash=sha256:097acd85d473d75af5bb98e41b61ff7fe35efe6675e4f9370ec6ec5126d160e9 \ --hash=sha256:343280667a4585d195ca1cf9cef84a4e178c4b6cf2274caef9859782b567d5e3 # via pytest +importlib-metadata==7.0.1 \ + --hash=sha256:4805911c3a4ec7c3966410053e9ec6a1fecd629117df5adee56dfc9432a1081e \ + --hash=sha256:f238736bb06590ae52ac1fab06a3a9ef1d8dce2b7a35b5ab329371d6c8f5d2cc + # via build iniconfig==1.1.1 \ --hash=sha256:011e24c64b7f47f6ebd835bb12a743f2fbe9a26d4cecaa7f53bc4f35ee9da8b3 \ --hash=sha256:bc3af051d7d14b2ee5ef9969666def0cd1a000e121eaea580d4a313df4b37f32 # via pytest -packaging==21.3 \ - --hash=sha256:dd47c42927d89ab911e606518907cc2d3a1f38bbd026385970643f9c5b8ecfeb \ - --hash=sha256:ef103e05f519cdc783ae24ea4e2e0f508a9c99b2d4969652eed6a2e1ea5bd522 - # via pytest +packaging==23.2 \ + --hash=sha256:048fb0e9405036518eaaf48a55953c750c11e1a1b68e0dd1a9d62ed0c092cfc5 \ + --hash=sha256:8c491190033a9af7e1d931d0b5dacc2ef47509b34dd0de67ed209b5203fc88c7 + # via + # -r requirements-dev.txt + # build + # pytest pluggy==1.0.0 \ --hash=sha256:4224373bacce55f955a878bf9cfa763c1e360858e330072059e10bad68531159 \ --hash=sha256:74134bbf457f031a36d68416e1509f34bd5ccc019f0bcc952c7b909d06b37bd3 # via pytest -pyparsing==3.0.9 \ - --hash=sha256:2b020ecf7d21b687f219b71ecad3631f644a47f01403fa1d1036b0c6416d70fb \ - --hash=sha256:5026bae9a10eeaefb61dab2f09052b9f4307d44aee4eda64b309723d8d206bbc - # via packaging +pyproject-hooks==1.0.0 \ + --hash=sha256:283c11acd6b928d2f6a7c73fa0d01cb2bdc5f07c57a2eeb6e83d5e56b97976f8 \ + --hash=sha256:f271b298b97f5955d53fb12b72c1fb1948c22c1a6b70b315c54cedaca0264ef5 + # via build pytest==7.4.0 \ --hash=sha256:78bf16451a2eb8c7a2ea98e32dc119fd2aa758f1d5d66dbf0a59d69a3969df32 \ --hash=sha256:b4bf8c45bd59934ed84001ad51e11b4ee40d40a1229d2c79f9c592b0a3f6bd8a @@ -109,9 +120,15 @@ tomli==2.0.1 \ --hash=sha256:939de3e7a6161af0c887ef91b7d41a53e7c5a1ca976325f429cb46ea9bc30ecc \ --hash=sha256:de526c12914f0c550d15924c62d72abc48d6fe7364aa87328337a31007fe8a4f # via + # build # coverage + # pyproject-hooks # pytest wheel==0.38.1 \ --hash=sha256:7a95f9a8dc0924ef318bd55b616112c70903192f524d120acc614f59547a9e1f \ --hash=sha256:ea041edf63f4ccba53ad6e035427997b3bb10ee88a4cd014ae82aeb9eea77bb9 # via -r requirements-dev.txt +zipp==3.17.0 \ + --hash=sha256:0e923e726174922dce09c53c59ad483ff7bbb8e572e00c7f7c46b88556409f31 \ + --hash=sha256:84e64a1c28cf7e91ed2078bb8cc8c259cb19b76942096c8d7b84947690cabaf0 + # via importlib-metadata diff --git a/requirements-dev.txt b/requirements-dev.txt index 5acb4e9a3602d..4727dacf56c05 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -7,3 +7,7 @@ pytest==7.4.0 pytest-cov==4.1.0 atomicwrites>=1.0 # Windows requirement colorama>0.3.0 # Windows requirement + +# Dependency test specific deps +packaging==23.2 +build==1.0.3 diff --git a/scripts/ci/run-dep-tests b/scripts/ci/run-dep-tests new file mode 100755 index 0000000000000..4936a8a085cdd --- /dev/null +++ b/scripts/ci/run-dep-tests @@ -0,0 +1,34 @@ +#!/usr/bin/env python +# Don't run tests from the root repo dir. +# We want to ensure we're importing from the installed +# binary package not from the CWD. + +import os +from contextlib import contextmanager +from subprocess import check_call + +_dname = os.path.dirname + +REPO_ROOT = _dname(_dname(_dname(os.path.abspath(__file__)))) + + +@contextmanager +def cd(path): + """Change directory while inside context manager.""" + cwd = os.getcwd() + try: + os.chdir(path) + yield + finally: + os.chdir(cwd) + + +def run(command): + env = os.environ.copy() + env['TESTS_REMOVE_REPO_ROOT_FROM_PATH'] = 'true' + return check_call(command, shell=True, env=env) + + +if __name__ == "__main__": + with cd(os.path.join(REPO_ROOT, "tests")): + run(f"{REPO_ROOT}/scripts/ci/run-tests dependencies") diff --git a/tests/dependencies/__init__.py b/tests/dependencies/__init__.py new file mode 100644 index 0000000000000..85c792b31b96f --- /dev/null +++ b/tests/dependencies/__init__.py @@ -0,0 +1,12 @@ +# Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. diff --git a/tests/dependencies/test_closure.py b/tests/dependencies/test_closure.py new file mode 100644 index 0000000000000..6635bc992f28e --- /dev/null +++ b/tests/dependencies/test_closure.py @@ -0,0 +1,390 @@ +# Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. +import functools +import importlib.metadata +import json +import shutil +import subprocess +import sys +import venv +from pathlib import Path +from typing import Dict, Iterator, List, Literal, Optional, Tuple + +import build +import pytest +from packaging.requirements import Requirement + +IS_WINDOWS = sys.platform == "win32" +VENV_BIN_DIRNAME = "Scripts" if IS_WINDOWS else "bin" +PYTHON_EXE_NAME = "python.exe" if IS_WINDOWS else "python" + +_DEPENDENCY_CLOSURE_TYPE = Literal["runtime", "build"] +_NESTED_STR_DICT = Dict[str, "_NESTED_STR_DICT"] + + +@pytest.fixture(scope="module") +def awscli_package(tmp_path_factory): + yield Package( + name="awscli", + workdir_path=tmp_path_factory.mktemp("awscli_deps"), + project_root=Path(__file__).parents[2], + ) + + +@pytest.fixture +def package(request, awscli_package): + if request.param == "awscli": + return awscli_package + for _, package in awscli_package.runtime_dependencies.walk(): + if request.param == package.name: + return package + raise ValueError(f"Could not find package: {request.param}") + + +class Package: + def __init__( + self, + name: str, + workdir_path: Path, + project_root: Optional[Path] = None, + from_requirement: Optional[Requirement] = None, + ): + self.name = name + self._workdir_path = workdir_path + self._project_root = project_root + self._from_requirement = from_requirement + + @functools.cached_property + def runtime_dependencies(self) -> "DependencyClosure": + return self._get_closure("runtime") + + @functools.cached_property + def build_dependencies(self) -> "DependencyClosure": + return self._get_closure("build") + + def _get_closure( + self, closure_type: _DEPENDENCY_CLOSURE_TYPE + ) -> "DependencyClosure": + closure = DependencyClosure(closure_type) + get_requirements = getattr(self, f"_get_{closure_type}_requirements") + for requirement in get_requirements(): + if self._requirement_applies_to_environment(requirement): + closure[requirement] = Package( + name=requirement.name, + workdir_path=self._workdir_path, + from_requirement=requirement, + ) + return closure + + def _get_runtime_requirements(self) -> List[Requirement]: + req_strings = importlib.metadata.distribution(self.name).requires + if req_strings is None: + return [] + return [Requirement(req_string) for req_string in req_strings] + + def _get_build_requirements(self) -> List[Requirement]: + build_req_strings = set() + venv = self._get_venv() + project_root = self._get_project_root(venv) + builder = build.ProjectBuilder( + project_root, python_executable=str(venv.python_exe) + ) + build_req_strings.update(builder.build_system_requires) + if builder.build_system_requires: + venv.pip(["install"] + list(builder.build_system_requires)) + build_req_strings.update(builder.get_requires_for_build("sdist")) + build_req_strings.update(builder.get_requires_for_build("wheel")) + return [ + Requirement(build_req_string) + for build_req_string in build_req_strings + ] + + def _get_venv(self) -> "Venv": + venv_basepath = self._workdir_path / "venv" + if self._from_requirement: + venv_path = venv_basepath / str(self._from_requirement) + else: + venv_path = venv_basepath / self.name + venv_path.mkdir(parents=True, exist_ok=True) + return Venv(venv_path) + + def _get_project_root(self, venv: "Venv") -> Path: + if self._project_root: + return self._project_root + if self._from_requirement is None: + raise ValueError( + "Must either declare a project_root or from_requirement " + "to download sdist" + ) + return self._get_unpacked_sdist(venv, self._from_requirement) + + def _get_unpacked_sdist( + self, venv: "Venv", requirement: Requirement + ) -> Path: + sdist_workdir = self._workdir_path / "sdist" / str(requirement) + sdist_workdir.mkdir(parents=True, exist_ok=True) + for path in sdist_workdir.iterdir(): + if path.is_dir(): + return path + return self._download_and_unpack_sdist( + venv, requirement, sdist_workdir + ) + + def _download_and_unpack_sdist( + self, venv: "Venv", requirement: Requirement, sdist_workdir: Path + ) -> Path: + venv.pip( + [ + "download", + str(requirement), + "--no-binary", + ":all:", + "--no-deps", + ], + cwd=sdist_workdir, + ) + sdist_path = list(sdist_workdir.iterdir())[0] + shutil.unpack_archive(sdist_path, extract_dir=sdist_workdir) + # Strip off the .tar.gz from the downloaded sdist as that will + # match the directory name when the sdist is unpacked + unpacked_sdist_dirname = sdist_path.name[:-7] + return sdist_workdir / unpacked_sdist_dirname + + def _requirement_applies_to_environment( + self, requirement: Requirement + ) -> bool: + # Do not include any requirements defined as extras as currently + # our dependency closure does not use any extras + if requirement.extras: + return False + # Only include requirements where the markers apply to the current + # environment. + if requirement.marker and not requirement.marker.evaluate(): + return False + return True + + +class DependencyClosure: + def __init__(self, closure_type: _DEPENDENCY_CLOSURE_TYPE): + self._closure_type = closure_type + self._req_to_package: Dict[Requirement, Package] = {} + + def __setitem__(self, key: Requirement, value: Package) -> None: + self._req_to_package[key] = value + + def __getitem__(self, key: Requirement) -> Package: + return self._req_to_package[key] + + def __delitem__(self, key: Requirement) -> None: + del self._req_to_package[key] + + def __iter__(self) -> Iterator[Requirement]: + return iter(self._req_to_package) + + def __len__(self) -> int: + return len(self._req_to_package) + + def walk(self) -> Iterator[Tuple[Requirement, Package]]: + for req, package in self._req_to_package.items(): + yield req, package + subclosure = getattr(package, f"{self._closure_type}_dependencies") + yield from subclosure.walk() + + def to_dict(self) -> _NESTED_STR_DICT: + reqs = {} + for req, package in self._req_to_package.items(): + subclosure = getattr(package, f"{self._closure_type}_dependencies") + reqs[str(req)] = subclosure.to_dict() + return reqs + + +class Venv: + def __init__(self, root: Path, create: bool = True): + self._root = root + if create: + venv.create(self._root, with_pip=True) + + @property + def python_exe(self) -> Path: + return self._root / VENV_BIN_DIRNAME / PYTHON_EXE_NAME + + def pip(self, args: List[str], cwd: Optional[Path] = None) -> None: + subprocess.run( + [str(self.python_exe), "-m", "pip"] + args, cwd=cwd, check=True + ) + + +class TestDependencyClosure: + # Most packages depend either implicitly or explcitly on setuptools + # and wheels to build. Declare this build closure here to cut down + # verbosity of tests. + _DEFAULT_BUILD_DEPENDENCIES = { + "setuptools", + "wheel", + "flit_core", # Build dependency of "wheel" + } + + # Most packages depend either implicitly or explcitly on setuptools + # and wheels to build. And these dependencies are usually unbounded. + _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES = { + "setuptools", + "wheel", + } + + def _is_bounded_version_requirement( + self, requirement: Requirement + ) -> bool: + for specifier in requirement.specifier: + if specifier.operator in ["==", "=<", "<"]: + return True + return False + + def _pformat_closure(self, closure: DependencyClosure) -> str: + return json.dumps(closure.to_dict(), sort_keys=True, indent=2) + + def test_expected_runtime_dependencies(self, awscli_package): + expected_dependencies = { + "botocore", + "colorama", + "docutils", + "jmespath", + "pyasn1", + "python-dateutil", + "PyYAML", + "rsa", + "s3transfer", + "six", + "urllib3", + } + actual_dependencies = set() + for _, package in awscli_package.runtime_dependencies.walk(): + actual_dependencies.add(package.name) + assert actual_dependencies == expected_dependencies, ( + f"Unexpected dependency found in awscli runtime closure: " + f"{self._pformat_closure(awscli_package.runtime_dependencies)}" + ) + + def test_expected_unbounded_runtime_dependencies(self, awscli_package): + expected_unbounded_dependencies = { + "pyasn1", # Transitive dependency from rsa + "six", # Transitive dependency from python-dateutil + } + actual_unbounded_dependencies = set() + for req, package in awscli_package.runtime_dependencies.walk(): + if not self._is_bounded_version_requirement(req): + actual_unbounded_dependencies.add(package.name) + assert ( + actual_unbounded_dependencies == expected_unbounded_dependencies + ), ( + f"Unexpected unbounded dependency found in awscli runtime closure: " + f"{self._pformat_closure(awscli_package.runtime_dependencies)}" + ) + + @pytest.mark.parametrize( + "package, expected_build_dependencies", + [ + ("awscli", _DEFAULT_BUILD_DEPENDENCIES), + ("botocore", _DEFAULT_BUILD_DEPENDENCIES), + ("colorama", _DEFAULT_BUILD_DEPENDENCIES), + ("docutils", _DEFAULT_BUILD_DEPENDENCIES), + ("jmespath", _DEFAULT_BUILD_DEPENDENCIES), + ("pyasn1", _DEFAULT_BUILD_DEPENDENCIES), + ( + "python-dateutil", + _DEFAULT_BUILD_DEPENDENCIES | {"setuptools_scm"}, + ), + ("PyYAML", _DEFAULT_BUILD_DEPENDENCIES | {"Cython"}), + ("rsa", _DEFAULT_BUILD_DEPENDENCIES), + ("s3transfer", _DEFAULT_BUILD_DEPENDENCIES), + ("six", _DEFAULT_BUILD_DEPENDENCIES), + ( + "urllib3", + _DEFAULT_BUILD_DEPENDENCIES + | { + # Direct build dependency of "urllib3" + "hatchling", + # Build dependendencies of hatchling + "editables", + "packaging", + "pathspec", + "pluggy", + "trove-classifiers", + # Build dependency of "pluggy" + "setuptools-scm", + # Build dependency of "trove-classifers" + "calver", + }, + ), + ], + indirect=["package"], + ) + def test_expected_build_dependencies( + self, package, expected_build_dependencies + ): + actual_dependencies = set() + for req, build_package in package.build_dependencies.walk(): + actual_dependencies.add(build_package.name) + assert actual_dependencies == expected_build_dependencies, ( + f"Unexpected dependency found in {package.name} build closure: " + f"{self._pformat_closure(package.build_dependencies)}" + ) + + @pytest.mark.parametrize( + "package, expected_unbounded_build_dependencies", + [ + ("awscli", _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES), + ("botocore", _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES), + ("colorama", _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES), + ("docutils", _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES), + ("jmespath", _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES), + ("pyasn1", _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES), + ( + "python-dateutil", + _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES | {"setuptools_scm"}, + ), + ("PyYAML", _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES), + ("rsa", _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES), + ("s3transfer", _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES), + ("six", _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES), + ( + "urllib3", + _DEFAULT_UNBOUNDED_BUILD_DEPENDENCIES + | { + # Build dependendencies of "hatchling" + "editables", + "packaging", + "pathspec", + "pluggy", + "trove-classifiers", + # Build dependency of "pluggy" + "setuptools-scm", + # Build dependency of "trove-classifers" + "calver", + # Unbounded build dependency of "packaging" and "editables" + "flit_core", + }, + ), + ], + indirect=["package"], + ) + def test_expected_unbounded_build_dependencies( + self, package, expected_unbounded_build_dependencies + ): + actual_dependencies = set() + for req, build_package in package.build_dependencies.walk(): + if not self._is_bounded_version_requirement(req): + actual_dependencies.add(build_package.name) + assert actual_dependencies == expected_unbounded_build_dependencies, ( + f"Unexpected unbounded dependency found in {package.name} build closure: " + f"{self._pformat_closure(package.build_dependencies)}" + )