diff --git a/pyproject.toml b/pyproject.toml index 8306cfb066..4351126acf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -53,8 +53,6 @@ ignore_missing_imports = true follow_imports = "silent" exclude = [ "build", - # launchpadlib is not typed - "snapcraft/remote/launchpad.py", "snapcraft_legacy", "tests/spread", "tests/legacy", @@ -67,13 +65,7 @@ plugins = [ [tool.pyright] include = ["snapcraft", "tests"] -exclude = [ - "build", - # launchpadlib is not typed - "snapcraft/remote/launchpad.py", - "tests/legacy", - "tests/spread", -] +exclude = ["build", "tests/legacy", "tests/spread"] pythonVersion = "3.10" [tool.pytest.ini_options] diff --git a/snapcraft/commands/remote.py b/snapcraft/commands/remote.py index a5bc7ea384..dbde5a4b61 100644 --- a/snapcraft/commands/remote.py +++ b/snapcraft/commands/remote.py @@ -21,7 +21,7 @@ import textwrap from enum import Enum from pathlib import Path -from typing import Optional +from typing import List, Optional from craft_cli import BaseCommand, emit from craft_cli.helptexts import HIDDEN @@ -30,9 +30,8 @@ from snapcraft.errors import MaintenanceBase, SnapcraftError from snapcraft.legacy_cli import run_legacy from snapcraft.parts import yaml_utils -from snapcraft.remote import get_build_id, is_repo -from snapcraft.utils import confirm_with_user, humanize_list -from snapcraft_legacy.internal.remote_build.errors import AcceptPublicUploadError +from snapcraft.remote import AcceptPublicUploadError, RemoteBuilder, is_repo +from snapcraft.utils import confirm_with_user, get_host_architecture, humanize_list _CONFIRMATION_PROMPT = ( "All data sent to remote builders will be publicly available. " @@ -216,23 +215,53 @@ def _get_project_name(self) -> str: def _run_new_remote_build(self) -> None: """Run new remote-build code.""" - # the build-id will be passed to the new remote-build code as part of #4323 - if self._parsed_args.build_id: - build_id = self._parsed_args.build_id - emit.debug(f"Using build ID {build_id!r} passed as a parameter.") - else: - build_id = get_build_id( - app_name="snapcraft", - project_name=self._get_project_name(), - project_path=Path(), - ) - emit.debug(f"Using computed build ID {build_id!r}.") + emit.progress("Setting up launchpad environment.") + remote_builder = RemoteBuilder( + app_name="snapcraft", + build_id=self._parsed_args.build_id, + project_name=self._get_project_name(), + architectures=self._determine_architectures(), + project_dir=Path(), + ) + + if self._parsed_args.status: + remote_builder.print_status() + return + + emit.progress("Looking for existing builds.") + has_outstanding_build = remote_builder.has_outstanding_build() + if self._parsed_args.recover and not has_outstanding_build: + emit.message("No build found.") + return + + if has_outstanding_build: + emit.message("Found previously started build.") + remote_builder.print_status() - # TODO: use new remote-build code (#4323) - emit.debug( - "Running fallback remote-build because new remote-build is not available." + # If recovery specified, monitor build and exit. + if self._parsed_args.recover or confirm_with_user( + "Do you wish to recover this build?", default=True + ): + emit.progress("Building") + remote_builder.monitor_build() + emit.progress("Cleaning") + remote_builder.clean_build() + return + + # Otherwise clean running build before we start a new one. + emit.progress("Cleaning previously existing build.") + remote_builder.clean_build() + + emit.message( + "If interrupted, resume with: 'snapcraft remote-build --recover " + f"--build-id {remote_builder.build_id}'." ) - run_legacy() + emit.progress("Starting build") + remote_builder.start_build() + emit.progress("Building") + remote_builder.monitor_build() + emit.progress("Cleaning") + remote_builder.clean_build() def _get_build_strategy(self) -> Optional[_Strategies]: """Get the build strategy from the envvar `SNAPCRAFT_REMOTE_BUILD_STRATEGY`. @@ -285,6 +314,56 @@ def _get_effective_base(self) -> str: return base + def _get_project_build_on_architectures(self) -> List[str]: + """Get a list of build-on architectures from the project's snapcraft.yaml. + + :returns: A list of architectures. + """ + with open(self._snapcraft_yaml, encoding="utf-8") as file: + data = yaml_utils.safe_load(file) + + project_archs = data.get("architectures") + + archs = [] + if project_archs: + for item in project_archs: + if "build-on" in item: + new_arch = item["build-on"] + if isinstance(new_arch, list): + archs.extend(new_arch) + else: + archs.append(new_arch) + + return archs + + def _determine_architectures(self) -> List[str]: + """Determine architectures to build for. + + The build architectures can be set via the `--build-on` parameter or determined + from the build-on architectures listed in the project's snapcraft.yaml. + + :returns: A list of architectures. + + :raises SnapcraftError: If `--build-on` was provided and architectures are + defined in the project's snapcraft.yaml. + """ + project_architectures = self._get_project_build_on_architectures() + if project_architectures and self._parsed_args.build_for: + raise SnapcraftError( + "Cannot use `--build-on` because architectures are already defined in " + "snapcraft.yaml." + ) + + if project_architectures: + archs = project_architectures + elif self._parsed_args.build_for: + archs = self._parsed_args.build_for + else: + # default to typical snapcraft behavior (build for host) + archs = [get_host_architecture()] + + return archs + def _get_esm_warning_for_base(base: str) -> str: """Return a warning appropriate for the base under ESM.""" diff --git a/snapcraft/legacy_cli.py b/snapcraft/legacy_cli.py index 335688817d..29c66da77a 100644 --- a/snapcraft/legacy_cli.py +++ b/snapcraft/legacy_cli.py @@ -25,7 +25,7 @@ import snapcraft_legacy from snapcraft_legacy.cli import legacy -_LIB_NAMES = ("craft_parts", "craft_providers", "craft_store") +_LIB_NAMES = ("craft_parts", "craft_providers", "craft_store", "snapcraft.remote") _ORIGINAL_LIB_NAME_LOG_LEVEL: Dict[str, int] = {} diff --git a/snapcraft/remote/__init__.py b/snapcraft/remote/__init__.py index a8cdf9683c..1fd5f8125c 100644 --- a/snapcraft/remote/__init__.py +++ b/snapcraft/remote/__init__.py @@ -17,25 +17,33 @@ """Remote-build and related utilities.""" from .errors import ( + AcceptPublicUploadError, GitError, LaunchpadHttpsError, RemoteBuildError, RemoteBuildTimeoutError, + UnsupportedArchitectureError, ) from .git import GitRepo, is_repo from .launchpad import LaunchpadClient -from .utils import get_build_id, rmtree +from .remote_builder import RemoteBuilder +from .utils import get_build_id, humanize_list, rmtree, validate_architectures from .worktree import WorkTree __all__ = [ "get_build_id", + "humanize_list", "is_repo", "rmtree", + "validate_architectures", + "AcceptPublicUploadError", "GitError", "GitRepo", "LaunchpadClient", "LaunchpadHttpsError", + "RemoteBuilder", "RemoteBuildError", "RemoteBuildTimeoutError", + "UnsupportedArchitectureError", "WorkTree", ] diff --git a/snapcraft/remote/errors.py b/snapcraft/remote/errors.py index 28540f3cba..30b2b3b97b 100644 --- a/snapcraft/remote/errors.py +++ b/snapcraft/remote/errors.py @@ -17,7 +17,7 @@ """Remote build errors.""" from dataclasses import dataclass -from typing import Optional +from typing import List, Optional @dataclass(repr=True) @@ -69,3 +69,32 @@ def __init__(self) -> None: details = "Verify connectivity to https://api.launchpad.net and retry build." super().__init__(brief=brief, details=details) + + +class UnsupportedArchitectureError(RemoteBuildError): + """Unsupported architecture error.""" + + def __init__(self, architectures: List[str]) -> None: + brief = "Architecture not supported by the remote builder." + details = ( + "The following architectures are not supported by the remote builder: " + f"{architectures}.\nPlease remove them from the " + "architecture list and try again." + ) + + super().__init__(brief=brief, details=details) + + +class AcceptPublicUploadError(RemoteBuildError): + """Accept public upload error.""" + + def __init__(self) -> None: + brief = "Cannot upload data to build servers." + details = ( + "Remote build needs explicit acknowledgement that data sent to build " + "servers is public.\n" + "In non-interactive runs, please use the option " + "`--launchpad-accept-public-upload`." + ) + + super().__init__(brief=brief, details=details) diff --git a/snapcraft/remote/git.py b/snapcraft/remote/git.py index 83b6c92a80..f6b67b8e35 100644 --- a/snapcraft/remote/git.py +++ b/snapcraft/remote/git.py @@ -137,7 +137,7 @@ def _init_repo(self) -> None: :raises GitError: if the repo cannot be initialized """ - logger.debug("Initializing git repository in {str(self.path)!r}") + logger.debug("Initializing git repository in %r", str(self.path)) try: pygit2.init_repository(self.path) diff --git a/snapcraft/remote/launchpad.py b/snapcraft/remote/launchpad.py index df63bd634c..b6bf075d0e 100644 --- a/snapcraft/remote/launchpad.py +++ b/snapcraft/remote/launchpad.py @@ -23,7 +23,7 @@ import time from datetime import datetime, timedelta, timezone from pathlib import Path -from typing import Any, Dict, List, Optional, Sequence +from typing import Any, Dict, List, Optional, Sequence, cast from urllib.parse import unquote, urlsplit import requests @@ -102,7 +102,7 @@ def __init__( self._project_name = project_name self._lp: Launchpad = self._login() - self.user = self._lp.me.name + self.user = self._lp.me.name # type: ignore self._deadline = deadline @@ -151,7 +151,7 @@ def _create_cache_directory(self) -> Path: cache_dir.mkdir(mode=0o700, exist_ok=True) return cache_dir - def _fetch_artifacts(self, snap: Entry) -> None: + def _fetch_artifacts(self, snap: Optional[Entry]) -> None: """Fetch build arftifacts (logs and snaps).""" builds = self._get_builds(snap) @@ -160,28 +160,32 @@ def _fetch_artifacts(self, snap: Entry) -> None: self._download_build_artifacts(build) self._download_log(build) - def _get_builds_collection_entry(self, snap: Entry) -> Optional[Entry]: + def _get_builds_collection_entry(self, snap: Optional[Entry]) -> Optional[Entry]: logger.debug("Fetching builds collection information from Launchpad...") - url = snap.builds_collection_link - return self._lp_load_url(url) + if snap: + url = cast(str, snap.builds_collection_link) + return self._lp_load_url(url) + return None - def _get_builds(self, snap: Entry) -> List[Dict[str, Any]]: + def _get_builds(self, snap: Optional[Entry]) -> List[Dict[str, Any]]: builds_collection = self._get_builds_collection_entry(snap) if builds_collection is None: return [] - return builds_collection.entries + return cast(List[Dict[str, Any]], builds_collection.entries) def _get_snap(self) -> Optional[Entry]: try: - return self._lp.snaps.getByName(name=self._lp_name, owner=self._lp_owner) - except restfulclient.errors.NotFound: + return self._lp.snaps.getByName( # type: ignore + name=self._lp_name, owner=self._lp_owner + ) + except restfulclient.errors.NotFound: # type: ignore return None def _issue_build_request(self, snap: Entry) -> Entry: - dist = self._lp.distributions["ubuntu"] + dist = self._lp.distributions["ubuntu"] # type: ignore archive = dist.main_archive - return snap.requestBuilds( + return snap.requestBuilds( # type: ignore archive=archive, pocket="Updates", ) @@ -214,7 +218,7 @@ def _wait_for_build_request_acceptance(self, build_request: Entry) -> None: if build_request.status == "Failed": # Build request failed. self.cleanup() - raise errors.RemoteBuildError(build_request.error_message) + raise errors.RemoteBuildError(cast(str, build_request.error_message)) if build_request.status != "Completed": # Shouldn't end up here. @@ -223,14 +227,14 @@ def _wait_for_build_request_acceptance(self, build_request: Entry) -> None: f"Unknown builder error - reported status: {build_request.status}" ) - if not build_request.builds.entries: + if not build_request.builds.entries: # type: ignore # Shouldn't end up here either. self.cleanup() raise errors.RemoteBuildError( "Unknown builder error - no build entries found." ) - build_number = _get_url_basename(build_request.self_link) + build_number = _get_url_basename(cast(str, build_request.self_link)) logger.debug("Build request accepted: %s", build_number) def _login(self) -> Launchpad: @@ -274,14 +278,14 @@ def _create_git_repository(self, force=False) -> Entry: self._lp_owner, self._lp_owner, ) - return self._lp.git_repositories.new( + return self._lp.git_repositories.new( # type: ignore name=self._lp_name, owner=self._lp_owner, target=self._lp_owner ) def _delete_git_repository(self) -> None: """Delete git repository.""" git_path = self.get_git_repo_path() - git_repo = self._lp.git_repositories.getByPath(path=git_path) + git_repo = self._lp.git_repositories.getByPath(path=git_path) # type: ignore # git_repositories.getByPath returns None if git repo does not exist. if git_repo is None: @@ -306,7 +310,7 @@ def _create_snap(self, force=False) -> Entry: "url=https://launchpad.net/%s/+snap/%s", self._lp_owner, self._lp_name ) - return self._lp.snaps.new( + return self._lp.snaps.new( # type: ignore name=self._lp_name, owner=self._lp_owner, git_repository_url=git_url, @@ -350,15 +354,17 @@ def monitor_build(self, interval: int = _LP_POLL_INTERVAL) -> None: builds = self._get_builds(snap) pending = False timestamp = str(datetime.now()) - logger.info("Build status as of %s:", timestamp) + status = f"Build status as of {timestamp}: " for build in builds: state = build["buildstate"] arch = build["arch_tag"] - logger.info("\tarch=%s\tstate=%s", arch, state) + status += f" {arch=} {state=}" if _is_build_pending(build): pending = True + logger.info(status) + if pending is False: break @@ -421,7 +427,7 @@ def _download_file(self, *, url: str, dst: str, gunzip: bool = False) -> None: def _download_build_artifacts(self, build: Dict[str, Any]) -> None: arch = build["arch_tag"] snap_build = self._lp_load_url(build["self_link"]) - urls = snap_build.getFileUrls() + urls = snap_build.getFileUrls() # type: ignore if not urls: logger.error("Snap file not available for arch %r.", arch) @@ -437,17 +443,6 @@ def _download_build_artifacts(self, build: Dict[str, Any]) -> None: else: logger.info("Fetched %s", file_name) - def _gitify_repository(self, repo_dir: Path) -> GitRepo: - """Git-ify source repository tree. - - :return: Git handler instance to git repository. - """ - repo = GitRepo(repo_dir) - if not repo.is_clean(): - repo.add_all() - repo.commit() - return GitRepo - def has_outstanding_build(self) -> bool: """Check if there is an existing build configured on Launchpad.""" snap = self._get_snap() @@ -455,14 +450,13 @@ def has_outstanding_build(self) -> bool: def push_source_tree(self, repo_dir: Path) -> None: """Push source tree to Launchpad.""" - git_handler = self._gitify_repository(repo_dir) lp_repo = self._create_git_repository(force=True) # This token will only be used once, immediately after issuing it, # so it can have a short expiry time. It's not a problem if it # expires before the build completes, or even before the push # completes. date_expires = datetime.now(timezone.utc) + timedelta(minutes=1) - token = lp_repo.issueAccessToken( + token = lp_repo.issueAccessToken( # type: ignore description=f"{self._app_name} remote-build for {self._build_id}", scopes=["repository:push"], date_expires=date_expires.isoformat(), @@ -475,4 +469,5 @@ def push_source_tree(self, repo_dir: Path) -> None: logger.info("Sending build data to Launchpad: %s", stripped_url) - git_handler.push_url(url, "main", "HEAD", token) + repo = GitRepo(repo_dir) + repo.push_url(url, "main", "HEAD", token) diff --git a/snapcraft/remote/remote_builder.py b/snapcraft/remote/remote_builder.py new file mode 100644 index 0000000000..dff8abde30 --- /dev/null +++ b/snapcraft/remote/remote_builder.py @@ -0,0 +1,133 @@ +# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- +# +# Copyright 2023 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3 as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Manager for creating, monitoring, and cleaning remote builds.""" + +import logging +from pathlib import Path +from typing import List, Optional + +from .launchpad import LaunchpadClient +from .utils import get_build_id, humanize_list, validate_architectures +from .worktree import WorkTree + +logger = logging.getLogger(__name__) + + +class RemoteBuilder: + """Remote builder class. + + :param app_name: Name of the application. + :param build_id: Unique identifier for the build. + :param project_name: Name of the project. + :param architectures: List of architectures to build on. + :param project_dir: Path of the project. + + :raises UnsupportedArchitectureError: if any architecture is not supported + for remote building. + :raises LaunchpadHttpsError: If a connection to Launchpad cannot be established. + """ + + def __init__( + self, + app_name: str, + build_id: Optional[str], + project_name: str, + architectures: List[str], + project_dir: Path, + ): + self._app_name = app_name + self._project_name = project_name + self._project_dir = project_dir + + if build_id: + self._build_id = build_id + else: + self._build_id = get_build_id( + app_name=self._app_name, + project_name=self._project_name, + project_path=self._project_dir, + ) + + validate_architectures(architectures) + self._architectures = architectures + + self._worktree = WorkTree( + app_name=self._app_name, + build_id=self._build_id, + project_dir=self._project_dir, + ) + + logger.debug("Setting up launchpad environment.") + + self._lpc = LaunchpadClient( + app_name=self._app_name, + build_id=self._build_id, + project_name=self._project_name, + architectures=self._architectures, + ) + + @property + def build_id(self) -> str: + """Get the build id.""" + return self._build_id + + def print_status(self) -> None: + """Print the status of a remote build in Launchpad.""" + if self._lpc.has_outstanding_build(): + build_status = self._lpc.get_build_status() + for arch, status in build_status.items(): + logger.info("Build status for arch %s: %s", arch, status) + else: + logger.info("No build found.") + + def has_outstanding_build(self) -> bool: + """Check if there is an existing build on Launchpad. + + :returns: True if there is an existing (incomplete) build on Launchpad. + """ + return self._lpc.has_outstanding_build() + + def monitor_build(self) -> None: + """Monitor and periodically log the status of a remote build in Launchpad.""" + logger.info( + "Building snap package for %s. This may take some time to finish.", + humanize_list(self._lpc.architectures, "and", "{}"), + ) + + logger.info("Building...") + self._lpc.monitor_build() + + logger.info("Build complete.") + + def clean_build(self) -> None: + """Clean the cache and Launchpad build.""" + logger.info("Cleaning existing builds and artefacts.") + self._lpc.cleanup() + self._worktree.clean_cache() + logger.info("Done.") + + def start_build(self) -> None: + """Start a build in Launchpad. + + A local copy of the project is created and pushed to Launchpad via git. + """ + self._worktree.init_repo() + + logger.debug("Cached project at %s", self._worktree.repo_dir) + self._lpc.push_source_tree(repo_dir=self._worktree.repo_dir) + + self._lpc.start_build() diff --git a/snapcraft/remote/utils.py b/snapcraft/remote/utils.py index ced03beb27..f4140b16ca 100644 --- a/snapcraft/remote/utils.py +++ b/snapcraft/remote/utils.py @@ -21,13 +21,33 @@ from functools import partial from hashlib import md5 from pathlib import Path -from typing import List +from typing import Iterable, List + +from .errors import UnsupportedArchitectureError + +_SUPPORTED_ARCHS = ["amd64", "arm64", "armhf", "i386", "ppc64el", "s390x"] + + +def validate_architectures(architectures: List[str]) -> None: + """Validate that architectures are supported for remote building. + + :param architectures: list of architectures to validate + + :raises UnsupportedArchitectureError: if any architecture in the list in not + supported for remote building. + """ + unsupported_archs = [] + for arch in architectures: + if arch not in _SUPPORTED_ARCHS: + unsupported_archs.append(arch) + if unsupported_archs: + raise UnsupportedArchitectureError(architectures=unsupported_archs) def get_build_id(app_name: str, project_name: str, project_path: Path) -> str: """Get the build id for a project. - The build id is formatted as `snapcraft--`. + The build id is formatted as `--`. The hash is a hash of all files in the project directory. :param app_name: Name of the application. @@ -80,15 +100,45 @@ def _compute_hash(directory: Path) -> str: return md5(all_hashes).hexdigest() # noqa: S324 (insecure-hash-function) +def humanize_list( + items: Iterable[str], + conjunction: str, + item_format: str = "{!r}", + sort: bool = True, +) -> str: + """Format a list into a human-readable string. + + :param items: list to humanize. + :param conjunction: the conjunction used to join the final element to + the rest of the list (e.g. 'and'). + :param item_format: format string to use per item. + :param sort: if true, sort the list. + """ + if not items: + return "" + + quoted_items = [item_format.format(item) for item in items] + + if sort: + quoted_items = sorted(quoted_items) + + if len(quoted_items) == 1: + return quoted_items[0] + + humanized = ", ".join(quoted_items[:-1]) + + if len(quoted_items) > 2: + humanized += "," + + return f"{humanized} {conjunction} {quoted_items[-1]}" + + def rmtree(directory: Path) -> None: """Cross-platform rmtree implementation. :param directory: Directory to remove. """ - shutil.rmtree( - str(directory.resolve()), - onerror=_remove_readonly, # type: ignore - ) + shutil.rmtree(str(directory.resolve()), onerror=_remove_readonly) def _remove_readonly(func, filepath, _): diff --git a/snapcraft/remote/worktree.py b/snapcraft/remote/worktree.py index af97a5e9f7..480bbbf04e 100644 --- a/snapcraft/remote/worktree.py +++ b/snapcraft/remote/worktree.py @@ -14,25 +14,54 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Manages tree for remote builds.""" +"""Manages trees for remote builds.""" from pathlib import Path +from shutil import copytree -from snapcraft.remote.utils import rmtree +from xdg import BaseDirectory +from .git import GitRepo +from .utils import rmtree -class WorkTree: - """Class to manages tree for remote builds.""" - def __init__(self, worktree_dir: Path) -> None: - """Create remote-build WorkTree. +class WorkTree: + """Class to manage trees for remote builds. - :param worktree_dir: Directory to use for working tree. - """ - # Working tree base directory. - self._base_dir = worktree_dir + :param app_name: Name of the application. + :param build_id: Unique identifier for the build. + :param project_dir: Path to project directory. + """ - # Initialize clean repo to ship to remote builder. + def __init__(self, app_name: str, build_id: str, project_dir: Path) -> None: + self._project_dir = project_dir + self._base_dir = Path( + BaseDirectory.save_cache_path(app_name, "remote-build", build_id) + ) self._repo_dir = self._base_dir / "repo" + + @property + def repo_dir(self) -> Path: + """Get path the cached repository.""" + return self._repo_dir + + def init_repo(self) -> None: + """Initialize a clean repo.""" if self._repo_dir.exists(): rmtree(self._repo_dir) + + copytree(self._project_dir, self._repo_dir) + + self._gitify_repository() + + def _gitify_repository(self) -> None: + """Git-ify source repository tree.""" + repo = GitRepo(self._repo_dir) + if not repo.is_clean(): + repo.add_all() + repo.commit() + + def clean_cache(self): + """Clean the cache.""" + if self._base_dir.exists(): + rmtree(self._base_dir) diff --git a/tests/unit/commands/test_remote.py b/tests/unit/commands/test_remote.py index 35403c475f..889585bc90 100644 --- a/tests/unit/commands/test_remote.py +++ b/tests/unit/commands/test_remote.py @@ -18,14 +18,14 @@ import sys from pathlib import Path +from unittest.mock import ANY, call import pytest from yaml import safe_dump from snapcraft import cli from snapcraft.parts.yaml_utils import CURRENT_BASES, ESM_BASES, LEGACY_BASES -from snapcraft.remote import GitRepo -from snapcraft_legacy.internal.remote_build.errors import AcceptPublicUploadError +from snapcraft.remote import AcceptPublicUploadError, GitRepo # remote-build control logic may check if the working dir is a git repo, # so execute all tests inside a test directory @@ -44,7 +44,7 @@ def use_new_remote_build(monkeypatch): monkeypatch.setenv("SNAPCRAFT_REMOTE_BUILD_STRATEGY", "disable-fallback") -@pytest.fixture +@pytest.fixture() def fake_sudo(monkeypatch): monkeypatch.setenv("SUDO_USER", "fake") monkeypatch.setattr("os.geteuid", lambda: 0) @@ -56,13 +56,20 @@ def mock_argv(mocker): return mocker.patch.object(sys, "argv", ["snapcraft", "remote-build"]) -@pytest.fixture +@pytest.fixture() def mock_confirm(mocker): return mocker.patch( "snapcraft.commands.remote.confirm_with_user", return_value=True ) +@pytest.fixture() +def mock_remote_builder(mocker): + _mock_remote_builder = mocker.patch("snapcraft.commands.remote.RemoteBuilder") + _mock_remote_builder.return_value.has_outstanding_build.return_value = False + return _mock_remote_builder + + @pytest.fixture() def mock_run_new_or_fallback_remote_build(mocker): return mocker.patch( @@ -70,6 +77,13 @@ def mock_run_new_or_fallback_remote_build(mocker): ) +@pytest.fixture() +def mock_run_new_remote_build(mocker): + return mocker.patch( + "snapcraft.commands.remote.RemoteBuildCommand._run_new_remote_build" + ) + + @pytest.fixture() def mock_run_legacy(mocker): return mocker.patch("snapcraft.commands.remote.run_legacy") @@ -297,15 +311,12 @@ def test_get_effective_base_core18_esm_warning( "create_snapcraft_yaml", CURRENT_BASES - {"core22"}, indirect=True ) @pytest.mark.usefixtures("create_snapcraft_yaml", "mock_confirm", "mock_argv") -def test_run_newer_than_core_22(emitter, mock_run_legacy): +def test_run_newer_than_core_22(emitter, mock_run_new_remote_build): """Bases newer than core22 must use new remote-build.""" cli.run() - # this should fail when new remote-build code is used (#4323) - mock_run_legacy.assert_called_once() - emitter.assert_debug( - "Running fallback remote-build because new remote-build is not available." - ) + mock_run_new_remote_build.assert_called_once() + emitter.assert_debug("Running new remote-build because base is newer than core22.") @pytest.mark.parametrize( @@ -327,7 +338,9 @@ def test_run_core22_and_older(emitter, mock_run_legacy): "envvar", ["force-fallback", "disable-fallback", "badvalue", None] ) @pytest.mark.usefixtures("create_snapcraft_yaml", "mock_confirm", "mock_argv") -def test_run_envvar_newer_than_core22(envvar, emitter, mock_run_legacy, monkeypatch): +def test_run_envvar_newer_than_core22( + envvar, emitter, mock_run_new_remote_build, monkeypatch +): """Bases newer than core22 run new remote-build regardless of envvar.""" if envvar: monkeypatch.setenv("SNAPCRAFT_REMOTE_BUILD_STRATEGY", envvar) @@ -336,23 +349,21 @@ def test_run_envvar_newer_than_core22(envvar, emitter, mock_run_legacy, monkeypa cli.run() - mock_run_legacy.assert_called_once() - emitter.assert_debug( - "Running fallback remote-build because new remote-build is not available." - ) + mock_run_new_remote_build.assert_called_once() + emitter.assert_debug("Running new remote-build because base is newer than core22.") @pytest.mark.parametrize( "create_snapcraft_yaml", LEGACY_BASES | {"core22"}, indirect=True ) @pytest.mark.usefixtures("create_snapcraft_yaml", "mock_confirm", "mock_argv") -def test_run_envvar_disable_fallback(emitter, mock_run_legacy, monkeypatch): +def test_run_envvar_disable_fallback(emitter, mock_run_new_remote_build, monkeypatch): """core22 and older bases run new remote-build if envvar is `disable-fallback`.""" monkeypatch.setenv("SNAPCRAFT_REMOTE_BUILD_STRATEGY", "disable-fallback") cli.run() - mock_run_legacy.assert_called_once() + mock_run_new_remote_build.assert_called_once() emitter.assert_debug( "Running new remote-build because environment variable " "'SNAPCRAFT_REMOTE_BUILD_STRATEGY' is 'disable-fallback'." @@ -426,15 +437,14 @@ def test_run_envvar_invalid(capsys, emitter, mock_run_legacy, monkeypatch): "create_snapcraft_yaml", LEGACY_BASES | {"core22"}, indirect=True ) @pytest.mark.usefixtures("create_snapcraft_yaml", "mock_confirm", "mock_argv") -def test_run_in_repo(emitter, mock_run_legacy, new_dir): +def test_run_in_repo(emitter, mock_run_new_remote_build, new_dir): """core22 and older bases run new remote-build if in a git repo.""" # initialize a git repo GitRepo(new_dir) cli.run() - # this should fail when new remote-build code is used (#4323) - mock_run_legacy.assert_called_once() + mock_run_new_remote_build.assert_called_once() emitter.assert_debug( "Running new remote-build because project is in a git repository." ) @@ -456,68 +466,200 @@ def test_run_not_in_repo(emitter, mock_run_legacy): "create_snapcraft_yaml", CURRENT_BASES - {"core22"}, indirect=True ) @pytest.mark.usefixtures("create_snapcraft_yaml", "mock_confirm", "mock_argv") -def test_run_in_repo_newer_than_core22(emitter, mock_run_legacy, monkeypatch, new_dir): +def test_run_in_repo_newer_than_core22( + emitter, mock_run_new_remote_build, monkeypatch, new_dir +): """Bases newer than core22 run new remote-build regardless of being in a repo.""" # initialize a git repo GitRepo(new_dir) cli.run() - # this should fail when new remote-build code is used (#4323) - mock_run_legacy.assert_called_once() - emitter.assert_debug( - "Running fallback remote-build because new remote-build is not available." + mock_run_new_remote_build.assert_called_once() + emitter.assert_debug("Running new remote-build because base is newer than core22.") + + +###################### +# Architecture tests # +###################### + + +@pytest.mark.parametrize("base", CURRENT_BASES | LEGACY_BASES) +@pytest.mark.parametrize( + ["archs", "expected_archs"], + [ + # single arch as scalar + ([{"build-on": "arm64", "build-for": "arm64"}], ["arm64"]), + # single arch as list + ([{"build-on": ["arm64"], "build-for": ["arm64"]}], ["arm64"]), + # no build-for as scalar + ([{"build-on": "arm64"}], ["arm64"]), + # no build-for as list + ([{"build-on": ["arm64"]}], ["arm64"]), + # multiple archs as scalars + ( + [ + {"build-on": "amd64", "build-for": "amd64"}, + {"build-on": "arm64", "build-for": "arm64"}, + ], + ["amd64", "arm64"], + ), + # multiple archs as lists + ( + [ + {"build-on": ["amd64"], "build-for": ["amd64"]}, + {"build-on": ["arm64"], "build-for": ["arm64"]}, + ], + ["amd64", "arm64"], + ), + # multiple build-ons + ( + [ + {"build-on": ["amd64", "arm64"], "build-for": "amd64"}, + {"build-on": ["armhf", "powerpc"], "build-for": "arm64"}, + ], + ["amd64", "arm64", "armhf", "powerpc"], + ), + ], +) +@pytest.mark.usefixtures("mock_argv", "mock_confirm", "use_new_remote_build") +def test_determine_architectures_from_snapcraft_yaml( + archs, expected_archs, base, snapcraft_yaml, mock_remote_builder +): + """Parse `build-on` architectures from a snapcraft.yaml file.""" + snapcraft_yaml(base=base, architectures=archs) + + cli.run() + + mock_remote_builder.assert_called_with( + app_name="snapcraft", + build_id=None, + project_name="mytest", + architectures=expected_archs, + project_dir=Path(), + ) + + +@pytest.mark.parametrize( + "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True +) +@pytest.mark.usefixtures( + "create_snapcraft_yaml", "mock_argv", "mock_confirm", "use_new_remote_build" +) +def test_determine_architectures_host_arch(mocker, mock_remote_builder): + """Use host architecture if not defined in the snapcraft.yaml.""" + mocker.patch( + "snapcraft.commands.remote.get_host_architecture", return_value="arm64" + ) + + cli.run() + + mock_remote_builder.assert_called_with( + app_name="snapcraft", + build_id=None, + project_name="mytest", + architectures=["arm64"], + project_dir=Path(), ) +@pytest.mark.parametrize( + ("args", "expected_archs"), + [ + (["--build-for", "amd64"], ["amd64"]), + (["--build-for", "amd64", "arm64"], ["amd64", "arm64"]), + # launchpad will accept and ignore duplicates + (["--build-for", "amd64", "amd64"], ["amd64", "amd64"]), + ], +) +@pytest.mark.parametrize( + "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True +) +@pytest.mark.usefixtures( + "create_snapcraft_yaml", "mock_confirm", "use_new_remote_build" +) +def test_determine_architectures_provided_by_user( + args, expected_archs, mocker, mock_remote_builder +): + """Use architectures provided by the user.""" + mocker.patch.object(sys, "argv", ["snapcraft", "remote-build"] + args) + + cli.run() + + mock_remote_builder.assert_called_with( + app_name="snapcraft", + build_id=None, + project_name="mytest", + architectures=expected_archs, + project_dir=Path(), + ) + + +@pytest.mark.parametrize("base", CURRENT_BASES | LEGACY_BASES) +@pytest.mark.usefixtures("mock_confirm", "use_new_remote_build") +def test_determine_architectures_error(base, capsys, snapcraft_yaml, mocker): + """Error if `--build-for` is provided and archs are in the snapcraft.yaml.""" + mocker.patch.object( + sys, "argv", ["snapcraft", "remote-build", "--build-for", "amd64"] + ) + snapcraft_yaml( + base=base, architectures=[{"build-on": "arm64", "build-for": "arm64"}] + ) + + cli.run() + + _, err = capsys.readouterr() + assert ( + "Cannot use `--build-on` because architectures are already defined in " + "snapcraft.yaml." + ) in err + + ################## # Build id tests # ################## -# The build-id is not currently used, so these unit tests test the log output. -# When #4323 is complete, these tests can be rewritten to verify the build-id passed -# to the new remote-build code. - @pytest.mark.parametrize( "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True ) @pytest.mark.usefixtures( - "create_snapcraft_yaml", - "mock_confirm", - "mock_argv", - "mock_run_legacy", - "use_new_remote_build", + "create_snapcraft_yaml", "mock_confirm", "use_new_remote_build" ) -def test_build_id_provided(emitter, mocker): - """Use the build id provided as an argument.""" +def test_build_id_provided(mocker, mock_remote_builder): + """Pass the build id provided as an argument.""" mocker.patch.object( sys, "argv", ["snapcraft", "remote-build", "--build-id", "test-build-id"] ) cli.run() - emitter.assert_debug("Using build ID 'test-build-id' passed as a parameter.") + mock_remote_builder.assert_called_with( + app_name="snapcraft", + build_id="test-build-id", + project_name="mytest", + architectures=ANY, + project_dir=Path(), + ) @pytest.mark.parametrize( "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True ) @pytest.mark.usefixtures( - "create_snapcraft_yaml", - "mock_confirm", - "mock_argv", - "mock_run_legacy", - "use_new_remote_build", + "create_snapcraft_yaml", "mock_confirm", "mock_argv", "use_new_remote_build" ) -def test_build_id_computed(emitter): - """Compute the build id.""" +def test_build_id_not_provided(mock_remote_builder): + """Pass `None` for the build id if it is not provided as an argument.""" + cli.run() - # The create_snapcraft_yaml fixture uses the project name 'mytest'. - # Look for an md5 hash (a 32 character lowercase hex string). - emitter.assert_debug( - "Using computed build ID 'snapcraft-mytest-[0-9a-f]{32}'.", regex=True + mock_remote_builder.assert_called_with( + app_name="snapcraft", + build_id=None, + project_name="mytest", + architectures=ANY, + project_dir=Path(), ) @@ -545,3 +687,119 @@ def test_build_id_no_project_name_error(base, capsys): _, err = capsys.readouterr() assert "Could not get project name from 'snapcraft.yaml'." in err + + +######################## +# Remote builder tests # +######################## + + +@pytest.mark.parametrize( + "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True +) +@pytest.mark.usefixtures( + "create_snapcraft_yaml", "mock_confirm", "use_new_remote_build" +) +def test_status(mocker, mock_remote_builder): + """Print the status when `--status` is provided.""" + mocker.patch.object(sys, "argv", ["snapcraft", "remote-build", "--status"]) + + cli.run() + + assert mock_remote_builder.mock_calls[-1] == call().print_status() + + +@pytest.mark.parametrize( + "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True +) +@pytest.mark.usefixtures( + "create_snapcraft_yaml", + "mock_confirm", + "mock_remote_builder", + "use_new_remote_build", +) +def test_recover_no_build(emitter, mocker): + """Warn if no build is found when `--recover` is provided.""" + mocker.patch.object(sys, "argv", ["snapcraft", "remote-build", "--recover"]) + + cli.run() + + emitter.assert_message("No build found.") + + +@pytest.mark.parametrize( + "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True +) +@pytest.mark.usefixtures( + "create_snapcraft_yaml", "mock_confirm", "use_new_remote_build" +) +def test_recover_build(emitter, mocker, mock_remote_builder): + """Recover a build when `--recover` is provided.""" + mocker.patch.object(sys, "argv", ["snapcraft", "remote-build", "--recover"]) + mock_remote_builder.return_value.has_outstanding_build.return_value = True + + cli.run() + + assert mock_remote_builder.mock_calls[-3:] == [ + call().print_status(), + call().monitor_build(), + call().clean_build(), + ] + + +@pytest.mark.parametrize( + "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True +) +@pytest.mark.usefixtures( + "create_snapcraft_yaml", "mock_argv", "mock_confirm", "use_new_remote_build" +) +def test_recover_build_user_confirms(emitter, mocker, mock_remote_builder): + """Recover a build when a user confirms.""" + mock_remote_builder.return_value.has_outstanding_build.return_value = True + + cli.run() + + assert mock_remote_builder.mock_calls[-3:] == [ + call().print_status(), + call().monitor_build(), + call().clean_build(), + ] + + +@pytest.mark.parametrize( + "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True +) +@pytest.mark.usefixtures("create_snapcraft_yaml", "mock_argv", "use_new_remote_build") +def test_recover_build_user_denies(emitter, mocker, mock_remote_builder): + """Clean and start a new build when a user denies to recover an existing build.""" + mocker.patch( + # confirm data upload, deny build recovery + "snapcraft.commands.remote.confirm_with_user", + side_effect=[True, False], + ) + mock_remote_builder.return_value.has_outstanding_build.return_value = True + + cli.run() + + assert mock_remote_builder.mock_calls[-3:] == [ + call().start_build(), + call().monitor_build(), + call().clean_build(), + ] + + +@pytest.mark.parametrize( + "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True +) +@pytest.mark.usefixtures( + "create_snapcraft_yaml", "mock_argv", "mock_confirm", "use_new_remote_build" +) +def test_remote_build(emitter, mocker, mock_remote_builder): + """Clean and start a new build.""" + cli.run() + + assert mock_remote_builder.mock_calls[-3:] == [ + call().start_build(), + call().monitor_build(), + call().clean_build(), + ] diff --git a/tests/unit/remote/test_errors.py b/tests/unit/remote/test_errors.py index 0b8fa6842f..ef65549aef 100644 --- a/tests/unit/remote/test_errors.py +++ b/tests/unit/remote/test_errors.py @@ -59,3 +59,54 @@ def test_launchpad_https_error(): assert error.details == ( "Verify connectivity to https://api.launchpad.net and retry build." ) + + +def test_unsupported_architecture_error(): + """Test UnsupportedArchitectureError.""" + error = errors.UnsupportedArchitectureError(architectures=["amd64", "arm64"]) + + assert str(error) == ( + "Architecture not supported by the remote builder.\nThe following " + "architectures are not supported by the remote builder: ['amd64', 'arm64'].\n" + "Please remove them from the architecture list and try again." + ) + assert repr(error) == ( + "UnsupportedArchitectureError(brief='Architecture not supported by the remote " + "builder.', details=\"The following architectures are not supported by the " + "remote builder: ['amd64', 'arm64'].\\nPlease remove them from the " + 'architecture list and try again.")' + ) + + assert error.brief == "Architecture not supported by the remote builder." + assert error.details == ( + "The following architectures are not supported by the remote builder: " + "['amd64', 'arm64'].\nPlease remove them from the architecture list and " + "try again." + ) + + +def test_accept_public_upload_error(): + """Test AcceptPublicUploadError.""" + error = errors.AcceptPublicUploadError() + + assert str(error) == ( + "Cannot upload data to build servers.\nRemote build needs explicit " + "acknowledgement that data sent to build servers is public.\n" + "In non-interactive runs, please use the option " + "`--launchpad-accept-public-upload`." + ) + assert repr(error) == ( + "AcceptPublicUploadError(brief='Cannot upload data to build servers.', " + "details='Remote build needs explicit acknowledgement that data sent to build " + "servers is public.\\n" + "In non-interactive runs, please use the option " + "`--launchpad-accept-public-upload`.')" + ) + + assert error.brief == "Cannot upload data to build servers." + assert error.details == ( + "Remote build needs explicit acknowledgement that data sent to build servers " + "is public.\n" + "In non-interactive runs, please use the option " + "`--launchpad-accept-public-upload`." + ) diff --git a/tests/unit/remote/test_launchpad.py b/tests/unit/remote/test_launchpad.py index b50443d879..7446477e49 100644 --- a/tests/unit/remote/test_launchpad.py +++ b/tests/unit/remote/test_launchpad.py @@ -445,29 +445,6 @@ def test_get_build_status(launchpad_client): } -def test_git_repository_create_clean(mock_git_repo, launchpad_client): - mock_git_repo.return_value.is_clean.return_value = True - launchpad_client._gitify_repository(Path()) - - assert mock_git_repo.mock_calls == [ - call(Path()), - call().is_clean(), - ] - - -def test_git_repository_create_dirty(mock_git_repo, launchpad_client): - mock_git_repo.return_value.is_clean.return_value = False - - launchpad_client._gitify_repository(Path()) - - assert mock_git_repo.mock_calls == [ - call(Path()), - call().is_clean(), - call().add_all(), - call().commit(), - ] - - def test_push_source_tree(new_dir, mock_git_repo, launchpad_client): now = datetime.now(timezone.utc) @@ -483,18 +460,19 @@ def test_push_source_tree(new_dir, mock_git_repo, launchpad_client): mock_git_repo.assert_has_calls( [ - call.push_url( + call(Path()), + call().push_url( "https://user:access-token@git.launchpad.net/~user/+git/id/", "main", "HEAD", "access-token", - ) + ), ] ) def test_push_source_tree_error(new_dir, mock_git_repo, launchpad_client): - mock_git_repo.push_url.side_effect = errors.GitError("test error") + mock_git_repo.return_value.push_url.side_effect = errors.GitError("test error") with pytest.raises(errors.GitError): launchpad_client.push_source_tree(Path()) diff --git a/tests/unit/remote/test_remote_builder.py b/tests/unit/remote/test_remote_builder.py new file mode 100644 index 0000000000..d5f8590178 --- /dev/null +++ b/tests/unit/remote/test_remote_builder.py @@ -0,0 +1,186 @@ +# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- +# +# Copyright 2023 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3 as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Remote builder tests.""" + +import re +from pathlib import Path +from unittest.mock import call, patch + +import pytest + +from snapcraft.remote import RemoteBuilder, UnsupportedArchitectureError +from snapcraft.remote.utils import _SUPPORTED_ARCHS + + +@pytest.fixture(autouse=True) +def mock_launchpad_client(mocker): + """Returns a mocked LaunchpadClient.""" + _mock_launchpad_client = mocker.patch( + "snapcraft.remote.remote_builder.LaunchpadClient" + ) + _mock_launchpad_client.return_value.has_outstanding_build.return_value = False + _mock_launchpad_client.return_value.architectures = ["amd64"] + return _mock_launchpad_client + + +@pytest.fixture(autouse=True) +def mock_worktree(mocker): + """Returns a mocked WorkTree.""" + return mocker.patch("snapcraft.remote.remote_builder.WorkTree") + + +@pytest.fixture() +def fake_remote_builder(new_dir, mock_launchpad_client, mock_worktree): + """Returns a fake RemoteBuilder.""" + return RemoteBuilder( + app_name="test-app", + build_id="test-build-id", + project_name="test-project", + architectures=["amd64"], + project_dir=Path(), + ) + + +def test_remote_builder_init(mock_launchpad_client, mock_worktree): + """Verify remote builder is properly initialized.""" + RemoteBuilder( + app_name="test-app", + build_id="test-build-id", + project_name="test-project", + architectures=["amd64"], + project_dir=Path(), + ) + + assert mock_launchpad_client.mock_calls == [ + call( + app_name="test-app", + build_id="test-build-id", + project_name="test-project", + architectures=["amd64"], + ) + ] + assert mock_worktree.mock_calls == [ + call(app_name="test-app", build_id="test-build-id", project_dir=Path()) + ] + + +@pytest.mark.usefixtures("new_dir") +def test_build_id_computed(): + """Compute a build id if it is not provided.""" + remote_builder = RemoteBuilder( + app_name="test-app", + build_id=None, + project_name="test-project", + architectures=["amd64"], + project_dir=Path(), + ) + + assert re.match("test-app-test-project-[0-9a-f]{32}", remote_builder.build_id) + + +@pytest.mark.parametrize("archs", (["amd64"], _SUPPORTED_ARCHS)) +def test_validate_architectures_supported(archs): + """Supported architectures should not raise an error.""" + RemoteBuilder( + app_name="test-app", + build_id="test-build-id", + project_name="test-project", + architectures=archs, + project_dir=Path(), + ) + + +@pytest.mark.parametrize( + "archs", + [ + # unsupported + ["bad"], + # supported and unsupported + ["amd64", "bad"], + # multiple supported and unsupported + ["bad", "amd64", "bad2", "arm64"], + ], +) +def test_validate_architectures_unsupported(archs): + """Raise an error for unsupported architectures.""" + with pytest.raises(UnsupportedArchitectureError): + RemoteBuilder( + app_name="test-app", + build_id="test-build-id", + project_name="test-project", + architectures=archs, + project_dir=Path(), + ) + + +@patch("logging.Logger.info") +def test_print_status_builds_found( + mock_log, mock_launchpad_client, fake_remote_builder +): + """Print the status of a remote build.""" + mock_launchpad_client.return_value.has_outstanding_build.return_value = True + mock_launchpad_client.return_value.get_build_status.return_value = { + "amd64": "Needs building", + "arm64": "Currently building", + } + + fake_remote_builder.print_status() + + assert mock_log.mock_calls == [ + call("Build status for arch %s: %s", "amd64", "Needs building"), + call("Build status for arch %s: %s", "arm64", "Currently building"), + ] + + +@patch("logging.Logger.info") +def test_print_status_no_build_found(mock_log, fake_remote_builder): + """Print the status of a remote build.""" + fake_remote_builder.print_status() + + assert mock_log.mock_calls == [call("No build found.")] + + +@pytest.mark.parametrize("has_builds", (True, False)) +def test_has_outstanding_build(has_builds, fake_remote_builder, mock_launchpad_client): + """Check for outstanding builds.""" + mock_launchpad_client.return_value.has_outstanding_build.return_value = has_builds + + assert fake_remote_builder.has_outstanding_build() == has_builds + + +def test_monitor_build(fake_remote_builder, mock_launchpad_client): + """Monitor a build.""" + fake_remote_builder.monitor_build() + + mock_launchpad_client.return_value.monitor_build.assert_called_once() + + +def test_clean_build(fake_remote_builder, mock_launchpad_client, mock_worktree): + """Clean a build.""" + fake_remote_builder.clean_build() + + mock_launchpad_client.return_value.cleanup.assert_called_once() + mock_worktree.return_value.clean_cache.assert_called_once() + + +def test_start_build(fake_remote_builder, mock_launchpad_client, mock_worktree): + """Start a build.""" + fake_remote_builder.start_build() + + mock_worktree.return_value.init_repo.assert_called_once() + mock_launchpad_client.return_value.push_source_tree.assert_called_once() + mock_launchpad_client.return_value.start_build.assert_called_once() diff --git a/tests/unit/remote/test_utils.py b/tests/unit/remote/test_utils.py index 20d3533841..18068267e8 100644 --- a/tests/unit/remote/test_utils.py +++ b/tests/unit/remote/test_utils.py @@ -21,7 +21,89 @@ import pytest -from snapcraft.remote import get_build_id, rmtree +from snapcraft.remote import ( + UnsupportedArchitectureError, + get_build_id, + humanize_list, + rmtree, + validate_architectures, +) +from snapcraft.remote.utils import _SUPPORTED_ARCHS + +############################### +# validate architecture tests # +############################### + + +@pytest.mark.parametrize(("archs"), [["amd64"], _SUPPORTED_ARCHS]) +def test_validate_architectures(archs): + """Validate architectures.""" + assert validate_architectures(archs) is None + + +@pytest.mark.parametrize( + ("archs", "expected_archs"), + [ + # invalid arch + (["unknown"], ["unknown"]), + # valid and invalid archs + (["amd64", "unknown"], ["unknown"]), + # multiple invalid archs + (["unknown1", "unknown2"], ["unknown1", "unknown2"]), + # multiple valid and invalid archs + (["unknown1", "unknown2"], ["unknown1", "unknown2"]), + ], +) +def test_validate_architectures_error(archs, expected_archs): + """Raise an error if an unsupported architecture is passed.""" + with pytest.raises(UnsupportedArchitectureError) as raised: + validate_architectures(archs) + + assert ( + "The following architectures are not supported by the remote builder: " + f"{expected_archs}" + ) in str(raised.value) + + +################# +# Humanize List # +################# + + +@pytest.mark.parametrize( + "items,conjunction,expected", + ( + ([], "and", ""), + (["foo"], "and", "'foo'"), + (["foo", "bar"], "and", "'bar' and 'foo'"), + (["foo", "bar", "baz"], "and", "'bar', 'baz', and 'foo'"), + (["foo", "bar", "baz", "qux"], "and", "'bar', 'baz', 'foo', and 'qux'"), + ([], "or", ""), + (["foo"], "or", "'foo'"), + (["foo", "bar"], "or", "'bar' or 'foo'"), + (["foo", "bar", "baz"], "or", "'bar', 'baz', or 'foo'"), + (["foo", "bar", "baz", "qux"], "or", "'bar', 'baz', 'foo', or 'qux'"), + ), +) +def test_humanize_list(items, conjunction, expected): + """Test humanize_list.""" + assert humanize_list(items, conjunction) == expected + + +def test_humanize_list_sorted(): + """Verify `sort` parameter.""" + input_list = ["z", "a", "m test", "1"] + + # unsorted list is in the same order as the original list + expected_list_unsorted = "'z', 'a', 'm test', and '1'" + + # sorted list is sorted alphanumerically + expected_list_sorted = "'1', 'a', 'm test', and 'z'" + + assert humanize_list(input_list, "and") == expected_list_sorted + assert humanize_list(input_list, "and", sort=True) == expected_list_sorted + assert humanize_list(input_list, "and", sort=False) == expected_list_unsorted + ################## # build id tests # diff --git a/tests/unit/remote/test_worktree.py b/tests/unit/remote/test_worktree.py index 3af859293c..ebf0068d8c 100644 --- a/tests/unit/remote/test_worktree.py +++ b/tests/unit/remote/test_worktree.py @@ -17,12 +17,75 @@ """Unit tests for the worktree module.""" from pathlib import Path +from unittest.mock import call + +import pytest from snapcraft.remote import WorkTree -def test_worktree_init(new_dir): - """Test initialization of WorkTree.""" - worktree = WorkTree(Path()) +@pytest.fixture(autouse=True) +def mock_git_repo(mocker): + """Returns a mocked GitRepo.""" + return mocker.patch("snapcraft.remote.worktree.GitRepo") + + +@pytest.fixture(autouse=True) +def mock_base_directory(mocker, new_dir): + """Returns a mocked `xdg.BaseDirectory`.""" + _mock_base_directory = mocker.patch("snapcraft.remote.worktree.BaseDirectory") + _mock_base_directory.save_cache_path.return_value = new_dir + return _mock_base_directory + + +@pytest.fixture(autouse=True) +def mock_copytree(mocker): + """Returns a mocked `shutil.copytree()`.""" + return mocker.patch("snapcraft.remote.worktree.copytree") + + +def test_worktree_init_clean( + mock_base_directory, mock_copytree, mock_git_repo, new_dir +): + """Test initialization of a WorkTree with a clean git repository.""" + mock_git_repo.return_value.is_clean.return_value = True + + worktree = WorkTree(app_name="test-app", build_id="test-id", project_dir=Path()) + worktree.init_repo() assert isinstance(worktree, WorkTree) + mock_base_directory.save_cache_path.assert_called_with( + "test-app", "remote-build", "test-id" + ) + assert mock_git_repo.mock_calls == [ + call(Path().resolve() / "repo"), + call().is_clean(), + ] + + +def test_worktree_init_dirty( + mock_base_directory, mock_copytree, mock_git_repo, new_dir +): + """Test initialization of a WorkTree with a clean git repository.""" + mock_git_repo.return_value.is_clean.return_value = False + + worktree = WorkTree(app_name="test-app", build_id="test-id", project_dir=Path()) + worktree.init_repo() + + assert isinstance(worktree, WorkTree) + mock_base_directory.save_cache_path.assert_called_with( + "test-app", "remote-build", "test-id" + ) + assert mock_git_repo.mock_calls == [ + call(Path().resolve() / "repo"), + call().is_clean(), + call().add_all(), + call().commit(), + ] + + +def test_worktree_repo_dir(new_dir): + """Verify the `repo_dir` property.""" + worktree = WorkTree(app_name="test-app", build_id="test-id", project_dir=Path()) + + assert worktree.repo_dir == Path().resolve() / "repo"