From d372dd67a52833595bbf953844afc529cc831fe5 Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Tue, 28 Apr 2026 14:10:20 -0700 Subject: [PATCH 1/2] replace git clone with zip downloads, reuse retry machinery --- devservices/constants.py | 5 - devservices/exceptions.py | 12 - devservices/utils/dependencies.py | 369 +--- devservices/utils/install_binary.py | 33 +- devservices/utils/retry.py | 40 + testing/utils.py | 41 + tests/commands/test_down.py | 319 ++-- tests/commands/test_status.py | 16 +- tests/commands/test_up.py | 145 +- tests/utils/test_dependencies.py | 2418 +++++++-------------------- 10 files changed, 1028 insertions(+), 2370 deletions(-) create mode 100644 devservices/utils/retry.py diff --git a/devservices/constants.py b/devservices/constants.py index b5783296..5832a49a 100644 --- a/devservices/constants.py +++ b/devservices/constants.py @@ -37,11 +37,6 @@ class DependencyType(StrEnum): DEVSERVICES_ORCHESTRATOR_LABEL = "orchestrator=devservices" DEPENDENCY_CONFIG_VERSION = "v1" -DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS = { - "protocol.version": "2", - "extensions.partialClone": "true", - "core.sparseCheckout": "true", -} DEVSERVICES_RELEASES_URL = ( "https://api.github.com/repos/getsentry/devservices/releases/latest" diff --git a/devservices/exceptions.py b/devservices/exceptions.py index a608fe1a..c17ebb2f 100644 --- a/devservices/exceptions.py +++ b/devservices/exceptions.py @@ -142,18 +142,6 @@ def __init__(self, command: str, returncode: int, stderr: str): self.stderr = stderr -class GitConfigError(Exception): - """Base class for git config related errors.""" - - pass - - -class FailedToSetGitConfigError(GitConfigError): - """Raised when a git config cannot be set.""" - - pass - - class ContainerHealthcheckFailedError(Exception): """Raised when a container is not healthy.""" diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py index cc1bec8e..41adbc47 100644 --- a/devservices/utils/dependencies.py +++ b/devservices/utils/dependencies.py @@ -1,21 +1,18 @@ from __future__ import annotations -import logging +import io import os import shutil -import subprocess -import tempfile -import time +import urllib.error +import urllib.request +import zipfile from collections import deque from concurrent.futures import ThreadPoolExecutor from concurrent.futures import as_completed from dataclasses import dataclass -from typing import TextIO from typing import TypeGuard -from sentry_sdk import capture_message from sentry_sdk import logger as sentry_logger -from sentry_sdk import set_context from devservices.configs.service_config import Dependency from devservices.configs.service_config import RemoteConfig @@ -23,21 +20,18 @@ from devservices.configs.service_config import load_service_config_from_file from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEPENDENCY_CONFIG_VERSION -from devservices.constants import DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR from devservices.constants import DEVSERVICES_DIR_NAME -from devservices.constants import LOGGER_NAME from devservices.constants import DependencyType from devservices.exceptions import ConfigNotFoundError from devservices.exceptions import ConfigParseError from devservices.exceptions import ConfigValidationError from devservices.exceptions import DependencyError from devservices.exceptions import DependencyNotInstalledError -from devservices.exceptions import FailedToSetGitConfigError from devservices.exceptions import InvalidDependencyConfigError from devservices.exceptions import ModeDoesNotExistError -from devservices.exceptions import UnableToCloneDependencyError from devservices.utils.file_lock import lock +from devservices.utils.retry import retry from devservices.utils.services import Service from devservices.utils.services import find_matching_service from devservices.utils.services import get_active_service_names @@ -45,17 +39,6 @@ from devservices.utils.state import State from devservices.utils.state import StateTables -RELEVANT_GIT_CONFIG_KEYS = [ - "init.defaultbranch", - "core.sparsecheckout", - "remote.origin.url", - "remote.origin.fetch", - "remote.origin.promisor", - "remote.origin.partialclonefilter", - "protocol.version", - "extensions.partialclone", -] - @dataclass(frozen=True, eq=True) class DependencyNode: @@ -129,87 +112,6 @@ class InstalledRemoteDependency: mode: str = "default" -class SparseCheckoutManager: - """ - Manages sparse checkout for a repo - """ - - def __init__(self, repo_dir: str): - self.repo_dir = repo_dir - - def init_sparse_checkout(self) -> None: - """ - Initialize sparse checkout for the repo - """ - _run_command(["git", "sparse-checkout", "init"], cwd=self.repo_dir) - - def set_sparse_checkout(self, pattern: str) -> None: - """ - Set sparse checkout patterns for the repo - """ - self.init_sparse_checkout() - _run_command(["git", "sparse-checkout", "set", pattern], cwd=self.repo_dir) - - -class GitConfigManager: - """ - Manages git config for a repo - """ - - def __init__( - self, - repo_dir: str, - config_options: dict[str, str], - sparse_pattern: str | None = None, - ) -> None: - self.repo_dir = repo_dir - self.config_options = config_options - self.sparse_pattern = sparse_pattern - self.sparse_checkout_manager = SparseCheckoutManager(repo_dir) - - def ensure_config(self) -> None: - """ - Ensure that the git config is set correctly for the repo - """ - # Otherwise, set the config options - for key, value in self.config_options.items(): - self._set_config(key, value) - - if self.sparse_pattern: - self.sparse_checkout_manager.set_sparse_checkout(self.sparse_pattern) - - def get_relevant_config(self) -> dict[str, str]: - """ - Get the relevant git config entries (to avoid logging sensitive information) - """ - git_config = ( - subprocess.check_output( - ["git", "config", "--list"], - cwd=self.repo_dir, - stderr=subprocess.PIPE, - ) - .decode() - .strip() - ) - git_config_dict = dict() - for line in git_config.split("\n"): - if not line: - continue - key, value = line.split("=") - if key in RELEVANT_GIT_CONFIG_KEYS: - git_config_dict[key] = value - return git_config_dict - - def _set_config(self, key: str, value: str) -> None: - """ - Set a git config option for the repo - """ - try: - _run_command(["git", "config", key, value], cwd=self.repo_dir) - except subprocess.CalledProcessError as e: - raise FailedToSetGitConfigError from e - - def install_and_verify_dependencies( service: Service, force_update_dependencies: bool = False, @@ -435,14 +337,7 @@ def install_dependency(dependency: RemoteConfig) -> set[InstalledRemoteDependenc DEVSERVICES_DEPENDENCIES_CACHE_DIR, f"{dependency.repo_name}.lock" ) with lock(lock_path): - if ( - os.path.exists(dependency_repo_dir) - and _is_valid_repo(dependency_repo_dir) - and _has_valid_config_file(dependency_repo_dir) - ): - _update_dependency(dependency, dependency_repo_dir) - else: - _checkout_dependency(dependency, dependency_repo_dir) + _fetch_dependency(dependency, dependency_repo_dir) if not verify_local_dependency(dependency): # TODO: what should we do if the local dependency isn't installed correctly? @@ -503,181 +398,95 @@ def install_dependency(dependency: RemoteConfig) -> set[InstalledRemoteDependenc return installed_dependencies -def _update_dependency( +def _parse_github_repo_path(repo_link: str) -> str: + url = repo_link.rstrip("/").removesuffix(".git") + if "github.com/" not in url: + raise ValueError(f"Not a GitHub URL: {repo_link}") + return url.split("github.com/", 1)[1] + + +def _fetch_dependency( dependency: RemoteConfig, dependency_repo_dir: str, ) -> None: sentry_logger.info( - "Updating dependency", + "Fetching dependency", extra={ "repo_name": dependency.repo_name, "repo_link": dependency.repo_link, "branch": dependency.branch, }, ) - git_config_manager = GitConfigManager( - dependency_repo_dir, - DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS, - f"{DEVSERVICES_DIR_NAME}/", - ) try: - git_config_manager.ensure_config() - except FailedToSetGitConfigError as e: + repo_path = _parse_github_repo_path(dependency.repo_link) + except ValueError as e: raise DependencyError( repo_name=dependency.repo_name, repo_link=dependency.repo_link, branch=dependency.branch, ) from e - try: - _run_command_with_retries( - [ - "git", - "fetch", - "origin", - dependency.branch, - "--filter=blob:none", - "--no-recurse-submodules", # Avoid fetching submodules - ], - cwd=dependency_repo_dir, + zip_url = f"https://api.github.com/repos/{repo_path}/zipball/{dependency.branch}" + + def _download() -> bytes: + req = urllib.request.Request( + zip_url, + headers={"Accept": "application/vnd.github+json"}, ) - except subprocess.CalledProcessError as e: - # Try to set the git config context to help with debugging - _try_set_git_config_context(git_config_manager) - raise DependencyError( - repo_name=dependency.repo_name, - repo_link=dependency.repo_link, - branch=dependency.branch, - stderr=e.stderr, - ) from e + with urllib.request.urlopen(req) as response: + return bytes(response.read()) - # Check if the local repo is up-to-date try: - local_commit = _rev_parse(dependency_repo_dir, "HEAD") - except subprocess.CalledProcessError as e: + zip_data = io.BytesIO(retry(_download, exceptions=(urllib.error.URLError,))) + except urllib.error.URLError as e: raise DependencyError( repo_name=dependency.repo_name, repo_link=dependency.repo_link, branch=dependency.branch, - stderr=e.stderr, ) from e try: - remote_commit = _rev_parse(dependency_repo_dir, "FETCH_HEAD") - except subprocess.CalledProcessError as e: - raise DependencyError( - repo_name=dependency.repo_name, - repo_link=dependency.repo_link, - branch=dependency.branch, - stderr=e.stderr, - ) from e - - if local_commit == remote_commit: - # Already up-to-date, don't pull anything - logger = logging.getLogger(LOGGER_NAME) - logger.debug( - "Dependency %s is already up-to-date, not pulling anything", - dependency.repo_name, - ) - return + with zipfile.ZipFile(zip_data) as zf: + names = zf.namelist() + if not names: + raise DependencyError( + repo_name=dependency.repo_name, + repo_link=dependency.repo_link, + branch=dependency.branch, + ) + # GitHub zips always have a single top-level directory: "owner-repo-sha/" + prefix = names[0].split("/")[0] + "/" + devservices_prefix = f"{prefix}{DEVSERVICES_DIR_NAME}/" + to_extract = [ + name + for name in names + if name.startswith(devservices_prefix) and not name.endswith("/") + ] + if not to_extract: + raise DependencyError( + repo_name=dependency.repo_name, + repo_link=dependency.repo_link, + branch=dependency.branch, + ) - # If it's not up-to-date, checkout the latest changes (forcibly) - try: - _run_command(["git", "checkout", "-f", "FETCH_HEAD"], cwd=dependency_repo_dir) - except subprocess.CalledProcessError as e: + if os.path.exists(dependency_repo_dir): + shutil.rmtree(dependency_repo_dir) + os.makedirs(dependency_repo_dir) + + for name in to_extract: + relative_path = name[len(prefix) :] + target = os.path.join(dependency_repo_dir, relative_path) + os.makedirs(os.path.dirname(target), exist_ok=True) + with zf.open(name) as src, open(target, "wb") as dst: + shutil.copyfileobj(src, dst) + except zipfile.BadZipFile as e: raise DependencyError( repo_name=dependency.repo_name, repo_link=dependency.repo_link, branch=dependency.branch, - stderr=e.stderr, ) from e -def _checkout_dependency( - dependency: RemoteConfig, - dependency_repo_dir: str, -) -> None: - sentry_logger.info( - "Checking out dependency", - extra={ - "repo_name": dependency.repo_name, - "repo_link": dependency.repo_link, - "branch": dependency.branch, - }, - ) - with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as temp_dir: - try: - _run_command( - [ - "git", - "clone", - "--filter=blob:none", - "--no-checkout", - dependency.repo_link, - temp_dir, - ], - cwd=temp_dir, - ) - except subprocess.CalledProcessError as e: - raise UnableToCloneDependencyError( - repo_name=dependency.repo_name, - repo_link=dependency.repo_link, - branch=dependency.branch, - stderr=e.stderr, - ) from e - - # Setup config for partial clone and sparse checkout - git_config_manager = GitConfigManager( - temp_dir, - DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS, - f"{DEVSERVICES_DIR_NAME}/", - ) - try: - git_config_manager.ensure_config() - except FailedToSetGitConfigError as e: - raise DependencyError( - repo_name=dependency.repo_name, - repo_link=dependency.repo_link, - branch=dependency.branch, - ) from e - - try: - _run_command( - ["git", "checkout", dependency.branch], - cwd=temp_dir, - ) - except subprocess.CalledProcessError as e: - raise DependencyError( - repo_name=dependency.repo_name, - repo_link=dependency.repo_link, - branch=dependency.branch, - stderr=e.stderr, - ) from e - - # Clean up the existing directory if it exists - if os.path.exists(dependency_repo_dir): - shutil.rmtree(dependency_repo_dir) - # Copy the cloned repo to the dependency cache directory - try: - shutil.copytree(temp_dir, dst=dependency_repo_dir) - except FileExistsError as e: - raise DependencyError( - repo_name=dependency.repo_name, - repo_link=dependency.repo_link, - branch=dependency.branch, - ) from e - - -def _is_valid_repo(path: str) -> bool: - if not os.path.exists(os.path.join(path, ".git")): - return False - try: - _run_command(["git", "rev-parse", "--is-inside-work-tree"], cwd=path) - return True - except subprocess.CalledProcessError: - return False - - def _has_valid_config_file(path: str) -> bool: return os.path.exists(os.path.join(path, DEVSERVICES_DIR_NAME, CONFIG_FILE_NAME)) @@ -694,64 +503,6 @@ def _has_remote_config(remote_config: RemoteConfig | None) -> TypeGuard[RemoteCo return remote_config is not None -def _rev_parse(repo_dir: str, ref: str) -> str: - logger = logging.getLogger(LOGGER_NAME) - logger.debug("Parsing revision for %s (%s)", ref, repo_dir) - rev = ( - subprocess.check_output( - ["git", "rev-parse", ref], cwd=repo_dir, stderr=subprocess.PIPE - ) - .strip() - .decode() - ) - logger.debug("Parsed revision %s for %s (%s)", rev, ref, repo_dir) - return rev - - -def _run_command( - cmd: list[str], cwd: str, stdout: int | TextIO | None = subprocess.DEVNULL -) -> None: - logger = logging.getLogger(LOGGER_NAME) - logger.debug("Running command: %s in %s", " ".join(cmd), cwd) - subprocess.run(cmd, cwd=cwd, check=True, stdout=stdout, stderr=subprocess.PIPE) - - -def _run_command_with_retries( - cmd: list[str], - cwd: str, - stdout: int | TextIO | None = subprocess.DEVNULL, - retries: int = 3, - backoff: int = 2, -) -> None: - for i in range(retries): - try: - _run_command(cmd, cwd=cwd, stdout=stdout) - break - except subprocess.CalledProcessError as e: - logger = logging.getLogger(LOGGER_NAME) - logger.debug( - "Attempt %s of %s for %s failed: %s", i + 1, retries, cmd, e.stderr - ) - capture_message( - f"Attempt {i + 1} of {retries} for {cmd} failed: {e.stderr}", - level="warning", - ) - if i == retries - 1: - raise e - time.sleep(backoff**i) - - -def _try_set_git_config_context( - git_config_manager: GitConfigManager, -) -> None: - try: - git_config = git_config_manager.get_relevant_config() - set_context("git_config", git_config) - except subprocess.CalledProcessError as e: - logger = logging.getLogger(LOGGER_NAME) - logger.exception(e) - - def get_remote_dependency_config(remote_config: RemoteConfig) -> ServiceConfig: dependency_repo_dir = os.path.join( DEVSERVICES_DEPENDENCIES_CACHE_DIR, diff --git a/devservices/utils/install_binary.py b/devservices/utils/install_binary.py index a1bc195b..9049b53f 100644 --- a/devservices/utils/install_binary.py +++ b/devservices/utils/install_binary.py @@ -3,12 +3,12 @@ import os import shutil import tempfile -import time from urllib.request import urlretrieve from devservices.constants import BINARY_PERMISSIONS from devservices.exceptions import BinaryInstallError from devservices.utils.console import Console +from devservices.utils.retry import retry def install_binary( @@ -21,26 +21,21 @@ def install_binary( with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as temp_dir: temp_file = os.path.join(temp_dir, binary_name) - # Download the binary with retries - max_retries = 3 - retry_delay_seconds = 1 console.info(f"Downloading {binary_name} {version} from {url}...") - for attempt in range(max_retries): - try: - urlretrieve(url, temp_file) - break - except Exception as e: - if attempt < max_retries - 1: - console.warning( - f"Download failed. Retrying in {retry_delay_seconds} seconds... (Attempt {attempt + 1}/{max_retries - 1})" - ) - time.sleep(retry_delay_seconds) - else: - raise BinaryInstallError( - f"Failed to download {binary_name} after {max_retries} attempts: {e}" - ) from e + try: + retry( + lambda: urlretrieve(url, temp_file), + retries=3, + delay=1.0, + on_retry=lambda e, remaining: console.warning( + f"Download failed. Retrying in 1 seconds... ({remaining} retries left)" + ), + ) + except Exception as e: + raise BinaryInstallError( + f"Failed to download {binary_name} after 3 attempts: {e}" + ) from e - # Make the binary executable try: os.chmod(temp_file, BINARY_PERMISSIONS) except (PermissionError, FileNotFoundError) as e: diff --git a/devservices/utils/retry.py b/devservices/utils/retry.py new file mode 100644 index 00000000..a0c078da --- /dev/null +++ b/devservices/utils/retry.py @@ -0,0 +1,40 @@ +from __future__ import annotations + +import time +from collections.abc import Callable +from typing import TypeVar + +T = TypeVar("T") + + +def retry( + fn: Callable[[], T], + *, + retries: int = 3, + delay: float = 1.0, + exceptions: tuple[type[BaseException], ...] = (Exception,), + on_retry: Callable[[BaseException, int], None] | None = None, +) -> T: + """ + Call fn() up to `retries` times, sleeping `delay` seconds between attempts. + Raises the last exception if all attempts fail. + + Args: + fn: Callable to invoke. + retries: Total number of attempts (must be >= 1). + delay: Seconds to sleep between attempts. + exceptions: Exception types that trigger a retry. + on_retry: Optional callback(exc, attempts_remaining) invoked before each sleep. + """ + if retries < 1: + raise ValueError("retries must be >= 1") + for attempt in range(retries): + try: + return fn() + except exceptions as e: + if attempt == retries - 1: + raise + if on_retry is not None: + on_retry(e, retries - attempt - 1) + time.sleep(delay) + raise AssertionError("unreachable") diff --git a/testing/utils.py b/testing/utils.py index 9ea28fc7..efbc52fb 100644 --- a/testing/utils.py +++ b/testing/utils.py @@ -1,9 +1,14 @@ from __future__ import annotations +import io import os import shutil import subprocess +import zipfile +from collections.abc import Callable +from collections.abc import Mapping from pathlib import Path +from unittest import mock import yaml @@ -39,3 +44,39 @@ def create_mock_git_repo(test_repo_src: str, path: Path) -> Path: run_git_command(["add", "."], cwd=path) run_git_command(["commit", "-m", "Initial commit"], cwd=path) return path + + +def make_zip_bytes( + config: Mapping[str, object] | str | None = None, + prefix: str = "owner-repo-abc123", +) -> bytes: + """Build an in-memory GitHub-style zipball with a devservices/config.yml entry.""" + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + if config is not None: + content = config if isinstance(config, str) else yaml.dump(config) + zf.writestr(f"{prefix}/devservices/config.yml", content) + return buf.getvalue() + + +def make_urlopen_response(zip_bytes: bytes) -> mock.MagicMock: + resp = mock.MagicMock() + resp.read.return_value = zip_bytes + resp.__enter__ = mock.Mock(return_value=resp) + resp.__exit__ = mock.Mock(return_value=False) + return resp + + +def url_dispatch( + zip_bytes_by_repo_name: dict[str, bytes], +) -> Callable[[mock.MagicMock], mock.MagicMock]: + """Return a urlopen side_effect that routes requests by repo name in the URL.""" + + def _side_effect(request: mock.MagicMock) -> mock.MagicMock: + url = request.full_url + for repo_name, zip_bytes in zip_bytes_by_repo_name.items(): + if f"/{repo_name}/" in url: + return make_urlopen_response(zip_bytes) + raise AssertionError(f"No mock zip configured for URL: {url}") + + return _side_effect diff --git a/tests/commands/test_down.py b/tests/commands/test_down.py index 8cd97adc..d71f92c1 100644 --- a/tests/commands/test_down.py +++ b/tests/commands/test_down.py @@ -26,8 +26,8 @@ from devservices.utils.state import State from devservices.utils.state import StateTables from testing.utils import create_config_file -from testing.utils import create_mock_git_repo -from testing.utils import run_git_command +from testing.utils import make_zip_bytes +from testing.utils import url_dispatch @mock.patch("devservices.utils.state.State.remove_service_entry") @@ -439,8 +439,6 @@ def test_down_overlapping_services( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - redis_repo_path = tmp_path / "redis" - create_mock_git_repo("blank_repo", redis_repo_path) redis_config = { "x-sentry-service-config": { "version": 0.1, @@ -454,9 +452,6 @@ def test_down_overlapping_services( "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(redis_repo_path, redis_config) - run_git_command(["add", "."], cwd=redis_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=redis_repo_path) example_config = { "x-sentry-service-config": { @@ -468,7 +463,7 @@ def test_down_overlapping_services( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "clickhouse": {"description": "Clickhouse"}, @@ -492,7 +487,7 @@ def test_down_overlapping_services( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, }, @@ -517,9 +512,15 @@ def test_down_overlapping_services( args = Namespace(service_name=None, debug=False, exclude_local=False) - with mock.patch( - "devservices.commands.down._bring_down_dependency" - ) as mock_bring_down_dependency: + with ( + mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch({"redis": make_zip_bytes(redis_config)}), + ), + mock.patch( + "devservices.commands.down._bring_down_dependency" + ) as mock_bring_down_dependency, + ): down(args) # Shouldn't stop redis because other-service is using it @@ -580,7 +581,6 @@ def test_down_does_not_stop_service_being_used_by_another_service( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - redis_repo_path = create_mock_git_repo("blank_repo", tmp_path / "redis") redis_config = { "x-sentry-service-config": { "version": 0.1, @@ -594,13 +594,7 @@ def test_down_does_not_stop_service_being_used_by_another_service( "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(redis_repo_path, redis_config) - run_git_command(["add", "."], cwd=redis_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=redis_repo_path) - example_repo_path = create_mock_git_repo( - "blank_repo", tmp_path / "example-service" - ) example_config = { "x-sentry-service-config": { "version": 0.1, @@ -611,7 +605,7 @@ def test_down_does_not_stop_service_being_used_by_another_service( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "clickhouse": {"description": "Clickhouse"}, @@ -624,11 +618,6 @@ def test_down_does_not_stop_service_being_used_by_another_service( }, }, } - create_config_file(example_repo_path, example_config) - run_git_command(["add", "."], cwd=example_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=example_repo_path - ) example_service_path = tmp_path / "code" / "example-service" create_config_file(example_service_path, example_config) @@ -643,7 +632,7 @@ def test_down_does_not_stop_service_being_used_by_another_service( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "example-service": { @@ -651,7 +640,7 @@ def test_down_does_not_stop_service_being_used_by_another_service( "remote": { "repo_name": "example-service", "branch": "main", - "repo_link": f"file://{example_repo_path}", + "repo_link": "https://github.com/getsentry/example-service", }, }, }, @@ -661,39 +650,48 @@ def test_down_does_not_stop_service_being_used_by_another_service( other_service_path = tmp_path / "code" / "other-service" create_config_file(other_service_path, other_config) - install_and_verify_dependencies( - Service( - name="other-service", - repo_path=str(other_service_path), - config=ServiceConfig( - version=0.1, - service_name="other-service", - dependencies={ - "redis": Dependency( - description="Redis", - dependency_type=DependencyType.SERVICE, - remote=RemoteConfig( - repo_name="redis", - repo_link=f"file://{redis_repo_path}", - branch="main", - mode="default", + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch( + { + "redis": make_zip_bytes(redis_config), + "example-service": make_zip_bytes(example_config), + } + ), + ): + install_and_verify_dependencies( + Service( + name="other-service", + repo_path=str(other_service_path), + config=ServiceConfig( + version=0.1, + service_name="other-service", + dependencies={ + "redis": Dependency( + description="Redis", + dependency_type=DependencyType.SERVICE, + remote=RemoteConfig( + repo_name="redis", + repo_link="https://github.com/getsentry/redis", + branch="main", + mode="default", + ), ), - ), - "example-service": Dependency( - description="Example service", - dependency_type=DependencyType.SERVICE, - remote=RemoteConfig( - repo_name="example-service", - repo_link=f"file://{example_repo_path}", - branch="main", - mode="default", + "example-service": Dependency( + description="Example service", + dependency_type=DependencyType.SERVICE, + remote=RemoteConfig( + repo_name="example-service", + repo_link="https://github.com/getsentry/example-service", + branch="main", + mode="default", + ), ), - ), - }, - modes={"default": ["redis", "example-service"]}, - ), + }, + modes={"default": ["redis", "example-service"]}, + ), + ) ) - ) os.chdir(example_service_path) @@ -749,7 +747,6 @@ def test_down_does_not_stop_nested_service_being_used_by_another_service( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - redis_repo_path = create_mock_git_repo("blank_repo", tmp_path / "redis") redis_config = { "x-sentry-service-config": { "version": 0.1, @@ -763,11 +760,7 @@ def test_down_does_not_stop_nested_service_being_used_by_another_service( "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(redis_repo_path, redis_config) - run_git_command(["add", "."], cwd=redis_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=redis_repo_path) - child_repo_path = create_mock_git_repo("blank_repo", tmp_path / "child-service") child_config = { "x-sentry-service-config": { "version": 0.1, @@ -783,16 +776,10 @@ def test_down_does_not_stop_nested_service_being_used_by_another_service( }, }, } - create_config_file(child_repo_path, child_config) - run_git_command(["add", "."], cwd=child_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=child_repo_path) child_service_path = tmp_path / "code" / "child-service" create_config_file(child_service_path, child_config) - parent_repo_path = create_mock_git_repo( - "blank_repo", tmp_path / "parent-service" - ) parent_config = { "x-sentry-service-config": { "version": 0.1, @@ -803,7 +790,7 @@ def test_down_does_not_stop_nested_service_being_used_by_another_service( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "child-service": { @@ -811,18 +798,13 @@ def test_down_does_not_stop_nested_service_being_used_by_another_service( "remote": { "repo_name": "child-service", "branch": "main", - "repo_link": f"file://{child_repo_path}", + "repo_link": "https://github.com/getsentry/child-service", }, }, }, "modes": {"default": ["redis", "child-service"]}, }, } - create_config_file(parent_repo_path, parent_config) - run_git_command(["add", "."], cwd=parent_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=parent_repo_path - ) grandparent_config = { "x-sentry-service-config": { @@ -834,7 +816,7 @@ def test_down_does_not_stop_nested_service_being_used_by_another_service( "remote": { "repo_name": "parent-service", "branch": "main", - "repo_link": f"file://{parent_repo_path}", + "repo_link": "https://github.com/getsentry/parent-service", }, }, }, @@ -845,29 +827,39 @@ def test_down_does_not_stop_nested_service_being_used_by_another_service( grandparent_service_path = tmp_path / "code" / "other-service" create_config_file(grandparent_service_path, grandparent_config) - install_and_verify_dependencies( - Service( - name="grandparent-service", - repo_path=str(grandparent_service_path), - config=ServiceConfig( - version=0.1, - service_name="grandparent-service", - dependencies={ - "parent-service": Dependency( - description="Parent service", - dependency_type=DependencyType.SERVICE, - remote=RemoteConfig( - repo_name="parent-service", - repo_link=f"file://{parent_repo_path}", - branch="main", - mode="default", + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch( + { + "redis": make_zip_bytes(redis_config), + "child-service": make_zip_bytes(child_config), + "parent-service": make_zip_bytes(parent_config), + } + ), + ): + install_and_verify_dependencies( + Service( + name="grandparent-service", + repo_path=str(grandparent_service_path), + config=ServiceConfig( + version=0.1, + service_name="grandparent-service", + dependencies={ + "parent-service": Dependency( + description="Parent service", + dependency_type=DependencyType.SERVICE, + remote=RemoteConfig( + repo_name="parent-service", + repo_link="https://github.com/getsentry/parent-service", + branch="main", + mode="default", + ), ), - ), - }, - modes={"default": ["parent-service"]}, - ), + }, + modes={"default": ["parent-service"]}, + ), + ) ) - ) os.chdir(child_service_path) @@ -924,8 +916,6 @@ def test_down_overlapping_non_remote_services( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - redis_repo_path = tmp_path / "redis" - create_mock_git_repo("blank_repo", redis_repo_path) redis_config = { "x-sentry-service-config": { "version": 0.1, @@ -939,9 +929,6 @@ def test_down_overlapping_non_remote_services( "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(redis_repo_path, redis_config) - run_git_command(["add", "."], cwd=redis_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=redis_repo_path) redis_service_path = tmp_path / "code" / "redis" create_config_file(redis_service_path, redis_config) @@ -956,7 +943,7 @@ def test_down_overlapping_non_remote_services( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "clickhouse": {"description": "Clickhouse"}, @@ -982,9 +969,15 @@ def test_down_overlapping_non_remote_services( args = Namespace(service_name=None, debug=False, exclude_local=False) - with mock.patch( - "devservices.commands.down._bring_down_dependency" - ) as mock_bring_down_dependency: + with ( + mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch({"redis": make_zip_bytes(redis_config)}), + ), + mock.patch( + "devservices.commands.down._bring_down_dependency" + ) as mock_bring_down_dependency, + ): down(args) # Shouldn't stop redis it's being used by itself @@ -1043,7 +1036,6 @@ def test_down_local_service_with_dependent_service_running( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - redis_repo_path = create_mock_git_repo("blank_repo", tmp_path / "redis") redis_config = { "x-sentry-service-config": { "version": 0.1, @@ -1057,13 +1049,7 @@ def test_down_local_service_with_dependent_service_running( "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(redis_repo_path, redis_config) - run_git_command(["add", "."], cwd=redis_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=redis_repo_path) - local_runtime_repo_path = create_mock_git_repo( - "blank_repo", tmp_path / "local-runtime-service" - ) local_runtime_config = { "x-sentry-service-config": { "version": 0.1, @@ -1074,7 +1060,7 @@ def test_down_local_service_with_dependent_service_running( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "clickhouse": {"description": "Clickhouse"}, @@ -1087,11 +1073,6 @@ def test_down_local_service_with_dependent_service_running( }, }, } - create_config_file(local_runtime_repo_path, local_runtime_config) - run_git_command(["add", "."], cwd=local_runtime_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=local_runtime_repo_path - ) local_runtime_service_path = tmp_path / "code" / "local-runtime-service" create_config_file(local_runtime_service_path, local_runtime_config) @@ -1106,7 +1087,7 @@ def test_down_local_service_with_dependent_service_running( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "local-runtime-service": { @@ -1114,7 +1095,7 @@ def test_down_local_service_with_dependent_service_running( "remote": { "repo_name": "local-runtime-service", "branch": "main", - "repo_link": f"file://{local_runtime_repo_path}", + "repo_link": "https://github.com/getsentry/local-runtime-service", }, }, }, @@ -1126,39 +1107,48 @@ def test_down_local_service_with_dependent_service_running( os.chdir(local_runtime_service_path) - install_and_verify_dependencies( - Service( - name="other-service", - repo_path=str(other_service_path), - config=ServiceConfig( - version=0.1, - service_name="other-service", - dependencies={ - "redis": Dependency( - description="Redis", - dependency_type=DependencyType.SERVICE, - remote=RemoteConfig( - repo_name="redis", - repo_link=f"file://{redis_repo_path}", - branch="main", - mode="default", + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch( + { + "redis": make_zip_bytes(redis_config), + "local-runtime-service": make_zip_bytes(local_runtime_config), + } + ), + ): + install_and_verify_dependencies( + Service( + name="other-service", + repo_path=str(other_service_path), + config=ServiceConfig( + version=0.1, + service_name="other-service", + dependencies={ + "redis": Dependency( + description="Redis", + dependency_type=DependencyType.SERVICE, + remote=RemoteConfig( + repo_name="redis", + repo_link="https://github.com/getsentry/redis", + branch="main", + mode="default", + ), ), - ), - "local-runtime-service": Dependency( - description="Local runtime service", - dependency_type=DependencyType.SERVICE, - remote=RemoteConfig( - repo_name="local-runtime-service", - repo_link=f"file://{local_runtime_repo_path}", - branch="main", - mode="default", + "local-runtime-service": Dependency( + description="Local runtime service", + dependency_type=DependencyType.SERVICE, + remote=RemoteConfig( + repo_name="local-runtime-service", + repo_link="https://github.com/getsentry/local-runtime-service", + branch="main", + mode="default", + ), ), - ), - }, - modes={"default": ["redis", "local-runtime-service"]}, - ), + }, + modes={"default": ["redis", "local-runtime-service"]}, + ), + ) ) - ) state = State() state.update_service_entry( @@ -1235,7 +1225,6 @@ def test_down_shared_and_local_dependencies( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - redis_repo_path = create_mock_git_repo("blank_repo", tmp_path / "redis") redis_config = { "x-sentry-service-config": { "version": 0.1, @@ -1249,13 +1238,7 @@ def test_down_shared_and_local_dependencies( "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(redis_repo_path, redis_config) - run_git_command(["add", "."], cwd=redis_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=redis_repo_path) - local_runtime_repo_path = create_mock_git_repo( - "blank_repo", tmp_path / "local-runtime-service" - ) local_runtime_config = { "x-sentry-service-config": { "version": 0.1, @@ -1266,7 +1249,7 @@ def test_down_shared_and_local_dependencies( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "clickhouse": {"description": "Clickhouse"}, @@ -1279,11 +1262,6 @@ def test_down_shared_and_local_dependencies( }, }, } - create_config_file(local_runtime_repo_path, local_runtime_config) - run_git_command(["add", "."], cwd=local_runtime_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=local_runtime_repo_path - ) local_runtime_service_path = tmp_path / "code" / "local-runtime-service" create_config_file(local_runtime_service_path, local_runtime_config) @@ -1298,7 +1276,7 @@ def test_down_shared_and_local_dependencies( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "local-runtime-service": { @@ -1306,7 +1284,7 @@ def test_down_shared_and_local_dependencies( "remote": { "repo_name": "local-runtime-service", "branch": "main", - "repo_link": f"file://{local_runtime_repo_path}", + "repo_link": "https://github.com/getsentry/local-runtime-service", }, }, }, @@ -1330,6 +1308,15 @@ def test_down_shared_and_local_dependencies( args = Namespace(service_name=None, debug=False, exclude_local=exclude_local) with ( + mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch( + { + "redis": make_zip_bytes(redis_config), + "local-runtime-service": make_zip_bytes(local_runtime_config), + } + ), + ), mock.patch( "devservices.commands.down._bring_down_dependency", ) as mock_bring_down_dependency, diff --git a/tests/commands/test_status.py b/tests/commands/test_status.py index df51a67c..32acec58 100644 --- a/tests/commands/test_status.py +++ b/tests/commands/test_status.py @@ -53,8 +53,6 @@ def test_get_status_json_results( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - test_service_repo_path = tmp_path / "test-service" - create_mock_git_repo("blank_repo", test_service_repo_path) config = { "x-sentry-service-config": { "version": 0.1, @@ -72,10 +70,8 @@ def test_get_status_json_results( }, }, } - service_path = tmp_path / "test-service" - create_config_file(service_path, config) - run_git_command(["add", "."], cwd=test_service_repo_path) - run_git_command(["commit", "-m", "Initial commit"], cwd=test_service_repo_path) + test_service_repo_path = tmp_path / "test-service" + create_config_file(test_service_repo_path, config) service = Service( name="test-service", repo_path=str(test_service_repo_path), @@ -96,7 +92,13 @@ def test_get_status_json_results( ), ) - results = get_status_json_results(service, set(), ["redis", "clickhouse"]) + with mock.patch( + "devservices.commands.status.run_cmd", + side_effect=lambda cmd, env: subprocess.CompletedProcess( + args=cmd, returncode=0, stdout="", stderr="" + ), + ): + results = get_status_json_results(service, set(), ["redis", "clickhouse"]) assert len(results) == 1 assert results[0].args == [ "docker", diff --git a/tests/commands/test_up.py b/tests/commands/test_up.py index 7c16bae8..b0334355 100644 --- a/tests/commands/test_up.py +++ b/tests/commands/test_up.py @@ -30,8 +30,8 @@ from devservices.utils.state import State from devservices.utils.state import StateTables from testing.utils import create_config_file -from testing.utils import create_mock_git_repo -from testing.utils import run_git_command +from testing.utils import make_zip_bytes +from testing.utils import url_dispatch @mock.patch("devservices.utils.state.State.remove_service_entry") @@ -1241,8 +1241,6 @@ def test_up_multiple_modes_overlapping_running_service( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - redis_repo_path = tmp_path / "redis" - create_mock_git_repo("blank_repo", redis_repo_path) mock_redis_config = { "x-sentry-service-config": { "version": 0.1, @@ -1254,9 +1252,6 @@ def test_up_multiple_modes_overlapping_running_service( "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(redis_repo_path, mock_redis_config) - run_git_command(["add", "."], cwd=redis_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=redis_repo_path) config = { "x-sentry-service-config": { "version": 0.1, @@ -1267,7 +1262,7 @@ def test_up_multiple_modes_overlapping_running_service( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "clickhouse": {"description": "Clickhouse"}, @@ -1290,7 +1285,7 @@ def test_up_multiple_modes_overlapping_running_service( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, }, @@ -1320,6 +1315,10 @@ def test_up_multiple_modes_overlapping_running_service( ) with ( + mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch({"redis": make_zip_bytes(mock_redis_config)}), + ), mock.patch( "devservices.commands.up._pull_dependency_images", ) as mock_pull_dependency_images, @@ -1465,7 +1464,6 @@ def test_up_dependency_set_to_local( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - redis_repo_path = create_mock_git_repo("blank_repo", tmp_path / "redis") redis_config = { "x-sentry-service-config": { "version": 0.1, @@ -1479,13 +1477,7 @@ def test_up_dependency_set_to_local( "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(redis_repo_path, redis_config) - run_git_command(["add", "."], cwd=redis_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=redis_repo_path) - local_runtime_repo_path = create_mock_git_repo( - "blank_repo", tmp_path / "local-runtime-service" - ) local_runtime_config = { "x-sentry-service-config": { "version": 0.1, @@ -1496,7 +1488,7 @@ def test_up_dependency_set_to_local( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "clickhouse": {"description": "Clickhouse"}, @@ -1509,11 +1501,6 @@ def test_up_dependency_set_to_local( }, }, } - create_config_file(local_runtime_repo_path, local_runtime_config) - run_git_command(["add", "."], cwd=local_runtime_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=local_runtime_repo_path - ) local_runtime_service_path = tmp_path / "code" / "local-runtime-service" create_config_file(local_runtime_service_path, local_runtime_config) @@ -1528,7 +1515,7 @@ def test_up_dependency_set_to_local( "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "local-runtime-service": { @@ -1536,7 +1523,7 @@ def test_up_dependency_set_to_local( "remote": { "repo_name": "local-runtime-service", "branch": "main", - "repo_link": f"file://{local_runtime_repo_path}", + "repo_link": "https://github.com/getsentry/local-runtime-service", }, }, }, @@ -1559,12 +1546,25 @@ def test_up_dependency_set_to_local( ) with ( + mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch( + { + "redis": make_zip_bytes(redis_config), + "local-runtime-service": make_zip_bytes(local_runtime_config), + } + ), + ), mock.patch( "devservices.commands.up._pull_dependency_images", ) as mock_pull_dependency_images, mock.patch( "devservices.commands.up._bring_up_dependency", ) as mock_bring_up_dependency, + mock.patch( + "devservices.commands.up.get_container_names_for_project", + return_value=[], + ), ): up(args) @@ -1819,9 +1819,6 @@ def test_up_nested_dependency_set_to_local( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - local_runtime_repo_path = create_mock_git_repo( - "blank_repo", tmp_path / "local-runtime-service" - ) local_runtime_config = { "x-sentry-service-config": { "version": 0.1, @@ -1835,18 +1832,10 @@ def test_up_nested_dependency_set_to_local( "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(local_runtime_repo_path, local_runtime_config) - run_git_command(["add", "."], cwd=local_runtime_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=local_runtime_repo_path - ) local_runtime_service_path = tmp_path / "code" / "local-runtime-service" create_config_file(local_runtime_service_path, local_runtime_config) - child_service_repo_path = create_mock_git_repo( - "blank_repo", tmp_path / "child-service" - ) child_config = { "x-sentry-service-config": { "version": 0.1, @@ -1857,7 +1846,7 @@ def test_up_nested_dependency_set_to_local( "remote": { "repo_name": "local-runtime-service", "branch": "main", - "repo_link": f"file://{local_runtime_repo_path}", + "repo_link": "https://github.com/getsentry/local-runtime-service", }, }, "child-service": {"description": "child-service"}, @@ -1868,11 +1857,6 @@ def test_up_nested_dependency_set_to_local( "child-service": {"image": "child-service:latest"}, }, } - create_config_file(child_service_repo_path, child_config) - run_git_command(["add", "."], cwd=child_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=child_service_repo_path - ) child_service_path = tmp_path / "code" / "child-service" create_config_file(child_service_path, child_config) @@ -1887,7 +1871,7 @@ def test_up_nested_dependency_set_to_local( "remote": { "repo_name": "child-service", "branch": "main", - "repo_link": f"file://{child_service_repo_path}", + "repo_link": "https://github.com/getsentry/child-service", }, }, }, @@ -1910,12 +1894,25 @@ def test_up_nested_dependency_set_to_local( ) with ( + mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch( + { + "local-runtime-service": make_zip_bytes(local_runtime_config), + "child-service": make_zip_bytes(child_config), + } + ), + ), mock.patch( "devservices.commands.up._pull_dependency_images", ) as mock_pull_dependency_images, mock.patch( "devservices.commands.up._bring_up_dependency", ) as mock_bring_up_dependency, + mock.patch( + "devservices.commands.up.get_container_names_for_project", + return_value=[], + ), ): up(args) @@ -2132,9 +2129,6 @@ def test_up_does_not_bring_up_nested_dependency_if_set_to_local_and_mode_does_no ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - local_runtime_repo_path = create_mock_git_repo( - "blank_repo", tmp_path / "local-runtime-service" - ) local_runtime_config = { "x-sentry-service-config": { "version": 0.1, @@ -2148,15 +2142,7 @@ def test_up_does_not_bring_up_nested_dependency_if_set_to_local_and_mode_does_no "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(local_runtime_repo_path, local_runtime_config) - run_git_command(["add", "."], cwd=local_runtime_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=local_runtime_repo_path - ) - child_service_repo_path = create_mock_git_repo( - "blank_repo", tmp_path / "child-service" - ) child_config = { "x-sentry-service-config": { "version": 0.1, @@ -2167,7 +2153,7 @@ def test_up_does_not_bring_up_nested_dependency_if_set_to_local_and_mode_does_no "remote": { "repo_name": "local-runtime-service", "branch": "main", - "repo_link": f"file://{local_runtime_repo_path}", + "repo_link": "https://github.com/getsentry/local-runtime-service", }, }, "child-service": {"description": "child-service"}, @@ -2181,11 +2167,6 @@ def test_up_does_not_bring_up_nested_dependency_if_set_to_local_and_mode_does_no "child-service": {"image": "child-service:latest"}, }, } - create_config_file(child_service_repo_path, child_config) - run_git_command(["add", "."], cwd=child_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=child_service_repo_path - ) child_service_path = tmp_path / "code" / "child-service" create_config_file(child_service_path, child_config) @@ -2200,7 +2181,7 @@ def test_up_does_not_bring_up_nested_dependency_if_set_to_local_and_mode_does_no "remote": { "repo_name": "child-service", "branch": "main", - "repo_link": f"file://{child_service_repo_path}", + "repo_link": "https://github.com/getsentry/child-service", }, }, }, @@ -2223,12 +2204,25 @@ def test_up_does_not_bring_up_nested_dependency_if_set_to_local_and_mode_does_no ) with ( + mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch( + { + "local-runtime-service": make_zip_bytes(local_runtime_config), + "child-service": make_zip_bytes(child_config), + } + ), + ), mock.patch( "devservices.commands.up._pull_dependency_images", ) as mock_pull_dependency_images, mock.patch( "devservices.commands.up._bring_up_dependency", ) as mock_bring_up_dependency, + mock.patch( + "devservices.commands.up.get_container_names_for_project", + return_value=[], + ), ): up(args) @@ -2318,7 +2312,6 @@ def test_up_does_not_bring_up_dependency_if_set_to_local_and_mode_does_not_conta ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), ): - redis_repo_path = create_mock_git_repo("blank_repo", tmp_path / "redis") redis_config = { "x-sentry-service-config": { "version": 0.1, @@ -2332,13 +2325,7 @@ def test_up_does_not_bring_up_dependency_if_set_to_local_and_mode_does_not_conta "redis": {"image": "redis:6.2.14-alpine"}, }, } - create_config_file(redis_repo_path, redis_config) - run_git_command(["add", "."], cwd=redis_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=redis_repo_path) - local_runtime_repo_path = create_mock_git_repo( - "blank_repo", tmp_path / "local-runtime-service" - ) local_runtime_config = { "x-sentry-service-config": { "version": 0.1, @@ -2349,7 +2336,7 @@ def test_up_does_not_bring_up_dependency_if_set_to_local_and_mode_does_not_conta "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "clickhouse": {"description": "Clickhouse"}, @@ -2362,11 +2349,6 @@ def test_up_does_not_bring_up_dependency_if_set_to_local_and_mode_does_not_conta }, }, } - create_config_file(local_runtime_repo_path, local_runtime_config) - run_git_command(["add", "."], cwd=local_runtime_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=local_runtime_repo_path - ) local_runtime_service_path = tmp_path / "code" / "local-runtime-service" create_config_file(local_runtime_service_path, local_runtime_config) @@ -2381,7 +2363,7 @@ def test_up_does_not_bring_up_dependency_if_set_to_local_and_mode_does_not_conta "remote": { "repo_name": "redis", "branch": "main", - "repo_link": f"file://{redis_repo_path}", + "repo_link": "https://github.com/getsentry/redis", }, }, "local-runtime-service": { @@ -2389,7 +2371,7 @@ def test_up_does_not_bring_up_dependency_if_set_to_local_and_mode_does_not_conta "remote": { "repo_name": "local-runtime-service", "branch": "main", - "repo_link": f"file://{local_runtime_repo_path}", + "repo_link": "https://github.com/getsentry/local-runtime-service", }, }, }, @@ -2412,12 +2394,25 @@ def test_up_does_not_bring_up_dependency_if_set_to_local_and_mode_does_not_conta ) with ( + mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=url_dispatch( + { + "redis": make_zip_bytes(redis_config), + "local-runtime-service": make_zip_bytes(local_runtime_config), + } + ), + ), mock.patch( "devservices.commands.up._pull_dependency_images", ) as mock_pull_dependency_images, mock.patch( "devservices.commands.up._bring_up_dependency", ) as mock_bring_up_dependency, + mock.patch( + "devservices.commands.up.get_container_names_for_project", + return_value=[], + ), ): up(args) diff --git a/tests/utils/test_dependencies.py b/tests/utils/test_dependencies.py index 2e6ed24b..e090ce2a 100644 --- a/tests/utils/test_dependencies.py +++ b/tests/utils/test_dependencies.py @@ -1,31 +1,32 @@ from __future__ import annotations -import shutil -import subprocess -from datetime import timedelta +import io +import urllib.error +import zipfile +from collections.abc import Callable +from collections.abc import Mapping from pathlib import Path from unittest import mock import pytest -from freezegun import freeze_time +import yaml from devservices.configs.service_config import Dependency from devservices.configs.service_config import RemoteConfig from devservices.configs.service_config import ServiceConfig from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEPENDENCY_CONFIG_VERSION -from devservices.constants import DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS from devservices.constants import DEVSERVICES_DIR_NAME from devservices.constants import DependencyType from devservices.exceptions import DependencyError from devservices.exceptions import DependencyNotInstalledError -from devservices.exceptions import FailedToSetGitConfigError from devservices.exceptions import InvalidDependencyConfigError from devservices.exceptions import ModeDoesNotExistError from devservices.exceptions import ServiceNotFoundError from devservices.utils.dependencies import DependencyNode -from devservices.utils.dependencies import GitConfigManager from devservices.utils.dependencies import InstalledRemoteDependency +from devservices.utils.dependencies import _fetch_dependency +from devservices.utils.dependencies import _parse_github_repo_path from devservices.utils.dependencies import construct_dependency_graph from devservices.utils.dependencies import get_installed_remote_dependencies from devservices.utils.dependencies import get_non_shared_remote_dependencies @@ -38,156 +39,169 @@ from devservices.utils.state import ServiceRuntime from devservices.utils.state import State from devservices.utils.state import StateTables -from testing.utils import create_config_file -from testing.utils import create_mock_git_repo -from testing.utils import run_git_command +BASIC_SERVICE_CONFIG = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "basic", + "dependencies": {}, + "modes": {"default": []}, + } +} -@mock.patch("devservices.utils.dependencies.subprocess.run") -def test_git_config_manager_ensure_config_failure( - mock_run: mock.Mock, tmp_path: Path -) -> None: - repo_dir = tmp_path / "test-repo" - create_mock_git_repo("basic_repo", repo_dir) - mock_run.side_effect = subprocess.CalledProcessError(returncode=1, cmd="test") - git_config_manager = GitConfigManager( - str(repo_dir), - { - "test.config": "test-value", - }, - ) - with pytest.raises(FailedToSetGitConfigError): - git_config_manager.ensure_config() +INVALID_SERVICE_CONFIG_YAML = "not_a_service_config: true\n" -def test_git_config_manager_ensure_config_simple_repo(tmp_path: Path) -> None: - repo_dir = tmp_path / "test-repo" - create_mock_git_repo("basic_repo", repo_dir) - with pytest.raises(subprocess.CalledProcessError): - subprocess.check_output(["git", "config", "--get", "test.config"], cwd=repo_dir) - git_config_manager = GitConfigManager( - str(repo_dir), - { - "test.config": "test-value", - }, - ) - git_config_manager.ensure_config() - assert ( - subprocess.check_output(["git", "config", "--get", "test.config"], cwd=repo_dir) - .decode() - .strip() - == "test-value" - ) +def _make_zip_bytes( + config: Mapping[str, object] | str | None = None, + prefix: str = "owner-repo-abc123", +) -> bytes: + """Build an in-memory GitHub-style zipball with only a devservices/ tree.""" + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + if config is not None: + content = config if isinstance(config, str) else yaml.dump(config) + zf.writestr(f"{prefix}/devservices/config.yml", content) + return buf.getvalue() -def test_git_config_manager_ensure_config_sparse_checkout(tmp_path: Path) -> None: - repo_dir = tmp_path / "test-repo" - create_mock_git_repo("basic_repo", repo_dir) - with pytest.raises(subprocess.CalledProcessError): - subprocess.check_output(["git", "sparse-checkout", "list"], cwd=repo_dir) - git_config_manager = GitConfigManager( - str(repo_dir), - { - "test.config": "test-value", - }, - sparse_pattern="test-pattern", +def _make_urlopen_response(zip_bytes: bytes) -> mock.MagicMock: + resp = mock.MagicMock() + resp.read.return_value = zip_bytes + resp.__enter__ = mock.Mock(return_value=resp) + resp.__exit__ = mock.Mock(return_value=False) + return resp + + +def _url_dispatch( + zip_bytes_by_repo_name: dict[str, bytes], +) -> Callable[[mock.MagicMock], mock.MagicMock]: + """Build a urlopen side_effect that routes requests by repo name in the URL.""" + + def _side_effect(request: mock.MagicMock) -> mock.MagicMock: + url = request.full_url + for repo_name, zip_bytes in zip_bytes_by_repo_name.items(): + if f"/{repo_name}/" in url: + return _make_urlopen_response(zip_bytes) + raise AssertionError(f"No mock zip configured for URL: {url}") + + return _side_effect + + +def test_parse_github_repo_path_valid() -> None: + assert ( + _parse_github_repo_path("https://github.com/getsentry/test-repo") + == "getsentry/test-repo" + ) + assert ( + _parse_github_repo_path("https://github.com/getsentry/test-repo.git") + == "getsentry/test-repo" ) - git_config_manager.ensure_config() assert ( - subprocess.check_output(["git", "sparse-checkout", "list"], cwd=repo_dir) - .decode() - .strip() - == "test-pattern" + _parse_github_repo_path("https://github.com/getsentry/test-repo/") + == "getsentry/test-repo" ) + assert _parse_github_repo_path("http://github.com/org/repo") == "org/repo" -def test_git_config_manager_ensure_config_sparse_checkout_overwrite( - tmp_path: Path, -) -> None: - repo_dir = tmp_path / "test-repo" - create_mock_git_repo("basic_repo", repo_dir) - with pytest.raises(subprocess.CalledProcessError): - subprocess.check_output(["git", "sparse-checkout", "list"], cwd=repo_dir) - git_config_manager = GitConfigManager( - str(repo_dir), - { - "test.config": "test-value", - }, - sparse_pattern="test-pattern", +def test_parse_github_repo_path_non_github() -> None: + with pytest.raises(ValueError): + _parse_github_repo_path("file:///path/to/repo") + with pytest.raises(ValueError): + _parse_github_repo_path("invalid-link") + with pytest.raises(ValueError): + _parse_github_repo_path("https://gitlab.com/org/repo") + + +def test_fetch_dependency_success(tmp_path: Path) -> None: + dep = RemoteConfig( + repo_name="test-repo", + branch="main", + repo_link="https://github.com/getsentry/test-repo", ) - git_config_manager.ensure_config() - assert ( - subprocess.check_output(["git", "sparse-checkout", "list"], cwd=repo_dir) - .decode() - .strip() - == "test-pattern" + zip_bytes = _make_zip_bytes(BASIC_SERVICE_CONFIG) + dest = str(tmp_path / "dest") + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(zip_bytes), + ): + _fetch_dependency(dep, dest) + assert (Path(dest) / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME).exists() + + +def test_fetch_dependency_network_error(tmp_path: Path) -> None: + dep = RemoteConfig( + repo_name="test-repo", + branch="main", + repo_link="https://github.com/getsentry/test-repo", ) + with ( + mock.patch("devservices.utils.retry.time.sleep"), + mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=urllib.error.URLError("connection refused"), + ), + pytest.raises(DependencyError), + ): + _fetch_dependency(dep, str(tmp_path / "dest")) - # Overwrite the sparse checkout pattern and ensure it is set correctly - git_config_manager = GitConfigManager( - str(repo_dir), - { - "test.config": "test-value", - }, - sparse_pattern="new-pattern", + +def test_fetch_dependency_bad_zip(tmp_path: Path) -> None: + dep = RemoteConfig( + repo_name="test-repo", + branch="main", + repo_link="https://github.com/getsentry/test-repo", ) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(b"not a zip"), + ): + with pytest.raises(DependencyError): + _fetch_dependency(dep, str(tmp_path / "dest")) - git_config_manager.ensure_config() - assert ( - subprocess.check_output(["git", "sparse-checkout", "list"], cwd=repo_dir) - .decode() - .strip() - == "new-pattern" +def test_fetch_dependency_empty_zip(tmp_path: Path) -> None: + dep = RemoteConfig( + repo_name="test-repo", + branch="main", + repo_link="https://github.com/getsentry/test-repo", ) + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w"): + pass + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(buf.getvalue()), + ): + with pytest.raises(DependencyError): + _fetch_dependency(dep, str(tmp_path / "dest")) -def test_git_config_manager_get_relevant_config_mostly_empty(tmp_path: Path) -> None: - repo_dir = tmp_path / "test-repo" - create_mock_git_repo("basic_repo", repo_dir) - git_config_manager = GitConfigManager( - str(repo_dir), - { - "init.defaultbranch": "main", - }, +def test_fetch_dependency_no_devservices_dir(tmp_path: Path) -> None: + dep = RemoteConfig( + repo_name="test-repo", + branch="main", + repo_link="https://github.com/getsentry/test-repo", ) - git_config_manager.ensure_config() - relevant_configs = git_config_manager.get_relevant_config() - assert relevant_configs == { - "init.defaultbranch": "main", - } + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr("owner-repo-abc123/README.md", "hello") + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(buf.getvalue()), + ): + with pytest.raises(DependencyError): + _fetch_dependency(dep, str(tmp_path / "dest")) -def test_git_config_manager_get_relevant_config_populated(tmp_path: Path) -> None: - repo_dir = tmp_path / "test-repo" - create_mock_git_repo("basic_repo", repo_dir) - git_config_manager = GitConfigManager( - str(repo_dir), - { - "init.defaultbranch": "main", - "core.sparsecheckout": "true", - "remote.origin.url": "test-url", - "remote.origin.fetch": "test-fetch", - "remote.origin.promisor": "true", - "remote.origin.partialclonefilter": "blob:none", - "protocol.version": "2", - "extensions.partialclone": "true", - "core.filemode": "true", # We don't care about this one - }, - f"{DEVSERVICES_DIR_NAME}/", +def test_fetch_dependency_non_github_url(tmp_path: Path) -> None: + dep = RemoteConfig( + repo_name="test-repo", + branch="main", + repo_link="file:///path/to/repo", ) - git_config_manager.ensure_config() - relevant_configs = git_config_manager.get_relevant_config() - assert relevant_configs == { - "init.defaultbranch": "main", - "core.sparsecheckout": "true", - "remote.origin.url": "test-url", - "remote.origin.fetch": "test-fetch", - "remote.origin.promisor": "true", - "remote.origin.partialclonefilter": "blob:none", - "protocol.version": "2", - "extensions.partialclone": "true", - } + with pytest.raises(DependencyError): + _fetch_dependency(dep, str(tmp_path / "dest")) def test_verify_local_dependencies_no_dependencies(tmp_path: Path) -> None: @@ -215,11 +229,10 @@ def test_verify_local_dependencies_with_remote_dependencies(tmp_path: Path) -> N "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") remote_config = RemoteConfig( repo_name="test-repo", branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", + repo_link="https://github.com/getsentry/test-repo", ) dependency = Dependency( description="Test dependency", @@ -228,7 +241,11 @@ def test_verify_local_dependencies_with_remote_dependencies(tmp_path: Path) -> N ) assert not verify_local_dependencies([dependency]) - install_dependency(remote_config) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(_make_zip_bytes(BASIC_SERVICE_CONFIG)), + ): + install_dependency(remote_config) assert verify_local_dependencies([dependency]) @@ -238,10 +255,7 @@ def test_get_installed_remote_dependencies_empty(tmp_path: Path) -> None: "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - installed_remote_dependencies = get_installed_remote_dependencies( - dependencies=[] - ) - assert installed_remote_dependencies == set() + assert get_installed_remote_dependencies(dependencies=[]) == set() def test_get_installed_remote_dependencies_single_dep_not_installed( @@ -251,13 +265,12 @@ def test_get_installed_remote_dependencies_single_dep_not_installed( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") mock_dependency = Dependency( description="test repo", remote=RemoteConfig( repo_name="test-repo", branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", + repo_link="https://github.com/getsentry/test-repo", ), dependency_type=DependencyType.SERVICE, ) @@ -270,34 +283,33 @@ def test_get_installed_remote_dependencies_single_dep_installed(tmp_path: Path) "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") mock_dependency = Dependency( description="test repo", remote=RemoteConfig( repo_name="test-repo", branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", + repo_link="https://github.com/getsentry/test-repo", ), dependency_type=DependencyType.SERVICE, ) - installed_remote_dependencies_initial = install_dependencies([mock_dependency]) - installed_remote_dependencies = get_installed_remote_dependencies( - dependencies=[mock_dependency] - ) - assert installed_remote_dependencies == installed_remote_dependencies_initial - assert installed_remote_dependencies == set( - [ - InstalledRemoteDependency( - service_name="basic", - repo_path=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - ), - ) - ] - ) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(_make_zip_bytes(BASIC_SERVICE_CONFIG)), + ): + installed_initial = install_dependencies([mock_dependency]) + installed = get_installed_remote_dependencies(dependencies=[mock_dependency]) + assert installed == installed_initial + assert installed == { + InstalledRemoteDependency( + service_name="basic", + repo_path=str( + tmp_path + / "dependency-dir" + / DEPENDENCY_CONFIG_VERSION + / "test-repo" + ), + ) + } def test_install_dependency_invalid_repo(tmp_path: Path) -> None: @@ -312,55 +324,38 @@ def test_install_dependency_invalid_repo(tmp_path: Path) -> None: install_dependency(remote_config) -@mock.patch("devservices.utils.dependencies.GitConfigManager.ensure_config") -def test_install_dependency_git_config_failure( - ensure_config_mock: mock.Mock, tmp_path: Path -) -> None: +def test_install_dependency_network_error(tmp_path: Path) -> None: with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") mock_dependency = RemoteConfig( repo_name="test-repo", branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", + repo_link="https://github.com/getsentry/test-repo", ) - ensure_config_mock.side_effect = FailedToSetGitConfigError() - - with pytest.raises(DependencyError) as e: + with ( + mock.patch("devservices.utils.retry.time.sleep"), + mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=urllib.error.URLError("network unreachable"), + ), + pytest.raises(DependencyError), + ): install_dependency(mock_dependency) - assert e.value.repo_name == "test-repo" - assert e.value.repo_link == f"file://{tmp_path / 'test-repo'}" - assert e.value.branch == "main" - - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - -@mock.patch("devservices.utils.dependencies._run_command") -def test_install_dependency_git_clone_failure( - mock_run_command: mock.Mock, tmp_path: Path -) -> None: +def test_install_dependency_basic(tmp_path: Path) -> None: with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") mock_dependency = RemoteConfig( repo_name="test-repo", branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", + repo_link="https://github.com/getsentry/test-repo", ) - # Sanity check that the config file is not in the dependency directory (yet) assert not ( tmp_path / "dependency-dir" @@ -370,14 +365,22 @@ def test_install_dependency_git_clone_failure( / CONFIG_FILE_NAME ).exists() - mock_run_command.side_effect = subprocess.CalledProcessError( - returncode=1, cmd="test" - ) - - with pytest.raises(DependencyError): + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(_make_zip_bytes(BASIC_SERVICE_CONFIG)), + ): install_dependency(mock_dependency) + # Files outside devservices/ must not be extracted assert not ( + tmp_path + / "dependency-dir" + / DEPENDENCY_CONFIG_VERSION + / "test-repo" + / "README.md" + ).exists() + + assert ( tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION @@ -386,1101 +389,216 @@ def test_install_dependency_git_clone_failure( / CONFIG_FILE_NAME ).exists() - mock_run_command.assert_called_once_with( - [ - "git", - "clone", - "--filter=blob:none", - "--no-checkout", - f"file://{tmp_path / 'test-repo'}", - mock.ANY, - ], - cwd=mock.ANY, - ) - -@mock.patch("devservices.utils.dependencies._run_command") -@mock.patch("devservices.utils.dependencies.GitConfigManager.ensure_config") -def test_install_dependency_git_checkout_failure( - mock_ensure_config: mock.Mock, mock_run_command: mock.Mock, tmp_path: Path -) -> None: +def test_install_dependency_basic_with_update(tmp_path: Path) -> None: with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") mock_dependency = RemoteConfig( repo_name="test-repo", branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", + repo_link="https://github.com/getsentry/test-repo", ) + config_v1 = dict(BASIC_SERVICE_CONFIG) + config_v2 = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "basic", + "dependencies": {}, + "modes": {"default": []}, + "extra": "added-in-v2", + } + } - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - mock_run_command.side_effect = [ - subprocess.CompletedProcess(args="", returncode=0, stdout=""), - subprocess.CalledProcessError(returncode=1, cmd="test"), - ] - - with pytest.raises(DependencyError): + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=[ + _make_urlopen_response(_make_zip_bytes(config_v1)), + _make_urlopen_response(_make_zip_bytes(config_v2)), + ], + ): install_dependency(mock_dependency) + config_path = ( + tmp_path + / "dependency-dir" + / DEPENDENCY_CONFIG_VERSION + / "test-repo" + / DEVSERVICES_DIR_NAME + / CONFIG_FILE_NAME + ) + assert config_path.exists() + first_content = config_path.read_text() - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() + install_dependency(mock_dependency) + second_content = config_path.read_text() - mock_ensure_config.assert_called_once() - - mock_run_command.assert_has_calls( - [ - mock.call( - [ - "git", - "clone", - "--filter=blob:none", - "--no-checkout", - f"file://{tmp_path / 'test-repo'}", - mock.ANY, - ], - cwd=mock.ANY, - ), - mock.call( - [ - "git", - "checkout", - "main", - ], - cwd=mock.ANY, - ), - ] - ) + assert first_content != second_content + assert "added-in-v2" in second_content -def test_install_dependency_rev_parse_local_commit_failure(tmp_path: Path) -> None: +def test_install_dependency_basic_with_new_file(tmp_path: Path) -> None: with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - mock_git_repo = create_mock_git_repo("basic_repo", tmp_path / "test-repo") mock_dependency = RemoteConfig( repo_name="test-repo", branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", + repo_link="https://github.com/getsentry/test-repo", ) + zip_v1 = _make_zip_bytes(BASIC_SERVICE_CONFIG) + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr( + "owner-repo-abc123/devservices/config.yml", + yaml.dump(BASIC_SERVICE_CONFIG), + ) + zf.writestr("owner-repo-abc123/devservices/extra.yml", "extra: true\n") + zip_v2 = buf.getvalue() - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - install_dependency(mock_dependency) - - assert ( + dest = ( tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "test-repo" / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - # Append a new line to the config file in the mock repo and commit the change - with open( - mock_git_repo / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME, - mode="a", - encoding="utf-8", - ) as f: - f.write("\nedited: true") - - run_git_command(["add", "."], cwd=mock_git_repo) - run_git_command(["commit", "-m", "Edit config file"], cwd=mock_git_repo) + ) - with ( - mock.patch( - "devservices.utils.dependencies._rev_parse", - ) as mock_rev_parse, - pytest.raises(DependencyError), + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=[ + _make_urlopen_response(zip_v1), + _make_urlopen_response(zip_v2), + ], ): - mock_rev_parse.side_effect = subprocess.CalledProcessError( - returncode=1, cmd="test" - ) install_dependency(mock_dependency) + assert not (dest / "extra.yml").exists() - mock_rev_parse.assert_called_once_with( - str( - tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "test-repo", - ), - "HEAD", - ) + install_dependency(mock_dependency) + assert (dest / CONFIG_FILE_NAME).exists() + assert (dest / "extra.yml").exists() -def test_install_dependency_rev_parse_remote_commit_failure(tmp_path: Path) -> None: +def test_install_dependency_basic_with_existing_dir(tmp_path: Path) -> None: with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - mock_git_repo = create_mock_git_repo("basic_repo", tmp_path / "test-repo") mock_dependency = RemoteConfig( repo_name="test-repo", branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", + repo_link="https://github.com/getsentry/test-repo", ) - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() + # Pre-create the destination with a stale file + dependency_dir = ( + tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "test-repo" + ) + dependency_dir.mkdir(parents=True, exist_ok=True) + (dependency_dir / "existing-file.txt").touch() + + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(_make_zip_bytes(BASIC_SERVICE_CONFIG)), + ): + install_dependency(mock_dependency) - install_dependency(mock_dependency) + assert not (dependency_dir / "existing-file.txt").exists() + assert (dependency_dir / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME).exists() - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - # Append a new line to the config file in the mock repo and commit the change - with open( - mock_git_repo / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME, - mode="a", - encoding="utf-8", - ) as f: - f.write("\nedited: true") +def test_install_dependency_basic_with_existing_invalid_dir(tmp_path: Path) -> None: + with mock.patch( + "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ): + mock_dependency = RemoteConfig( + repo_name="test-repo", + branch="main", + repo_link="https://github.com/getsentry/test-repo", + ) - run_git_command(["add", "."], cwd=mock_git_repo) - run_git_command(["commit", "-m", "Edit config file"], cwd=mock_git_repo) + dependency_dir = ( + tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "test-repo" + ) + dependency_dir.mkdir(parents=True, exist_ok=True) + (dependency_dir / ".git").mkdir() + (dependency_dir / "existing-file.txt").touch() - with ( - mock.patch( - "devservices.utils.dependencies._rev_parse", - ) as mock_rev_parse, - pytest.raises(DependencyError), + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(_make_zip_bytes(BASIC_SERVICE_CONFIG)), ): - mock_rev_parse.side_effect = [ - subprocess.CompletedProcess(args="", returncode=0, stderr=""), - subprocess.CalledProcessError(returncode=1, cmd="test"), - ] install_dependency(mock_dependency) - mock_rev_parse.assert_has_calls( - [ - mock.call( - str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo", - ), - "HEAD", - ), - mock.call( - str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo", - ), - "FETCH_HEAD", - ), - ] - ) + assert (dependency_dir / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME).exists() -def test_install_dependency_git_fetch_transient_failure(tmp_path: Path) -> None: +def test_install_dependency_basic_with_overwrite(tmp_path: Path) -> None: + """A second install overwrites any local edits to the destination.""" with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - mock_git_repo = create_mock_git_repo("basic_repo", tmp_path / "test-repo") mock_dependency = RemoteConfig( repo_name="test-repo", branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", + repo_link="https://github.com/getsentry/test-repo", ) - - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( + zip_bytes = _make_zip_bytes(BASIC_SERVICE_CONFIG) + config_path = ( tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "test-repo" / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME - ).exists() - - install_dependency(mock_dependency) - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() + ) - # Append a new line to the config file in the mock repo and commit the change - with open( - mock_git_repo / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME, - mode="a", - encoding="utf-8", - ) as f: - f.write("\nedited: true") + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(zip_bytes), + ): + install_dependency(mock_dependency) + original = config_path.read_text() - run_git_command(["add", "."], cwd=mock_git_repo) - run_git_command(["commit", "-m", "Edit config file"], cwd=mock_git_repo) + # Tamper with the installed file + config_path.write_text(original + "\nlocal_edit: true") - with ( - freeze_time("2024-05-14 00:00:00") as frozen_time, - mock.patch("devservices.utils.dependencies.time.sleep") as mock_sleep, - mock.patch( - "devservices.utils.dependencies._rev_parse", - return_value="123456", - ), - mock.patch( - "devservices.utils.dependencies._run_command" - ) as mock_run_command, - mock.patch( - "devservices.utils.dependencies._is_valid_repo", - return_value=True, - ), - mock.patch("devservices.utils.dependencies.GitConfigManager.ensure_config"), - ): - mock_sleep.side_effect = lambda _: frozen_time.tick(timedelta(seconds=1)) - mock_run_command.side_effect = [ - subprocess.CalledProcessError(returncode=1, cmd="test"), - subprocess.CalledProcessError(returncode=1, cmd="test"), - subprocess.CompletedProcess(args="", returncode=0, stdout=""), - subprocess.CompletedProcess(args="", returncode=0, stdout=""), - ] install_dependency(mock_dependency) - mock_run_command.assert_has_calls( - [ - mock.call( - [ - "git", - "fetch", - "origin", - "main", - "--filter=blob:none", - "--no-recurse-submodules", - ], - cwd=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - ), - stdout=subprocess.DEVNULL, - ), - mock.call( - [ - "git", - "fetch", - "origin", - "main", - "--filter=blob:none", - "--no-recurse-submodules", - ], - cwd=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - ), - stdout=subprocess.DEVNULL, - ), - mock.call( - [ - "git", - "fetch", - "origin", - "main", - "--filter=blob:none", - "--no-recurse-submodules", - ], - cwd=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - ), - stdout=subprocess.DEVNULL, - ), - ] - ) + assert config_path.read_text() == original -def test_install_dependency_git_fetch_failure_with_retries(tmp_path: Path) -> None: +def test_install_dependency_basic_noop_reinstall(tmp_path: Path) -> None: with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - mock_git_repo = create_mock_git_repo("basic_repo", tmp_path / "test-repo") mock_dependency = RemoteConfig( repo_name="test-repo", branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", + repo_link="https://github.com/getsentry/test-repo", ) - - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - install_dependency(mock_dependency) - - assert ( + zip_bytes = _make_zip_bytes(BASIC_SERVICE_CONFIG) + config_path = ( tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "test-repo" / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME - ).exists() + ) - # Append a new line to the config file in the mock repo and commit the change - with open( - mock_git_repo / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME, - mode="a", - encoding="utf-8", - ) as f: - f.write("\nedited: true") - - run_git_command(["add", "."], cwd=mock_git_repo) - run_git_command(["commit", "-m", "Edit config file"], cwd=mock_git_repo) - - with ( - freeze_time("2024-05-14 00:00:00") as frozen_time, - mock.patch("devservices.utils.dependencies.time.sleep") as mock_sleep, - mock.patch( - "devservices.utils.dependencies._run_command" - ) as mock_run_command, - mock.patch( - "devservices.utils.dependencies._is_valid_repo", - return_value=True, - ), - mock.patch("devservices.utils.dependencies.GitConfigManager.ensure_config"), - pytest.raises(DependencyError), - ): - mock_sleep.side_effect = lambda _: frozen_time.tick(timedelta(seconds=1)) - mock_run_command.side_effect = [ - subprocess.CalledProcessError(returncode=1, cmd="test"), - subprocess.CalledProcessError(returncode=1, cmd="test"), - subprocess.CalledProcessError(returncode=1, cmd="test"), - ] - install_dependency(mock_dependency) - - mock_run_command.assert_has_calls( - [ - mock.call( - [ - "git", - "fetch", - "origin", - "main", - "--filter=blob:none", - "--no-recurse-submodules", - ], - cwd=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - ), - stdout=subprocess.DEVNULL, - ), - mock.call( - [ - "git", - "fetch", - "origin", - "main", - "--filter=blob:none", - "--no-recurse-submodules", - ], - cwd=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - ), - stdout=subprocess.DEVNULL, - ), - mock.call( - [ - "git", - "fetch", - "origin", - "main", - "--filter=blob:none", - "--no-recurse-submodules", - ], - cwd=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - ), - stdout=subprocess.DEVNULL, - ), - ] - ) - - -def test_install_dependency_update_git_checkout_failure(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - mock_git_repo = create_mock_git_repo("basic_repo", tmp_path / "test-repo") - mock_dependency = RemoteConfig( - repo_name="test-repo", - branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", - ) - - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - install_dependency(mock_dependency) - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - # Append a new line to the config file in the mock repo and commit the change - with open( - mock_git_repo / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME, - mode="a", - encoding="utf-8", - ) as f: - f.write("\nedited: true") - - run_git_command(["add", "."], cwd=mock_git_repo) - run_git_command(["commit", "-m", "Edit config file"], cwd=mock_git_repo) - - with ( - mock.patch( - "devservices.utils.dependencies._rev_parse", - side_effect=["123456", "654321"], - ), - mock.patch( - "devservices.utils.dependencies._run_command" - ) as mock_run_command, - mock.patch( - "devservices.utils.dependencies._is_valid_repo", - return_value=True, - ), - mock.patch("devservices.utils.dependencies.GitConfigManager.ensure_config"), - pytest.raises(DependencyError), - ): - mock_run_command.side_effect = [ - subprocess.CompletedProcess(args="", returncode=0, stdout=""), - subprocess.CalledProcessError(returncode=1, cmd="test"), - ] - install_dependency(mock_dependency) - - mock_run_command.assert_has_calls( - [ - mock.call( - [ - "git", - "fetch", - "origin", - "main", - "--filter=blob:none", - "--no-recurse-submodules", - ], - cwd=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - ), - stdout=subprocess.DEVNULL, - ), - mock.call( - [ - "git", - "checkout", - "-f", - "FETCH_HEAD", - ], - cwd=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - ), - ), - ] - ) - - -def test_install_dependency_basic(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") - mock_dependency = RemoteConfig( - repo_name="test-repo", - branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", - ) - - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - install_dependency(mock_dependency) - - # Make sure that files outside of the devservices directory are not copied - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / "README.md" - ).exists() - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - # Check that the git config options are set correctly - for ( - git_config_option_key, - git_config_option_value, - ) in DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS.items(): - assert ( - subprocess.check_output( - ["git", "config", "--get", git_config_option_key], - cwd=tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo", - ) - .decode() - .strip() - == git_config_option_value - ) - - -def test_install_dependency_basic_with_edit(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - mock_git_repo = create_mock_git_repo("basic_repo", tmp_path / "test-repo") - mock_dependency = RemoteConfig( - repo_name="test-repo", - branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", - ) - - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - install_dependency(mock_dependency) - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - # Append a new line to the config file in the mock repo and commit the change - with open( - mock_git_repo / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME, mode="a" - ) as f: - f.write("\nedited: true") - - run_git_command(["add", "."], cwd=mock_git_repo) - run_git_command(["commit", "-m", "Edit config file"], cwd=mock_git_repo) - - install_dependency(mock_dependency) - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - # Check that the config file in the dependency directory has the new line appended - with open( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME, - mode="r", - ) as f: - assert f.read().endswith("\nedited: true") - - -def test_install_dependency_basic_with_new_tracked_file(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - mock_git_repo = create_mock_git_repo("basic_repo", tmp_path / "test-repo") - mock_dependency = RemoteConfig( - repo_name="test-repo", - branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", - ) - - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - install_dependency(mock_dependency) - - # Sanity check that the new file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / "new-file.txt" - ).exists() - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - # Add a new file to the mock repo and commit the change - with open(mock_git_repo / DEVSERVICES_DIR_NAME / "new-file.txt", mode="w") as f: - f.write("New test file") - run_git_command(["add", "."], cwd=mock_git_repo) - run_git_command(["commit", "-m", "Add new file"], cwd=mock_git_repo) - - install_dependency(mock_dependency) - - # Sanity check that the existing config file is still there - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - # Check that the new file is now in the dependency directory - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / "new-file.txt" - ).exists() - - -def test_install_dependency_basic_with_existing_dir(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") - mock_dependency = RemoteConfig( - repo_name="test-repo", - branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", - ) - - # Create the dependency directory and populate it - dependency_dir = ( - tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "test-repo" - ) - dependency_dir.mkdir(parents=True, exist_ok=True) - (dependency_dir / "existing-file.txt").touch() - - install_dependency(mock_dependency) - - # Make sure that files outside of the devservices directory are not copied - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / "README.md" - ).exists() - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - -def test_install_dependency_basic_with_existing_invalid_repo(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") - mock_dependency = RemoteConfig( - repo_name="test-repo", - branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", - ) - - # Create the dependency directory and populate it - dependency_dir = ( - tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "test-repo" - ) - dependency_dir.mkdir(parents=True, exist_ok=True) - dependency_git_dir = dependency_dir / ".git" - dependency_git_dir.mkdir(parents=True, exist_ok=True) - (dependency_dir / "existing-file.txt").touch() - - install_dependency(mock_dependency) - - # Make sure that files outside of the devservices directory are not copied - assert not (tmp_path / "dependency-dir" / "test-repo" / "README.md").exists() - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - -def test_install_dependency_basic_with_existing_repo_conflicts(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - mock_git_repo = create_mock_git_repo("basic_repo", tmp_path / "test-repo") - mock_dependency = RemoteConfig( - repo_name="test-repo", - branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", - ) - - install_dependency(mock_dependency) - - # Make sure that files outside of the devservices directory are not copied - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / "README.md" - ).exists() - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - # Append a new line to the config file in the mock repo and commit the change - with open( - mock_git_repo / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME, mode="a" - ) as f: - f.write("\nedited: true") - - run_git_command(["add", "."], cwd=mock_git_repo) - run_git_command(["commit", "-m", "Edit config file"], cwd=mock_git_repo) - - # Edit the working copy and leave changes unstaged - with open( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME, - mode="a", - ) as f: - f.write("\nConflict") - - install_dependency(mock_dependency) - - # Check that the config file in the dependency directory has the new line appended - with open( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME, - mode="r", - ) as f: - assert f.read().endswith("\nedited: true") - - -def test_install_dependency_basic_with_corrupted_repo(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - mock_git_repo = create_mock_git_repo("basic_repo", tmp_path / "test-repo") - mock_dependency = RemoteConfig( - repo_name="test-repo", - branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", - ) - - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - install_dependency(mock_dependency) - - # Sanity check that the new file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / "new-file.txt" - ).exists() - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - # Corrupt the git repository by deleting the .git directory - shutil.rmtree(mock_git_repo / ".git") - - with pytest.raises(DependencyError): - install_dependency(mock_dependency) - - -def test_install_dependency_basic_with_noop_update(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") - mock_dependency = RemoteConfig( - repo_name="test-repo", - branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", - ) - - # Sanity check that the config file is not in the dependency directory (yet) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - install_dependency(mock_dependency) - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - # Check if the local repo is up-to-date - install_dependency(mock_dependency) - - # Sanity check that the existing config file is still there - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - -def test_install_dependency_basic_git_config_self_fix(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - create_mock_git_repo("basic_repo", tmp_path / "test-repo") - mock_dependency = RemoteConfig( - repo_name="test-repo", - branch="main", - repo_link=f"file://{tmp_path / 'test-repo'}", - ) - - install_dependency(mock_dependency) - - # Check that the git config options are set correctly - for ( - git_config_option_key, - git_config_option_value, - ) in DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS.items(): - assert ( - subprocess.check_output( - ["git", "config", "--get", git_config_option_key], - cwd=tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo", - ) - .decode() - .strip() - == git_config_option_value - ) - - # Mess up the git config by setting the wrong values - for ( - git_config_option_key, - git_config_option_value, - ) in DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS.items(): - run_git_command( - ["config", git_config_option_key, "wrong-value"], - cwd=tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo", - ) - - for ( - git_config_option_key, - git_config_option_value, - ) in DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS.items(): - assert ( - subprocess.check_output( - ["git", "config", "--get", git_config_option_key], - cwd=tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo", - ) - .decode() - .strip() - != git_config_option_value - ) - - install_dependency(mock_dependency) - - # Check that the git config options are set correctly - for ( - git_config_option_key, - git_config_option_value, - ) in DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS.items(): - assert ( - subprocess.check_output( - ["git", "config", "--get", git_config_option_key], - cwd=tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "test-repo", - ) - .decode() - .strip() - == git_config_option_value - ) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(zip_bytes), + ): + install_dependency(mock_dependency) + assert config_path.exists() + install_dependency(mock_dependency) + assert config_path.exists() def test_install_dependency_nested_dependency(tmp_path: Path) -> None: @@ -1488,142 +606,7 @@ def test_install_dependency_nested_dependency(tmp_path: Path) -> None: "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - nested_repo_path = create_mock_git_repo("basic_repo", tmp_path / "nested-repo") - main_repo_path = create_mock_git_repo("blank_repo", tmp_path / "main-repo") - mock_git_repo_config = { - "x-sentry-service-config": { - "version": 0.1, - "service_name": "complex", - "dependencies": { - "nested-repo": { - "description": "nested dependency", - "remote": { - "repo_name": "nested-repo", - "repo_link": f"file://{nested_repo_path}", - "branch": "main", - }, - } - }, - "modes": {"default": ["nested-repo"]}, - } - } - create_config_file(main_repo_path, mock_git_repo_config) - run_git_command(["add", "."], cwd=main_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=main_repo_path) - - main_repo_dependency = RemoteConfig( - repo_name="main-repo", - branch="main", - repo_link=f"file://{main_repo_path}", - ) - - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "main-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "nested-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - installed_remote_dependencies = install_dependency(main_repo_dependency) - - assert installed_remote_dependencies == set( - [ - InstalledRemoteDependency( - service_name="basic", - repo_path=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "nested-repo" - ), - ), - InstalledRemoteDependency( - service_name="complex", - repo_path=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "main-repo" - ), - ), - ] - ) - - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "main-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "nested-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - - -def test_install_dependency_nested_dependency_missing_nested_dependency( - tmp_path: Path, -) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - main_repo_path = create_mock_git_repo("blank_repo", tmp_path / "main-repo") - mock_git_repo_config = { - "x-sentry-service-config": { - "version": 0.1, - "service_name": "complex", - "dependencies": { - "nested-repo": { - "description": "nested dependency", - "remote": { - "repo_name": "nested-repo", - "repo_link": "invalid-link", - "branch": "main", - }, - } - }, - "modes": {"default": ["nested-repo"]}, - } - } - create_config_file(main_repo_path, mock_git_repo_config) - run_git_command(["add", "."], cwd=main_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=main_repo_path) - - main_repo_dependency = RemoteConfig( - repo_name="main-repo", - branch="main", - repo_link=f"file://{main_repo_path}", - ) - - with pytest.raises(DependencyError): - install_dependency(main_repo_dependency) - - -def test_install_dependency_nested_dependency_with_edits(tmp_path: Path) -> None: - with mock.patch( - "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", - str(tmp_path / "dependency-dir"), - ): - nested_repo_path = create_mock_git_repo("basic_repo", tmp_path / "nested-repo") - main_repo_path = create_mock_git_repo("blank_repo", tmp_path / "main-repo") - mock_git_repo_config = { + main_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "complex", @@ -1632,7 +615,7 @@ def test_install_dependency_nested_dependency_with_edits(tmp_path: Path) -> None "description": "nested dependency", "remote": { "repo_name": "nested-repo", - "repo_link": f"file://{nested_repo_path}", + "repo_link": "https://github.com/getsentry/nested-repo", "branch": "main", }, } @@ -1640,57 +623,43 @@ def test_install_dependency_nested_dependency_with_edits(tmp_path: Path) -> None "modes": {"default": ["nested-repo"]}, } } - create_config_file(main_repo_path, mock_git_repo_config) - run_git_command(["add", "."], cwd=main_repo_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=main_repo_path) + main_zip = _make_zip_bytes(main_config) + nested_zip = _make_zip_bytes(BASIC_SERVICE_CONFIG) main_repo_dependency = RemoteConfig( repo_name="main-repo", branch="main", - repo_link=f"file://{main_repo_path}", + repo_link="https://github.com/getsentry/main-repo", ) - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "main-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - assert not ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "nested-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=_url_dispatch( + {"main-repo": main_zip, "nested-repo": nested_zip} + ), + ): + installed = install_dependency(main_repo_dependency) - installed_remote_dependencies = install_dependency(main_repo_dependency) - - assert installed_remote_dependencies == set( - [ - InstalledRemoteDependency( - service_name="basic", - repo_path=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "nested-repo" - ), + assert installed == { + InstalledRemoteDependency( + service_name="basic", + repo_path=str( + tmp_path + / "dependency-dir" + / DEPENDENCY_CONFIG_VERSION + / "nested-repo" ), - InstalledRemoteDependency( - service_name="complex", - repo_path=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "main-repo" - ), + ), + InstalledRemoteDependency( + service_name="complex", + repo_path=str( + tmp_path + / "dependency-dir" + / DEPENDENCY_CONFIG_VERSION + / "main-repo" ), - ] - ) + ), + } assert ( tmp_path @@ -1709,59 +678,54 @@ def test_install_dependency_nested_dependency_with_edits(tmp_path: Path) -> None / CONFIG_FILE_NAME ).exists() - with open( - main_repo_path / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME, mode="a" - ) as f: - f.write("\nedited: true") - - run_git_command(["add", "."], cwd=main_repo_path) - run_git_command(["commit", "-m", "Edit config file"], cwd=main_repo_path) - - with open( - nested_repo_path / DEVSERVICES_DIR_NAME / CONFIG_FILE_NAME, mode="a" - ) as f: - f.write("\nedited: true") - - run_git_command(["add", "."], cwd=nested_repo_path) - run_git_command(["commit", "-m", "Edit config file"], cwd=nested_repo_path) - install_dependency(main_repo_dependency) +def test_install_dependency_nested_dependency_missing_nested_dependency( + tmp_path: Path, +) -> None: + with mock.patch( + "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", + str(tmp_path / "dependency-dir"), + ): + main_config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "complex", + "dependencies": { + "nested-repo": { + "description": "nested dependency", + "remote": { + "repo_name": "nested-repo", + "repo_link": "invalid-link", + "branch": "main", + }, + } + }, + "modes": {"default": ["nested-repo"]}, + } + } - with open( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "main-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME, - mode="r", - ) as f: - assert f.read().endswith("\nedited: true") + main_repo_dependency = RemoteConfig( + repo_name="main-repo", + branch="main", + repo_link="https://github.com/getsentry/main-repo", + ) - with open( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "nested-repo" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME, - mode="r", - ) as f: - assert f.read().endswith("\nedited: true") + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(_make_zip_bytes(main_config)), + ): + with pytest.raises(DependencyError): + install_dependency(main_repo_dependency) def test_install_dependency_does_not_install_unnecessary_dependencies( tmp_path: Path, ) -> None: - """ - Test that installing a dependency does not install nested dependencies not in the modes. - """ + """Installing a dependency only pulls in nested deps in the active mode.""" with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - repo_a_path = create_mock_git_repo("blank_repo", tmp_path / "repo-a") - repo_b_path = create_mock_git_repo("basic_repo", tmp_path / "repo-b") repo_a_config = { "x-sentry-service-config": { "version": 0.1, @@ -1771,7 +735,7 @@ def test_install_dependency_does_not_install_unnecessary_dependencies( "description": "nested dependency", "remote": { "repo_name": "repo-b", - "repo_link": f"file://{repo_b_path}", + "repo_link": "https://github.com/getsentry/repo-b", "branch": "main", }, }, @@ -1787,54 +751,46 @@ def test_install_dependency_does_not_install_unnecessary_dependencies( "modes": {"default": ["repo-b"], "other": ["unnecessary-repo"]}, }, } - create_config_file(repo_a_path, repo_a_config) - run_git_command(["add", "."], cwd=repo_a_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=repo_a_path) - repo_a_dependency = RemoteConfig( - repo_name="repo-a", - branch="main", - repo_link=f"file://{repo_a_path}", - ) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=_url_dispatch( + { + "repo-a": _make_zip_bytes(repo_a_config), + "repo-b": _make_zip_bytes(BASIC_SERVICE_CONFIG), + } + ), + ): + installed = install_dependency( + RemoteConfig( + repo_name="repo-a", + branch="main", + repo_link="https://github.com/getsentry/repo-a", + ) + ) - installed_remote_dependencies = install_dependency(repo_a_dependency) - - assert installed_remote_dependencies == set( - [ - InstalledRemoteDependency( - service_name="repo-a", - repo_path=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "repo-a" - ), + assert installed == { + InstalledRemoteDependency( + service_name="repo-a", + repo_path=str( + tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "repo-a" ), - InstalledRemoteDependency( - service_name="basic", - repo_path=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "repo-b" - ), + ), + InstalledRemoteDependency( + service_name="basic", + repo_path=str( + tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "repo-b" ), - ] - ) + ), + } -def test_install_dependency_invalid_mode( - tmp_path: Path, -) -> None: - """ - Test that installing a dependency with an invalid mode raises an error. - """ +def test_install_dependency_invalid_mode(tmp_path: Path) -> None: + """Installing with an invalid mode raises ModeDoesNotExistError.""" with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - repo_a_path = create_mock_git_repo("blank_repo", tmp_path / "repo-a") - repo_b_path = create_mock_git_repo("basic_repo", tmp_path / "repo-b") repo_a_config = { "x-sentry-service-config": { "version": 0.1, @@ -1844,7 +800,7 @@ def test_install_dependency_invalid_mode( "description": "nested dependency", "remote": { "repo_name": "repo-b", - "repo_link": f"file://{repo_b_path}", + "repo_link": "https://github.com/getsentry/repo-b", "branch": "main", }, }, @@ -1852,31 +808,28 @@ def test_install_dependency_invalid_mode( "modes": {"default": ["repo-b"]}, }, } - create_config_file(repo_a_path, repo_a_config) - run_git_command(["add", "."], cwd=repo_a_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=repo_a_path) - - repo_a_dependency = RemoteConfig( - repo_name="repo-a", - branch="main", - repo_link=f"file://{repo_a_path}", - mode="invalid-mode", - ) - with pytest.raises(ModeDoesNotExistError): - install_dependency(repo_a_dependency) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response(_make_zip_bytes(repo_a_config)), + ): + with pytest.raises(ModeDoesNotExistError): + install_dependency( + RemoteConfig( + repo_name="repo-a", + branch="main", + repo_link="https://github.com/getsentry/repo-a", + mode="invalid-mode", + ) + ) def test_install_dependency_invalid_nested_dependency(tmp_path: Path) -> None: - """ - Test that installing a nested dependency with an invalid config raises an error. - """ + """Installing a dependency whose nested dep has an invalid config raises InvalidDependencyConfigError.""" with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - repo_a_path = create_mock_git_repo("blank_repo", tmp_path / "repo-a") - repo_c_path = create_mock_git_repo("invalid_repo", tmp_path / "repo-c") repo_a_config = { "x-sentry-service-config": { "version": 0.1, @@ -1886,7 +839,7 @@ def test_install_dependency_invalid_nested_dependency(tmp_path: Path) -> None: "description": "nested dependency", "remote": { "repo_name": "repo-c", - "repo_link": f"file://{repo_c_path}", + "repo_link": "https://github.com/getsentry/repo-c", "branch": "main", }, }, @@ -1894,32 +847,33 @@ def test_install_dependency_invalid_nested_dependency(tmp_path: Path) -> None: "modes": {"default": ["repo-c"]}, } } - create_config_file(repo_a_path, repo_a_config) - run_git_command(["add", "."], cwd=repo_a_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=repo_a_path) - - repo_a_dependency = RemoteConfig( - repo_name="repo-a", - branch="main", - repo_link=f"file://{repo_a_path}", - ) - with pytest.raises(InvalidDependencyConfigError): - install_dependency(repo_a_dependency) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=_url_dispatch( + { + "repo-a": _make_zip_bytes(repo_a_config), + "repo-c": _make_zip_bytes(INVALID_SERVICE_CONFIG_YAML), + } + ), + ): + with pytest.raises(InvalidDependencyConfigError): + install_dependency( + RemoteConfig( + repo_name="repo-a", + branch="main", + repo_link="https://github.com/getsentry/repo-a", + ) + ) def test_install_dependencies_nested_dependency_file_contention(tmp_path: Path) -> None: - """ - Test that installing multiple dependencies that share a nested dependency - does not cause file contention issues. - """ + """Concurrent installs of repos sharing a nested dep succeed without corruption.""" with mock.patch( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - repo_a_path = create_mock_git_repo("blank_repo", tmp_path / "repo-a") - repo_b_path = create_mock_git_repo("blank_repo", tmp_path / "repo-b") - repo_c_path = create_mock_git_repo("basic_repo", tmp_path / "repo-c") + shared_config = BASIC_SERVICE_CONFIG repo_a_config = { "x-sentry-service-config": { "version": 0.1, @@ -1929,7 +883,7 @@ def test_install_dependencies_nested_dependency_file_contention(tmp_path: Path) "description": "nested dependency", "remote": { "repo_name": "repo-c", - "repo_link": f"file://{repo_c_path}", + "repo_link": "https://github.com/getsentry/repo-c", "branch": "main", }, }, @@ -1937,10 +891,6 @@ def test_install_dependencies_nested_dependency_file_contention(tmp_path: Path) "modes": {"default": ["repo-c"]}, } } - create_config_file(repo_a_path, repo_a_config) - run_git_command(["add", "."], cwd=repo_a_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=repo_a_path) - repo_b_config = { "x-sentry-service-config": { "version": 0.1, @@ -1950,7 +900,7 @@ def test_install_dependencies_nested_dependency_file_contention(tmp_path: Path) "description": "nested dependency", "remote": { "repo_name": "repo-c", - "repo_link": f"file://{repo_c_path}", + "repo_link": "https://github.com/getsentry/repo-c", "branch": "main", }, }, @@ -1958,88 +908,68 @@ def test_install_dependencies_nested_dependency_file_contention(tmp_path: Path) "modes": {"default": ["repo-c"]}, } } - create_config_file(repo_b_path, repo_b_config) - run_git_command(["add", "."], cwd=repo_b_path) - run_git_command(["commit", "-m", "Add devservices config"], cwd=repo_b_path) - repo_a_dependency = Dependency( + repo_a_dep = Dependency( description="repo a", remote=RemoteConfig( repo_name="repo-a", branch="main", - repo_link=f"file://{repo_a_path}", + repo_link="https://github.com/getsentry/repo-a", ), dependency_type=DependencyType.SERVICE, ) - repo_b_dependency = Dependency( + repo_b_dep = Dependency( description="repo b", remote=RemoteConfig( repo_name="repo-b", branch="main", - repo_link=f"file://{repo_b_path}", + repo_link="https://github.com/getsentry/repo-b", ), dependency_type=DependencyType.SERVICE, ) - dependencies = [repo_a_dependency, repo_b_dependency] - - installed_remote_dependencies = install_dependencies(dependencies) - - assert installed_remote_dependencies == set( - [ - InstalledRemoteDependency( - service_name="repo-a", - repo_path=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "repo-a" - ), + + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=_url_dispatch( + { + "repo-a": _make_zip_bytes(repo_a_config), + "repo-b": _make_zip_bytes(repo_b_config), + "repo-c": _make_zip_bytes(shared_config), + } + ), + ): + installed = install_dependencies([repo_a_dep, repo_b_dep]) + + assert installed == { + InstalledRemoteDependency( + service_name="repo-a", + repo_path=str( + tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "repo-a" ), - InstalledRemoteDependency( - service_name="repo-b", - repo_path=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "repo-b" - ), + ), + InstalledRemoteDependency( + service_name="repo-b", + repo_path=str( + tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "repo-b" ), - InstalledRemoteDependency( - service_name="basic", - repo_path=str( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "repo-c" - ), + ), + InstalledRemoteDependency( + service_name="basic", + repo_path=str( + tmp_path / "dependency-dir" / DEPENDENCY_CONFIG_VERSION / "repo-c" ), - ] - ) + ), + } - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "repo-a" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "repo-b" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() - assert ( - tmp_path - / "dependency-dir" - / DEPENDENCY_CONFIG_VERSION - / "repo-c" - / DEVSERVICES_DIR_NAME - / CONFIG_FILE_NAME - ).exists() + for repo_name in ("repo-a", "repo-b", "repo-c"): + assert ( + tmp_path + / "dependency-dir" + / DEPENDENCY_CONFIG_VERSION + / repo_name + / DEVSERVICES_DIR_NAME + / CONFIG_FILE_NAME + ).exists() @mock.patch( @@ -2101,15 +1031,13 @@ def test_get_non_shared_remote_dependencies_no_shared_dependencies( ) non_shared_remote_dependencies = get_non_shared_remote_dependencies( service_to_stop, - set( - [ - InstalledRemoteDependency( - service_name="dependency-1", - repo_path="/path/to/dependency-1", - mode="default", - ) - ] - ), + { + InstalledRemoteDependency( + service_name="dependency-1", + repo_path="/path/to/dependency-1", + mode="default", + ) + }, exclude_local=exclude_local, ) assert len(non_shared_remote_dependencies) == 1 @@ -2128,15 +1056,13 @@ def test_get_non_shared_remote_dependencies_no_shared_dependencies( ) @mock.patch( "devservices.utils.dependencies.get_installed_remote_dependencies", - return_value=set( - [ - InstalledRemoteDependency( - service_name="dependency-1", - repo_path="/path/to/dependency-1", - mode="default", - ) - ] - ), + return_value={ + InstalledRemoteDependency( + service_name="dependency-1", + repo_path="/path/to/dependency-1", + mode="default", + ) + }, ) @mock.patch( "devservices.utils.dependencies.find_matching_service", @@ -2199,15 +1125,13 @@ def test_get_non_shared_remote_dependencies_shared_dependencies( ) non_shared_remote_dependencies = get_non_shared_remote_dependencies( service_to_stop, - set( - [ - InstalledRemoteDependency( - service_name="dependency-1", - repo_path="/path/to/dependency-1", - mode="default", - ) - ] - ), + { + InstalledRemoteDependency( + service_name="dependency-1", + repo_path="/path/to/dependency-1", + mode="default", + ) + }, exclude_local=exclude_local, ) assert len(non_shared_remote_dependencies) == 0 @@ -2234,15 +1158,13 @@ def test_get_non_shared_remote_dependencies_shared_dependencies( ) @mock.patch( "devservices.utils.dependencies.get_installed_remote_dependencies", - return_value=set( - [ - InstalledRemoteDependency( - service_name="dependency-1", - repo_path="/path/to/dependency-1", - mode="default", - ) - ] - ), + return_value={ + InstalledRemoteDependency( + service_name="dependency-1", + repo_path="/path/to/dependency-1", + mode="default", + ) + }, ) @mock.patch( "devservices.utils.dependencies.find_matching_service", @@ -2324,20 +1246,18 @@ def test_get_non_shared_remote_dependencies_nested_shared_dependencies( ) non_shared_remote_dependencies = get_non_shared_remote_dependencies( service_to_stop, - set( - [ - InstalledRemoteDependency( - service_name="dependency-1", - repo_path="/path/to/dependency-1", - mode="default", - ), - InstalledRemoteDependency( - service_name="dependency-2", - repo_path="/path/to/dependency-2", - mode="default", - ), - ] - ), + { + InstalledRemoteDependency( + service_name="dependency-1", + repo_path="/path/to/dependency-1", + mode="default", + ), + InstalledRemoteDependency( + service_name="dependency-2", + repo_path="/path/to/dependency-2", + mode="default", + ), + }, exclude_local=exclude_local, ) assert non_shared_remote_dependencies == { @@ -2348,7 +1268,7 @@ def test_get_non_shared_remote_dependencies_nested_shared_dependencies( ) } mock_find_matching_service.assert_called_once_with("service-2") - # dependency-4 is not installed, so it should not be passed to get_installed_remote_dependencies + # dependency-4 is not in the active mode so it should not be passed to get_installed_remote_dependencies mock_get_installed_remote_dependencies.assert_called_once_with( [ Dependency( @@ -2443,35 +1363,32 @@ def test_get_non_shared_remote_dependencies_with_local_runtime_dependency( ) non_shared_remote_dependencies = get_non_shared_remote_dependencies( service_to_stop, - set( - [ - InstalledRemoteDependency( - service_name="dependency-1", - repo_path="/path/to/dependency-1", - mode="default", - ), - InstalledRemoteDependency( - service_name="service-2", - repo_path="/path/to/service-2", - mode="default", - ), - ] - ), + { + InstalledRemoteDependency( + service_name="dependency-1", + repo_path="/path/to/dependency-1", + mode="default", + ), + InstalledRemoteDependency( + service_name="service-2", + repo_path="/path/to/service-2", + mode="default", + ), + }, exclude_local=exclude_local, ) - # If exclude_local is True, we don't include service-2 in the list of non-shared remote dependencies - # because it is a service that is a dependency of service-1, but is also running alongside it, so in we don't want to bring it down, - # unless we explictly want to bring down dependencies with local runtimes, via the --exclude-local flag. - expected_non_shared_remote_dependencies = ( - { + # If exclude_local is True, service-2 is excluded from non-shared deps because it + # is running with a local runtime and is depended upon by service-1. + if exclude_local: + assert non_shared_remote_dependencies == { InstalledRemoteDependency( service_name="dependency-1", repo_path="/path/to/dependency-1", mode="default", ) } - if exclude_local - else { + else: + assert non_shared_remote_dependencies == { InstalledRemoteDependency( service_name="dependency-1", repo_path="/path/to/dependency-1", @@ -2483,22 +1400,6 @@ def test_get_non_shared_remote_dependencies_with_local_runtime_dependency( mode="default", ), } - ) - assert non_shared_remote_dependencies == expected_non_shared_remote_dependencies - mock_find_matching_service.assert_called_once_with("service-2") - mock_get_installed_remote_dependencies.assert_called_once_with( - [ - Dependency( - description="dependency-3", - remote=RemoteConfig( - repo_name="dependency-3", - repo_link="file://path/to/dependency-3", - branch="main", - ), - dependency_type=DependencyType.SERVICE, - ) - ] - ) @mock.patch("devservices.utils.dependencies.install_dependencies", return_value=[]) @@ -2621,12 +1522,8 @@ def test_install_and_verify_dependencies_mode_does_not_exist(tmp_path: Path) -> install_and_verify_dependencies(service, modes=["unknown-mode"]) -def test_construct_dependency_graph_simple( - tmp_path: Path, -) -> None: - dependency_service_repo_path = tmp_path / "dependency-service-repo" - create_mock_git_repo("blank_repo", dependency_service_repo_path) - dependency_service_repo_config = { +def test_construct_dependency_graph_simple(tmp_path: Path) -> None: + dependency_service_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "dependency-1", @@ -2643,11 +1540,6 @@ def test_construct_dependency_graph_simple( }, }, } - create_config_file(dependency_service_repo_path, dependency_service_repo_config) - run_git_command(["add", "."], cwd=dependency_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=dependency_service_repo_path - ) service = Service( name="test-service", repo_path="/path/to/test-service", @@ -2659,15 +1551,13 @@ def test_construct_dependency_graph_simple( description="dependency-1", remote=RemoteConfig( repo_name="dependency-1", - repo_link=f"file://{dependency_service_repo_path}", + repo_link="https://github.com/getsentry/dependency-1", branch="main", ), dependency_type=DependencyType.SERVICE, ), }, - modes={ - "default": ["dependency-1"], - }, + modes={"default": ["dependency-1"]}, ), ) @@ -2675,7 +1565,14 @@ def test_construct_dependency_graph_simple( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - install_and_verify_dependencies(service) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + return_value=_make_urlopen_response( + _make_zip_bytes(dependency_service_config) + ), + ): + install_and_verify_dependencies(service) + dependency_graph = construct_dependency_graph(service, ["default"]) assert dependency_graph.graph == { DependencyNode( @@ -2704,14 +1601,8 @@ def test_construct_dependency_graph_simple( ] -def test_construct_dependency_graph_one_nested_dependency( - tmp_path: Path, -) -> None: - parent_service_repo_path = tmp_path / "parent-service-repo" - child_service_repo_path = tmp_path / "child-service-repo" - create_mock_git_repo("blank_repo", parent_service_repo_path) - create_mock_git_repo("blank_repo", child_service_repo_path) - parent_service_repo_config = { +def test_construct_dependency_graph_one_nested_dependency(tmp_path: Path) -> None: + parent_service_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "parent-service", @@ -2720,7 +1611,7 @@ def test_construct_dependency_graph_one_nested_dependency( "description": "child-service", "remote": { "repo_name": "child-service", - "repo_link": f"file://{child_service_repo_path}", + "repo_link": "https://github.com/getsentry/child-service", "branch": "main", }, }, @@ -2736,7 +1627,7 @@ def test_construct_dependency_graph_one_nested_dependency( }, }, } - child_service_repo_config = { + child_service_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "child-service", @@ -2753,16 +1644,6 @@ def test_construct_dependency_graph_one_nested_dependency( }, }, } - create_config_file(parent_service_repo_path, parent_service_repo_config) - create_config_file(child_service_repo_path, child_service_repo_config) - run_git_command(["add", "."], cwd=parent_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=parent_service_repo_path - ) - run_git_command(["add", "."], cwd=child_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=child_service_repo_path - ) service = Service( name="grandparent-service", repo_path="/path/to/grandparent-service", @@ -2774,7 +1655,7 @@ def test_construct_dependency_graph_one_nested_dependency( description="parent-service", remote=RemoteConfig( repo_name="parent-service", - repo_link=f"file://{parent_service_repo_path}", + repo_link="https://github.com/getsentry/parent-service", branch="main", ), dependency_type=DependencyType.SERVICE, @@ -2784,9 +1665,7 @@ def test_construct_dependency_graph_one_nested_dependency( dependency_type=DependencyType.COMPOSE, ), }, - modes={ - "default": ["parent-service", "grandparent-service"], - }, + modes={"default": ["parent-service", "grandparent-service"]}, ), ) @@ -2794,7 +1673,17 @@ def test_construct_dependency_graph_one_nested_dependency( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - install_and_verify_dependencies(service) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=_url_dispatch( + { + "parent-service": _make_zip_bytes(parent_service_config), + "child-service": _make_zip_bytes(child_service_config), + } + ), + ): + install_and_verify_dependencies(service) + dependency_graph = construct_dependency_graph(service, ["default"]) assert dependency_graph.graph == { DependencyNode( @@ -2827,10 +1716,10 @@ def test_construct_dependency_graph_one_nested_dependency( name="grandparent-service", dependency_type=DependencyType.SERVICE ): { DependencyNode( - name="parent-service", dependency_type=DependencyType.SERVICE + name="grandparent-service", dependency_type=DependencyType.COMPOSE ), DependencyNode( - name="grandparent-service", dependency_type=DependencyType.COMPOSE + name="parent-service", dependency_type=DependencyType.SERVICE ), }, } @@ -2843,6 +1732,7 @@ def test_construct_dependency_graph_one_nested_dependency( name="parent-service", dependency_type=DependencyType.SERVICE ) ), "Child service should come before parent service in the starting order" + assert starting_order.index( DependencyNode( name="parent-service", dependency_type=DependencyType.SERVICE @@ -2854,14 +1744,8 @@ def test_construct_dependency_graph_one_nested_dependency( ), "Parent service should come before grandparent service in the starting order" -def test_construct_dependency_graph_shared_dependency( - tmp_path: Path, -) -> None: - parent_service_repo_path = tmp_path / "parent-service-repo" - child_service_repo_path = tmp_path / "child-service-repo" - create_mock_git_repo("blank_repo", parent_service_repo_path) - create_mock_git_repo("blank_repo", child_service_repo_path) - parent_service_repo_config = { +def test_construct_dependency_graph_shared_dependency(tmp_path: Path) -> None: + parent_service_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "parent-service", @@ -2870,7 +1754,7 @@ def test_construct_dependency_graph_shared_dependency( "description": "child-service", "remote": { "repo_name": "child-service", - "repo_link": f"file://{child_service_repo_path}", + "repo_link": "https://github.com/getsentry/child-service", "branch": "main", }, }, @@ -2886,7 +1770,7 @@ def test_construct_dependency_graph_shared_dependency( }, }, } - child_service_repo_config = { + child_service_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "child-service", @@ -2903,16 +1787,6 @@ def test_construct_dependency_graph_shared_dependency( }, }, } - create_config_file(parent_service_repo_path, parent_service_repo_config) - create_config_file(child_service_repo_path, child_service_repo_config) - run_git_command(["add", "."], cwd=parent_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=parent_service_repo_path - ) - run_git_command(["add", "."], cwd=child_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=child_service_repo_path - ) service = Service( name="grandparent-service", repo_path="/path/to/grandparent-service", @@ -2924,7 +1798,7 @@ def test_construct_dependency_graph_shared_dependency( description="parent-service", remote=RemoteConfig( repo_name="parent-service", - repo_link=f"file://{parent_service_repo_path}", + repo_link="https://github.com/getsentry/parent-service", branch="main", ), dependency_type=DependencyType.SERVICE, @@ -2937,7 +1811,7 @@ def test_construct_dependency_graph_shared_dependency( description="child-service", remote=RemoteConfig( repo_name="child-service", - repo_link=f"file://{child_service_repo_path}", + repo_link="https://github.com/getsentry/child-service", branch="main", ), dependency_type=DependencyType.SERVICE, @@ -2953,7 +1827,17 @@ def test_construct_dependency_graph_shared_dependency( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - install_and_verify_dependencies(service) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=_url_dispatch( + { + "parent-service": _make_zip_bytes(parent_service_config), + "child-service": _make_zip_bytes(child_service_config), + } + ), + ): + install_and_verify_dependencies(service) + dependency_graph = construct_dependency_graph(service, ["default"]) assert dependency_graph.graph == { DependencyNode( @@ -3017,14 +1901,8 @@ def test_construct_dependency_graph_shared_dependency( ), "Parent service should come before grandparent service in the starting order" -def test_construct_dependency_graph_non_self_reference( - tmp_path: Path, -) -> None: - parent_service_repo_path = tmp_path / "parent-service-repo" - child_service_repo_path = tmp_path / "child-service-repo" - create_mock_git_repo("blank_repo", parent_service_repo_path) - create_mock_git_repo("blank_repo", child_service_repo_path) - parent_service_repo_config = { +def test_construct_dependency_graph_non_self_reference(tmp_path: Path) -> None: + parent_service_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "parent-service", @@ -3033,7 +1911,7 @@ def test_construct_dependency_graph_non_self_reference( "description": "child-service", "remote": { "repo_name": "child-service", - "repo_link": f"file://{child_service_repo_path}", + "repo_link": "https://github.com/getsentry/child-service", "branch": "main", }, }, @@ -3049,7 +1927,7 @@ def test_construct_dependency_graph_non_self_reference( }, }, } - child_service_repo_config = { + child_service_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "child-service", @@ -3066,16 +1944,6 @@ def test_construct_dependency_graph_non_self_reference( }, }, } - create_config_file(parent_service_repo_path, parent_service_repo_config) - create_config_file(child_service_repo_path, child_service_repo_config) - run_git_command(["add", "."], cwd=parent_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=parent_service_repo_path - ) - run_git_command(["add", "."], cwd=child_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=child_service_repo_path - ) service = Service( name="grandparent-service", repo_path="/path/to/grandparent-service", @@ -3087,7 +1955,7 @@ def test_construct_dependency_graph_non_self_reference( description="parent-service", remote=RemoteConfig( repo_name="parent-service", - repo_link=f"file://{parent_service_repo_path}", + repo_link="https://github.com/getsentry/parent-service", branch="main", ), dependency_type=DependencyType.SERVICE, @@ -3097,9 +1965,7 @@ def test_construct_dependency_graph_non_self_reference( dependency_type=DependencyType.COMPOSE, ), }, - modes={ - "default": ["parent-service", "grandparent-service-container"], - }, + modes={"default": ["parent-service", "grandparent-service-container"]}, ), ) @@ -3107,7 +1973,17 @@ def test_construct_dependency_graph_non_self_reference( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - install_and_verify_dependencies(service) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=_url_dispatch( + { + "parent-service": _make_zip_bytes(parent_service_config), + "child-service": _make_zip_bytes(child_service_config), + } + ), + ): + install_and_verify_dependencies(service) + dependency_graph = construct_dependency_graph(service, ["default"]) assert dependency_graph.graph == { DependencyNode( @@ -3172,16 +2048,8 @@ def test_construct_dependency_graph_non_self_reference( ), "Parent service should come before grandparent service in the starting order" -def test_construct_dependency_graph_complex( - tmp_path: Path, -) -> None: - parent_service_repo_path = tmp_path / "parent-service-repo" - child_service_repo_path = tmp_path / "child-service-repo" - grandparent_service_repo_path = tmp_path / "grandparent-service-repo" - create_mock_git_repo("blank_repo", parent_service_repo_path) - create_mock_git_repo("blank_repo", child_service_repo_path) - create_mock_git_repo("blank_repo", grandparent_service_repo_path) - parent_service_repo_config = { +def test_construct_dependency_graph_complex(tmp_path: Path) -> None: + parent_service_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "parent-service", @@ -3190,7 +2058,7 @@ def test_construct_dependency_graph_complex( "description": "child-service", "remote": { "repo_name": "child-service", - "repo_link": f"file://{child_service_repo_path}", + "repo_link": "https://github.com/getsentry/child-service", "branch": "main", }, }, @@ -3217,7 +2085,7 @@ def test_construct_dependency_graph_complex( }, }, } - child_service_repo_config = { + child_service_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "child-service", @@ -3242,7 +2110,7 @@ def test_construct_dependency_graph_complex( }, }, } - grandparent_service_repo_config = { + grandparent_service_config = { "x-sentry-service-config": { "version": 0.1, "service_name": "grandparent-service", @@ -3251,7 +2119,7 @@ def test_construct_dependency_graph_complex( "description": "parent-service", "remote": { "repo_name": "parent-service", - "repo_link": f"file://{parent_service_repo_path}", + "repo_link": "https://github.com/getsentry/parent-service", "branch": "main", }, }, @@ -3278,21 +2146,6 @@ def test_construct_dependency_graph_complex( }, }, } - create_config_file(parent_service_repo_path, parent_service_repo_config) - create_config_file(child_service_repo_path, child_service_repo_config) - create_config_file(grandparent_service_repo_path, grandparent_service_repo_config) - run_git_command(["add", "."], cwd=parent_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=parent_service_repo_path - ) - run_git_command(["add", "."], cwd=child_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=child_service_repo_path - ) - run_git_command(["add", "."], cwd=grandparent_service_repo_path) - run_git_command( - ["commit", "-m", "Add devservices config"], cwd=grandparent_service_repo_path - ) service = Service( name="complex-service", repo_path="/path/to/complex-service", @@ -3304,7 +2157,7 @@ def test_construct_dependency_graph_complex( description="child-service", remote=RemoteConfig( repo_name="child-service", - repo_link=f"file://{child_service_repo_path}", + repo_link="https://github.com/getsentry/child-service", branch="main", ), dependency_type=DependencyType.SERVICE, @@ -3313,7 +2166,7 @@ def test_construct_dependency_graph_complex( description="grandparent-service", remote=RemoteConfig( repo_name="grandparent-service", - repo_link=f"file://{grandparent_service_repo_path}", + repo_link="https://github.com/getsentry/grandparent-service", branch="main", ), dependency_type=DependencyType.SERVICE, @@ -3333,7 +2186,18 @@ def test_construct_dependency_graph_complex( "devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR", str(tmp_path / "dependency-dir"), ): - install_and_verify_dependencies(service) + with mock.patch( + "devservices.utils.dependencies.urllib.request.urlopen", + side_effect=_url_dispatch( + { + "parent-service": _make_zip_bytes(parent_service_config), + "child-service": _make_zip_bytes(child_service_config), + "grandparent-service": _make_zip_bytes(grandparent_service_config), + } + ), + ): + install_and_verify_dependencies(service) + dependency_graph = construct_dependency_graph(service, ["default"]) assert dependency_graph.graph == { DependencyNode( From c46f8c8aae615368a486b9d421dda54dba4c8046 Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Tue, 28 Apr 2026 14:42:42 -0700 Subject: [PATCH 2/2] review --- devservices/exceptions.py | 7 ------- devservices/utils/dependencies.py | 8 ++------ 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/devservices/exceptions.py b/devservices/exceptions.py index c17ebb2f..8487d413 100644 --- a/devservices/exceptions.py +++ b/devservices/exceptions.py @@ -102,13 +102,6 @@ def __str__(self) -> str: return f"DependencyError: {self.repo_name} ({self.repo_link}) on {self.branch}" -class UnableToCloneDependencyError(DependencyError): - """Raised when a dependency is unable to be cloned.""" - - def __str__(self) -> str: - return f"Unable to clone dependency: {self.repo_name} ({self.repo_link}) on {self.branch}" - - class InvalidDependencyConfigError(DependencyError): """Raised when a dependency's config is invalid.""" diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py index 41adbc47..b1a0944a 100644 --- a/devservices/utils/dependencies.py +++ b/devservices/utils/dependencies.py @@ -438,7 +438,7 @@ def _download() -> bytes: try: zip_data = io.BytesIO(retry(_download, exceptions=(urllib.error.URLError,))) - except urllib.error.URLError as e: + except (urllib.error.URLError, ValueError) as e: raise DependencyError( repo_name=dependency.repo_name, repo_link=dependency.repo_link, @@ -479,7 +479,7 @@ def _download() -> bytes: os.makedirs(os.path.dirname(target), exist_ok=True) with zf.open(name) as src, open(target, "wb") as dst: shutil.copyfileobj(src, dst) - except zipfile.BadZipFile as e: + except (zipfile.BadZipFile, OSError) as e: raise DependencyError( repo_name=dependency.repo_name, repo_link=dependency.repo_link, @@ -487,10 +487,6 @@ def _download() -> bytes: ) from e -def _has_valid_config_file(path: str) -> bool: - return os.path.exists(os.path.join(path, DEVSERVICES_DIR_NAME, CONFIG_FILE_NAME)) - - def _get_remote_configs(dependencies: list[Dependency]) -> list[RemoteConfig]: return [ dependency.remote