From 13a4250c137ce1f9e0087b7a17cca434b8c32bff Mon Sep 17 00:00:00 2001 From: Ahmed Elbayaa Date: Wed, 21 Apr 2021 17:00:46 -0700 Subject: [PATCH 1/4] [Refactor] extract git-clone functionality out of InitTemplates class to its own class --- mypy.ini | 2 +- samcli/commands/init/init_templates.py | 148 +++------------ samcli/lib/utils/git_repo.py | 188 +++++++++++++++++++ tests/unit/commands/init/test_cli.py | 54 +++--- tests/unit/commands/init/test_templates.py | 67 ++----- tests/unit/lib/utils/test_git_repo.py | 206 +++++++++++++++++++++ 6 files changed, 467 insertions(+), 198 deletions(-) create mode 100644 samcli/lib/utils/git_repo.py create mode 100644 tests/unit/lib/utils/test_git_repo.py diff --git a/mypy.ini b/mypy.ini index 30040750a0..497c022c95 100644 --- a/mypy.ini +++ b/mypy.ini @@ -59,6 +59,6 @@ ignore_missing_imports=True ignore_missing_imports=True # progressive add typechecks and these modules already complete the process, let's keep them clean -[mypy-samcli.commands.build,samcli.lib.build.*,samcli.commands.local.cli_common.invoke_context,samcli.commands.local.lib.local_lambda,samcli.lib.providers.*] +[mypy-samcli.commands.build,samcli.lib.build.*,samcli.commands.local.cli_common.invoke_context,samcli.commands.local.lib.local_lambda,samcli.lib.providers.*,samcli.lib.utils.git_repo.py] disallow_untyped_defs=True disallow_incomplete_defs=True \ No newline at end of file diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index 303a2632af..d90ac00694 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -4,23 +4,18 @@ import itertools import json -import os import logging -import platform -import shutil -import subprocess - +import os from pathlib import Path from typing import Dict import click from samcli.cli.main import global_cfg -from samcli.commands.exceptions import UserException, AppTemplateUpdateException -from samcli.lib.utils import osutils -from samcli.lib.utils.osutils import rmtree_callback -from samcli.local.common.runtime_template import RUNTIME_DEP_TEMPLATE_MAPPING, get_local_lambda_images_location +from samcli.commands.exceptions import UserException +from samcli.lib.utils.git_repo import GitRepo, CloneRepoException, CloneRepoUnstableStateException from samcli.lib.utils.packagetype import IMAGE +from samcli.local.common.runtime_template import RUNTIME_DEP_TEMPLATE_MAPPING, get_local_lambda_images_location LOG = logging.getLogger(__name__) @@ -31,13 +26,12 @@ class InvalidInitTemplateError(UserException): class InitTemplates: def __init__(self, no_interactive=False, auto_clone=True): - self._repo_url = "https://github.com/aws/aws-sam-cli-app-templates" - self._repo_name = "aws-sam-cli-app-templates" - self._temp_repo_name = "TEMP-aws-sam-cli-app-templates" - self.repo_path = None - self.clone_attempted = False self._no_interactive = no_interactive - self._auto_clone = auto_clone + self._git_repo: GitRepo = GitRepo( + url="https://github.com/aws/aws-sam-cli-app-templates", + name="aws-sam-cli-app-templates", + auto_clone=auto_clone, + ) def prompt_for_location(self, package_type, runtime, base_image, dependency_manager): """ @@ -89,7 +83,7 @@ def prompt_for_location(self, package_type, runtime, base_image, dependency_mana if template_md.get("init_location") is not None: return (template_md["init_location"], template_md["appTemplate"]) if template_md.get("directory") is not None: - return (os.path.join(self.repo_path, template_md["directory"]), template_md["appTemplate"]) + return (os.path.join(self._git_repo.local_path, template_md["directory"]), template_md["appTemplate"]) raise InvalidInitTemplateError("Invalid template. This should not be possible, please raise an issue.") def location_from_app_template(self, package_type, runtime, base_image, dependency_manager, app_template): @@ -99,7 +93,7 @@ def location_from_app_template(self, package_type, runtime, base_image, dependen if template.get("init_location") is not None: return template["init_location"] if template.get("directory") is not None: - return os.path.join(self.repo_path, template["directory"]) + return os.path.join(self._git_repo.local_path, template["directory"]) raise InvalidInitTemplateError("Invalid template. This should not be possible, please raise an issue.") except StopIteration as ex: msg = "Can't find application template " + app_template + " - check valid values in interactive init." @@ -112,14 +106,23 @@ def _check_app_template(entry: Dict, app_template: str) -> bool: return bool(entry["appTemplate"] == app_template) def init_options(self, package_type, runtime, base_image, dependency_manager): - if not self.clone_attempted: - self._clone_repo() - if self.repo_path is None: + if not self._git_repo.clone_attempted: + shared_dir: Path = global_cfg.config_dir + try: + self._git_repo.clone(clone_dir=shared_dir, replace_existing=True) + except CloneRepoUnstableStateException as ex: + raise UserException(str(ex)) from ex + except (OSError, CloneRepoException): + # If can't clone, try using an old clone from a previous run if already exist + expected_previous_clone_local_path: Path = shared_dir.joinpath(self._git_repo.name) + if expected_previous_clone_local_path.exists(): + self._git_repo.local_path = expected_previous_clone_local_path + if self._git_repo.local_path is None: return self._init_options_from_bundle(package_type, runtime, dependency_manager) return self._init_options_from_manifest(package_type, runtime, base_image, dependency_manager) def _init_options_from_manifest(self, package_type, runtime, base_image, dependency_manager): - manifest_path = os.path.join(self.repo_path, "manifest.json") + manifest_path = os.path.join(self._git_repo.local_path, "manifest.json") with open(str(manifest_path)) as fp: body = fp.read() manifest_body = json.loads(body) @@ -154,109 +157,6 @@ def _init_options_from_bundle(package_type, runtime, dependency_manager): ) raise InvalidInitTemplateError(msg) - @staticmethod - def _shared_dir_check(shared_dir: Path) -> bool: - try: - shared_dir.mkdir(mode=0o700, parents=True, exist_ok=True) - return True - except OSError as ex: - LOG.warning("WARN: Unable to create shared directory.", exc_info=ex) - return False - - def _clone_repo(self): - if not self._auto_clone: - return # Unit test escape hatch - # check if we have templates stored already - shared_dir = global_cfg.config_dir - if not self._shared_dir_check(shared_dir): - # Nothing we can do if we can't access the shared config directory, use bundled. - return - expected_path = os.path.normpath(os.path.join(shared_dir, self._repo_name)) - if self._template_directory_exists(expected_path): - self._overwrite_existing_templates(expected_path) - else: - # simply create the app templates repo - self._clone_new_app_templates(shared_dir, expected_path) - self.clone_attempted = True - - def _overwrite_existing_templates(self, expected_path: str): - self.repo_path = expected_path - # workflow to clone a copy to a new directory and overwrite - with osutils.mkdir_temp(ignore_errors=True) as tempdir: - try: - expected_temp_path = os.path.normpath(os.path.join(tempdir, self._repo_name)) - LOG.info("\nCloning app templates from %s", self._repo_url) - subprocess.check_output( - [self._git_executable(), "clone", self._repo_url, self._repo_name], - cwd=tempdir, - stderr=subprocess.STDOUT, - ) - # Now we need to delete the old repo and move this one. - self._replace_app_templates(expected_temp_path, expected_path) - self.repo_path = expected_path - except OSError as ex: - LOG.warning("WARN: Could not clone app template repo.", exc_info=ex) - except subprocess.CalledProcessError as clone_error: - output = clone_error.output.decode("utf-8") - if "not found" in output.lower(): - click.echo("WARN: Could not clone app template repo.") - - @staticmethod - def _replace_app_templates(temp_path: str, dest_path: str) -> None: - try: - LOG.debug("Removing old templates from %s", dest_path) - shutil.rmtree(dest_path, onerror=rmtree_callback) - LOG.debug("Copying templates from %s to %s", temp_path, dest_path) - shutil.copytree(temp_path, dest_path, ignore=shutil.ignore_patterns("*.git")) - except (OSError, shutil.Error) as ex: - # UNSTABLE STATE - # it's difficult to see how this scenario could happen except weird permissions, user will need to debug - raise AppTemplateUpdateException( - "Unstable state when updating app templates. " - "Check that you have permissions to create/delete files in the AWS SAM shared directory " - "or file an issue at https://github.com/awslabs/aws-sam-cli/issues" - ) from ex - - def _clone_new_app_templates(self, shared_dir, expected_path): - with osutils.mkdir_temp(ignore_errors=True) as tempdir: - expected_temp_path = os.path.normpath(os.path.join(tempdir, self._repo_name)) - try: - LOG.info("\nCloning app templates from %s", self._repo_url) - subprocess.check_output( - [self._git_executable(), "clone", self._repo_url], - cwd=tempdir, - stderr=subprocess.STDOUT, - ) - shutil.copytree(expected_temp_path, expected_path, ignore=shutil.ignore_patterns("*.git")) - self.repo_path = expected_path - except OSError as ex: - LOG.warning("WARN: Can't clone app repo, git executable not found", exc_info=ex) - except subprocess.CalledProcessError as clone_error: - output = clone_error.output.decode("utf-8") - if "not found" in output.lower(): - click.echo("WARN: Could not clone app template repo.") - - @staticmethod - def _template_directory_exists(expected_path: str) -> bool: - path = Path(expected_path) - return path.exists() - - @staticmethod - def _git_executable() -> str: - execname = "git" - if platform.system().lower() == "windows": - options = [execname, "{}.cmd".format(execname), "{}.exe".format(execname), "{}.bat".format(execname)] - else: - options = [execname] - for name in options: - try: - subprocess.Popen([name], stdout=subprocess.PIPE, stderr=subprocess.PIPE) - # No exception. Let's pick this - return name - except OSError as ex: - LOG.debug("Unable to find executable %s", name, exc_info=ex) - raise OSError("Cannot find git, was looking at executables: {}".format(options)) - def is_dynamic_schemas_template(self, package_type, app_template, runtime, base_image, dependency_manager): """ Check if provided template is dynamic template e.g: AWS Schemas template. diff --git a/samcli/lib/utils/git_repo.py b/samcli/lib/utils/git_repo.py new file mode 100644 index 0000000000..076f0c3046 --- /dev/null +++ b/samcli/lib/utils/git_repo.py @@ -0,0 +1,188 @@ +""" Manage Git repo """ + +import logging +import os +import platform +import shutil +import subprocess +from pathlib import Path +from typing import Optional + +from samcli.lib.utils import osutils +from samcli.lib.utils.osutils import rmtree_callback + +LOG = logging.getLogger(__name__) + + +class CloneRepoException(Exception): + """ + Exception class when clone repo fails. + """ + + +class CloneRepoUnstableStateException(CloneRepoException): + """ + Exception class when clone repo enters an unstable state. + """ + + +class GitRepo: + """ + Class for managing a Git repo, currently it has a clone functionality only + + Attributes + ---------- + _url: str + The URL of this Git repository, example "https://github.com/aws/aws-sam-cli" + _name: str + The name of this Git repository, example "aws-sam-cli" + _local_path: Path + The path of the local clone of this Git repository + _clone_attempted: bool + whether an attempt to clone this Git repository took place or not + _auto_clone: bool + Just for unit-testing, set to false to skip cloning + + Methods + ------- + clone(self, clone_dir: Path, clone_name=None, replace_existing=False) -> Path: + creates a local clone of this Git repository + """ + + def __init__(self, url: str, name: str, auto_clone: bool = True) -> None: + self._url: str = url + self._name: str = name + self._local_path: Optional[Path] = None + self._clone_attempted: bool = False + self._auto_clone: bool = auto_clone + + @property + def url(self) -> str: + return self._url + + @property + def name(self) -> str: + return self._name + + @property + def local_path(self) -> Optional[Path]: + return self._local_path + + @local_path.setter + def local_path(self, value: Path) -> None: + self._local_path = value + + @property + def clone_attempted(self) -> bool: + return self._clone_attempted + + @clone_attempted.setter + def clone_attempted(self, value: bool) -> None: + self._clone_attempted = value + + @property + def auto_clone(self) -> bool: + return self._auto_clone + + @staticmethod + def _ensure_clone_directory_exists(clone_dir: Path) -> None: + try: + clone_dir.mkdir(mode=0o700, parents=True, exist_ok=True) + except OSError as ex: + LOG.warning("WARN: Unable to create clone directory.", exc_info=ex) + raise + + @staticmethod + def _git_executable() -> str: + if platform.system().lower() == "windows": + executables = ["git", "git.cmd", "git.exe", "git.bat"] + else: + executables = ["git"] + + for executable in executables: + try: + subprocess.Popen([executable], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + # No exception. Let's pick this + return executable + except OSError as ex: + LOG.debug("Unable to find executable %s", executable, exc_info=ex) + + raise OSError("Cannot find git, was looking at executables: {}".format(executables)) + + def clone(self, clone_dir: Path, clone_name: Optional[str] = None, replace_existing: bool = False) -> Path: + """ + creates a local clone of this Git repository + + Parameters + ---------- + clone_dir: Path + The directory to create the local clone inside + clone_name: Optional[str] + The dirname of the local clone, if not provided, repository's name (self._name) will be used + replace_existing: bool + Whether to replace the current local clone directory if already exists or not + + Returns + ------- + The path of the created local clone + + Raises + ------ + OSError: + when file management errors like unable to mkdir, copytree, rmtree ...etc + CloneRepoException: + General errors like for example; if an error occurred while running `git clone` + or if the local_clone already exists and replace_existing is not set + CloneRepoUnstableStateException: + when reaching unstable state, for example with replace_existing flag set, unstable state can happen + if removed the current local clone but failed to copy the new one from the temp location to the destination + """ + if not self._auto_clone: # Unit test escape hatch + return None # type: ignore + GitRepo._ensure_clone_directory_exists(clone_dir=clone_dir) + # clone to temp then move to the destination(repo_local_path) + with osutils.mkdir_temp(ignore_errors=True) as tempdir: + try: + clone_name = clone_name if clone_name else self._name + temp_path = os.path.normpath(os.path.join(tempdir, clone_name)) + git_executable: str = GitRepo._git_executable() + LOG.info("\nCloning from %s", self._url) + subprocess.check_output( + [git_executable, "clone", self._url, clone_name], + cwd=tempdir, + stderr=subprocess.STDOUT, + ) + self._local_path = self._persist_local_repo(temp_path, clone_dir, clone_name, replace_existing) + return self._local_path + except OSError as ex: + LOG.warning("WARN: Could not clone repo %s", self._url, exc_info=ex) + raise + except subprocess.CalledProcessError as clone_error: + output = clone_error.output.decode("utf-8") + if "not found" in output.lower(): + LOG.warning("WARN: Could not clone repo %s", self._url, exc_info=clone_error) + raise CloneRepoException from clone_error + finally: + self._clone_attempted = True + + @staticmethod + def _persist_local_repo(temp_path: str, dest_dir: Path, dest_name: str, replace_existing: bool) -> Path: + dest_path = os.path.normpath(dest_dir.joinpath(dest_name)) + try: + if Path(dest_path).exists(): + if not replace_existing: + raise CloneRepoException(f"Can not clone to {dest_path}, directory already exist") + LOG.debug("Removing old repo at %s", dest_path) + shutil.rmtree(dest_path, onerror=rmtree_callback) + + LOG.debug("Copying from %s to %s", temp_path, dest_path) + shutil.copytree(temp_path, dest_path, ignore=shutil.ignore_patterns("*.git")) + return Path(dest_path) + except (OSError, shutil.Error) as ex: + # UNSTABLE STATE + # it's difficult to see how this scenario could happen except weird permissions, user will need to debug + raise CloneRepoUnstableStateException( + "Unstable state when updating repo. " + f"Check that you have permissions to create/delete files in {dest_dir} directory " + "or file an issue at https://github.com/awslabs/aws-sam-cli/issues" + ) from ex diff --git a/tests/unit/commands/init/test_cli.py b/tests/unit/commands/init/test_cli.py index d80b3022a7..3931c958ef 100644 --- a/tests/unit/commands/init/test_cli.py +++ b/tests/unit/commands/init/test_cli.py @@ -1,3 +1,4 @@ +from pathlib import Path from unittest import TestCase from unittest.mock import patch, ANY @@ -5,22 +6,25 @@ import click from click.testing import CliRunner -from samcli.commands.init.init_templates import InitTemplates +from samcli.commands.exceptions import UserException from samcli.commands.init import cli as init_cmd from samcli.commands.init import do_cli as init_cli +from samcli.commands.init.init_templates import InitTemplates from samcli.lib.init import GenerateProjectFailedError -from samcli.commands.exceptions import UserException +from samcli.lib.utils.git_repo import GitRepo from samcli.lib.utils.packagetype import IMAGE, ZIP class MockInitTemplates: def __init__(self, no_interactive=False, auto_clone=True): - self._repo_url = "https://github.com/awslabs/aws-sam-cli-app-templates.git" - self._repo_name = "aws-sam-cli-app-templates" - self.repo_path = "repository" - self.clone_attempted = True self._no_interactive = no_interactive - self._auto_clone = auto_clone + self._git_repo: GitRepo = GitRepo( + url="https://github.com/awslabs/aws-sam-cli-app-templates.git", + name="aws-sam-cli-app-templates", + auto_clone=auto_clone, + ) + self._git_repo.clone_attempted = True + self._git_repo.local_path = Path("repository") class TestCli(TestCase): @@ -40,9 +44,9 @@ def setUp(self): self.extra_context = '{"project_name": "testing project", "runtime": "python3.6"}' self.extra_context_as_json = {"project_name": "testing project", "runtime": "python3.6"} - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_cli(self, generate_project_patch, sd_mock): + def test_init_cli(self, generate_project_patch, cd_mock): # GIVEN generate_project successfully created a project # WHEN a project name has been passed init_cli( @@ -75,9 +79,9 @@ def test_init_cli(self, generate_project_patch, sd_mock): self.extra_context_as_json, ) - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_image_cli(self, generate_project_patch, sd_mock): + def test_init_image_cli(self, generate_project_patch, cd_mock): # GIVEN generate_project successfully created a project # WHEN a project name has been passed init_cli( @@ -110,9 +114,9 @@ def test_init_image_cli(self, generate_project_patch, sd_mock): {"runtime": "nodejs12.x", "project_name": "testing project"}, ) - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_image_java_cli(self, generate_project_patch, sd_mock): + def test_init_image_java_cli(self, generate_project_patch, cd_mock): # GIVEN generate_project successfully created a project # WHEN a project name has been passed init_cli( @@ -145,8 +149,8 @@ def test_init_image_java_cli(self, generate_project_patch, sd_mock): {"runtime": "java11", "project_name": "testing project"}, ) - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") - def test_init_fails_invalid_template(self, sd_mock): + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") + def test_init_fails_invalid_template(self, cd_mock): # WHEN an unknown app template is passed in # THEN an exception should be raised with self.assertRaises(UserException): @@ -167,8 +171,8 @@ def test_init_fails_invalid_template(self, sd_mock): auto_clone=False, ) - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") - def test_init_fails_invalid_dep_mgr(self, sd_mock): + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") + def test_init_fails_invalid_dep_mgr(self, cd_mock): # WHEN an unknown app template is passed in # THEN an exception should be raised with self.assertRaises(UserException): @@ -189,9 +193,9 @@ def test_init_fails_invalid_dep_mgr(self, sd_mock): auto_clone=False, ) - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_cli_generate_project_fails(self, generate_project_patch, sd_mock): + def test_init_cli_generate_project_fails(self, generate_project_patch, cd_mock): # GIVEN generate_project fails to create a project generate_project_patch.side_effect = GenerateProjectFailedError( project=self.name, provider_error="Something wrong happened" @@ -221,9 +225,9 @@ def test_init_cli_generate_project_fails(self, generate_project_patch, sd_mock): self.location, self.runtime, self.dependency_manager, self.output_dir, self.name, self.no_input ) - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_cli_generate_project_image_fails(self, generate_project_patch, sd_mock): + def test_init_cli_generate_project_image_fails(self, generate_project_patch, cd_mock): # GIVEN generate_project fails to create a project generate_project_patch.side_effect = GenerateProjectFailedError( project=self.name, provider_error="Something wrong happened" @@ -1050,9 +1054,9 @@ def test_init_passes_dynamic_event_bridge_template(self, generate_project_patch, self.extra_context_as_json, ) - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_cli_int_from_location(self, generate_project_patch, sd_mock): + def test_init_cli_int_from_location(self, generate_project_patch, cd_mock): # WHEN the user follows interactive init prompts # 2: selecting custom location @@ -1079,9 +1083,9 @@ def test_init_cli_int_from_location(self, generate_project_patch, sd_mock): None, ) - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_cli_no_package_type(self, generate_project_patch, sd_mock): + def test_init_cli_no_package_type(self, generate_project_patch, cd_mock): # WHEN the user follows interactive init prompts # 1: selecting template source diff --git a/tests/unit/commands/init/test_templates.py b/tests/unit/commands/init/test_templates.py index b422b5dbf0..0e11d6aed9 100644 --- a/tests/unit/commands/init/test_templates.py +++ b/tests/unit/commands/init/test_templates.py @@ -1,23 +1,20 @@ import json import subprocess -import click - -from unittest.mock import mock_open, patch, PropertyMock, MagicMock +from pathlib import Path from re import search from unittest import TestCase -from samcli.lib.utils.packagetype import IMAGE, ZIP - -from pathlib import Path +from unittest.mock import mock_open, patch, PropertyMock, MagicMock from samcli.commands.init.init_templates import InitTemplates +from samcli.lib.utils.packagetype import IMAGE, ZIP class TestTemplates(TestCase): @patch("subprocess.check_output") - @patch("samcli.commands.init.init_templates.InitTemplates._git_executable") - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") + @patch("samcli.lib.utils.git_repo.GitRepo._git_executable") + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") @patch("shutil.copytree") - def test_location_from_app_template_zip(self, subprocess_mock, git_exec_mock, sd_mock, copy_mock): + def test_location_from_app_template_zip(self, subprocess_mock, git_exec_mock, cd_mock, copy_mock): it = InitTemplates(True) manifest = { @@ -35,16 +32,16 @@ def test_location_from_app_template_zip(self, subprocess_mock, git_exec_mock, sd m = mock_open(read_data=manifest_json) with patch("samcli.cli.global_config.GlobalConfig.config_dir", new_callable=PropertyMock) as mock_cfg: - mock_cfg.return_value = "/tmp/test-sam" + mock_cfg.return_value = Path("/tmp/test-sam") with patch("samcli.commands.init.init_templates.open", m): location = it.location_from_app_template(ZIP, "ruby2.5", None, "bundler", "hello-world") self.assertTrue(search("mock-ruby-template", location)) @patch("subprocess.check_output") - @patch("samcli.commands.init.init_templates.InitTemplates._git_executable") - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") + @patch("samcli.lib.utils.git_repo.GitRepo._git_executable") + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") @patch("shutil.copytree") - def test_location_from_app_template_image(self, subprocess_mock, git_exec_mock, sd_mock, copy_mock): + def test_location_from_app_template_image(self, subprocess_mock, git_exec_mock, cd_mock, copy_mock): it = InitTemplates(True) manifest = { @@ -62,63 +59,37 @@ def test_location_from_app_template_image(self, subprocess_mock, git_exec_mock, m = mock_open(read_data=manifest_json) with patch("samcli.cli.global_config.GlobalConfig.config_dir", new_callable=PropertyMock) as mock_cfg: - mock_cfg.return_value = "/tmp/test-sam" + mock_cfg.return_value = Path("/tmp/test-sam") with patch("samcli.commands.init.init_templates.open", m): location = it.location_from_app_template( IMAGE, None, "ruby2.5-image", "bundler", "hello-world-lambda-image" ) self.assertTrue(search("mock-ruby-image-template", location)) - @patch("samcli.commands.init.init_templates.InitTemplates._git_executable") + @patch("samcli.lib.utils.git_repo.GitRepo._git_executable") @patch("click.prompt") - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") - def test_fallback_options(self, git_exec_mock, prompt_mock, sd_mock): + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") + def test_fallback_options(self, git_exec_mock, prompt_mock, cd_mock): prompt_mock.return_value = "1" with patch("subprocess.check_output", new_callable=MagicMock) as mock_sub: with patch("samcli.cli.global_config.GlobalConfig.config_dir", new_callable=PropertyMock) as mock_cfg: mock_sub.side_effect = OSError("Fail") - mock_cfg.return_value = "/tmp/test-sam" + mock_cfg.return_value = Path("/tmp/test-sam") it = InitTemplates(True) location, app_template = it.prompt_for_location(ZIP, "ruby2.5", None, "bundler") self.assertTrue(search("cookiecutter-aws-sam-hello-ruby", location)) self.assertEqual("hello-world", app_template) - @patch("samcli.commands.init.init_templates.InitTemplates._git_executable") + @patch("samcli.lib.utils.git_repo.GitRepo._git_executable") @patch("click.prompt") - @patch("samcli.commands.init.init_templates.InitTemplates._shared_dir_check") - def test_fallback_process_error(self, git_exec_mock, prompt_mock, sd_mock): + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") + def test_fallback_process_error(self, git_exec_mock, prompt_mock, cd_mock): prompt_mock.return_value = "1" with patch("subprocess.check_output", new_callable=MagicMock) as mock_sub: with patch("samcli.cli.global_config.GlobalConfig.config_dir", new_callable=PropertyMock) as mock_cfg: mock_sub.side_effect = subprocess.CalledProcessError("fail", "fail", "not found".encode("utf-8")) - mock_cfg.return_value = "/tmp/test-sam" + mock_cfg.return_value = Path("/tmp/test-sam") it = InitTemplates(True) location, app_template = it.prompt_for_location(ZIP, "ruby2.5", None, "bundler") self.assertTrue(search("cookiecutter-aws-sam-hello-ruby", location)) self.assertEqual("hello-world", app_template) - - def test_git_executable_windows(self): - with patch("platform.system", new_callable=MagicMock) as mock_platform: - mock_platform.return_value = "Windows" - with patch("subprocess.Popen", new_callable=MagicMock) as mock_popen: - it = InitTemplates(True) - executable = it._git_executable() - self.assertEqual(executable, "git") - - def test_git_executable_fails(self): - with patch("subprocess.Popen", new_callable=MagicMock) as mock_popen: - mock_popen.side_effect = OSError("fail") - it = InitTemplates(True) - with self.assertRaises(OSError): - executable = it._git_executable() - - def test_shared_dir_check(self): - it = InitTemplates(True, False) - shared_dir_mock = MagicMock() - self.assertTrue(it._shared_dir_check(shared_dir_mock)) - - def test_shared_dir_failure(self): - it = InitTemplates(True, False) - shared_dir_mock = MagicMock() - shared_dir_mock.mkdir.side_effect = OSError("fail") - self.assertFalse(it._shared_dir_check(shared_dir_mock)) diff --git a/tests/unit/lib/utils/test_git_repo.py b/tests/unit/lib/utils/test_git_repo.py new file mode 100644 index 0000000000..8a905ba742 --- /dev/null +++ b/tests/unit/lib/utils/test_git_repo.py @@ -0,0 +1,206 @@ +import subprocess +from pathlib import Path +from unittest import TestCase +from unittest.mock import patch, MagicMock, ANY, call + +from samcli.lib.utils.git_repo import GitRepo, rmtree_callback, CloneRepoException, CloneRepoUnstableStateException + +REPO_URL = "REPO URL" +REPO_NAME = "REPO NAME" +CLONE_DIR = "/tmp/local/clone/dir" +EXPECTED_DEFAULT_CLONE_PATH = f"{CLONE_DIR}/{REPO_NAME}" + + +class TestGitRepo(TestCase): + def setUp(self): + self.repo = GitRepo(url=REPO_URL, name=REPO_NAME, auto_clone=True) + self.local_clone_dir = MagicMock() + self.local_clone_dir.joinpath.side_effect = lambda sub_dir: f"{CLONE_DIR}/{sub_dir}" + + def test_ensure_clone_directory_exists(self): + self.repo._ensure_clone_directory_exists(self.local_clone_dir) # No exception is thrown + self.local_clone_dir.mkdir.assert_called_once_with(mode=0o700, parents=True, exist_ok=True) + + def test_ensure_clone_directory_exists_fail(self): + self.local_clone_dir.mkdir.side_effect = OSError + with self.assertRaises(OSError): + self.repo._ensure_clone_directory_exists(self.local_clone_dir) + + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_git_executable_not_windows(self, mock_platform, mock_popen): + mock_platform.return_value = "Not Windows" + executable = self.repo._git_executable() + self.assertEqual(executable, "git") + + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_git_executable_windows(self, mock_platform, mock_popen): + mock_platform.return_value = "Windows" + executable = self.repo._git_executable() + self.assertEqual(executable, "git") + + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + def test_git_executable_fails(self, mock_popen): + mock_popen.side_effect = OSError("fail") + with self.assertRaises(OSError): + self.repo._git_executable() + + @patch("samcli.lib.utils.git_repo.osutils") + @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") + def test_clone_will_return_immediately_if_auto_clone_flag_is_set_to_false(self, cd_mock, osutils_mock): + repo = GitRepo(url=REPO_URL, name=REPO_NAME, auto_clone=False) + repo.clone(clone_dir=self.local_clone_dir) + cd_mock.assert_not_called() + osutils_mock.mkdir_temp.assert_not_called() + + @patch("samcli.lib.utils.git_repo.shutil") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_will_use_repo_name_by_default(self, platform_mock, popen_mock, check_output_mock, shutil_mock): + local_path: Path = self.repo.clone(clone_dir=self.local_clone_dir) + self.assertTrue(REPO_NAME in str(local_path)) + local_path = self.repo.clone(clone_dir=self.local_clone_dir, clone_name="REPO NOT NAME") + self.assertFalse(REPO_NAME in str(local_path)) + + @patch("samcli.lib.utils.git_repo.Path.exists") + @patch("samcli.lib.utils.git_repo.shutil") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_happy_case(self, platform_mock, popen_mock, check_output_mock, shutil_mock, path_exist_mock): + path_exist_mock.return_value = False + self.repo.clone(clone_dir=self.local_clone_dir) + self.local_clone_dir.mkdir.assert_called_once_with(mode=0o700, parents=True, exist_ok=True) + popen_mock.assert_called_once_with(["git"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + check_output_mock.assert_has_calls( + [call(["git", "clone", self.repo.url, self.repo.name], cwd=ANY, stderr=subprocess.STDOUT)] + ) + shutil_mock.rmtree.assert_not_called() + shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY) + shutil_mock.ignore_patterns.assert_called_with("*.git") + + @patch("samcli.lib.utils.git_repo.Path.exists") + @patch("samcli.lib.utils.git_repo.shutil") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_create_new_local_repo( + self, platform_mock, popen_mock, check_output_mock, shutil_mock, path_exist_mock + ): + path_exist_mock.return_value = False + self.repo.clone(clone_dir=self.local_clone_dir) + shutil_mock.rmtree.assert_not_called() + shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY) + shutil_mock.ignore_patterns.assert_called_with("*.git") + + @patch("samcli.lib.utils.git_repo.Path.exists") + @patch("samcli.lib.utils.git_repo.shutil") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_replace_current_local_repo_if_replace_existing_flag_is_set( + self, platform_mock, popen_mock, check_output_mock, shutil_mock, path_exist_mock + ): + path_exist_mock.return_value = True + self.repo.clone(clone_dir=self.local_clone_dir, replace_existing=True) + self.local_clone_dir.mkdir.assert_called_once_with(mode=0o700, parents=True, exist_ok=True) + shutil_mock.rmtree.assert_called_with(EXPECTED_DEFAULT_CLONE_PATH, onerror=rmtree_callback) + shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY) + shutil_mock.ignore_patterns.assert_called_with("*.git") + + @patch("samcli.lib.utils.git_repo.Path.exists") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_fail_if_current_local_repo_exists_and_replace_existing_flag_is_not_set( + self, platform_mock, popen_mock, check_output_mock, path_exist_mock + ): + path_exist_mock.return_value = True + with self.assertRaises(CloneRepoException): + self.repo.clone(clone_dir=self.local_clone_dir) # replace_existing=False by default + + @patch("samcli.lib.utils.git_repo.shutil") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_attempt_is_set_to_true_after_clone(self, platform_mock, popen_mock, check_output_mock, shutil_mock): + self.assertFalse(self.repo.clone_attempted) + self.repo.clone(clone_dir=self.local_clone_dir) + self.assertTrue(self.repo.clone_attempted) + + @patch("samcli.lib.utils.git_repo.shutil") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_attempt_is_set_to_true_even_if_clone_failed( + self, platform_mock, popen_mock, check_output_mock, shutil_mock + ): + check_output_mock.side_effect = subprocess.CalledProcessError("fail", "fail", "not found".encode("utf-8")) + self.assertFalse(self.repo.clone_attempted) + try: + with self.assertRaises(CloneRepoException): + self.repo.clone(clone_dir=self.local_clone_dir) + except: + pass + self.assertTrue(self.repo.clone_attempted) + + @patch("samcli.lib.utils.git_repo.shutil") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_failed_to_create_the_clone_directory( + self, platform_mock, popen_mock, check_output_mock, shutil_mock + ): + self.local_clone_dir.mkdir.side_effect = OSError + try: + with self.assertRaises(OSError): + self.repo.clone(clone_dir=self.local_clone_dir) + except: + pass + self.local_clone_dir.mkdir.assert_called_once_with(mode=0o700, parents=True, exist_ok=True) + popen_mock.assert_not_called() + check_output_mock.assert_not_called() + shutil_mock.assert_not_called() + + @patch("samcli.lib.utils.git_repo.shutil") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_when_the_subprocess_fail(self, platform_mock, popen_mock, check_output_mock, shutil_mock): + check_output_mock.side_effect = subprocess.CalledProcessError("fail", "fail", "any reason".encode("utf-8")) + with self.assertRaises(CloneRepoException): + self.repo.clone(clone_dir=self.local_clone_dir) + + @patch("samcli.lib.utils.git_repo.LOG") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_when_the_git_repo_not_found(self, platform_mock, popen_mock, check_output_mock, log_mock): + check_output_mock.side_effect = subprocess.CalledProcessError("fail", "fail", "not found".encode("utf-8")) + try: + with self.assertRaises(CloneRepoException): + self.repo.clone(clone_dir=self.local_clone_dir) + except Exception: + pass + log_mock.warning.assert_called() + + @patch("samcli.lib.utils.git_repo.Path.exists") + @patch("samcli.lib.utils.git_repo.shutil") + @patch("samcli.lib.utils.git_repo.subprocess.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_when_failed_to_move_cloned_repo_from_temp_to_final_destination( + self, platform_mock, popen_mock, check_output_mock, shutil_mock, path_exist_mock + ): + path_exist_mock.return_value = True + shutil_mock.copytree.side_effect = OSError + try: + with self.assertRaises(CloneRepoUnstableStateException): + self.repo.clone(clone_dir=self.local_clone_dir, replace_existing=True) + except Exception: + pass + shutil_mock.rmtree.assert_called_once_with(EXPECTED_DEFAULT_CLONE_PATH, onerror=rmtree_callback) + shutil_mock.copytree.assert_called_once_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY) + shutil_mock.ignore_patterns.assert_called_once_with("*.git") From a5109654208f1f03a52d3bf152bbbfe82936cad3 Mon Sep 17 00:00:00 2001 From: Ahmed Elbayaa Date: Thu, 22 Apr 2021 11:51:48 -0700 Subject: [PATCH 2/4] apply review comments --- samcli/commands/init/__init__.py | 3 +- samcli/commands/init/init_templates.py | 7 ++-- samcli/lib/utils/git_repo.py | 12 ++----- tests/unit/commands/init/test_cli.py | 47 ++++++++------------------ tests/unit/lib/utils/test_git_repo.py | 16 +++------ 5 files changed, 25 insertions(+), 60 deletions(-) diff --git a/samcli/commands/init/__init__.py b/samcli/commands/init/__init__.py index ebf1d18fff..f53b77daa5 100644 --- a/samcli/commands/init/__init__.py +++ b/samcli/commands/init/__init__.py @@ -256,7 +256,6 @@ def do_cli( app_template, no_input, extra_context, - auto_clone=True, ): """ Implementation of the ``cli`` method @@ -274,7 +273,7 @@ def do_cli( image_bool = name and pt_explicit and base_image if location or zip_bool or image_bool: # need to turn app_template into a location before we generate - templates = InitTemplates(no_interactive, auto_clone) + templates = InitTemplates(no_interactive) if package_type == IMAGE and image_bool: base_image, runtime = _get_runtime_from_image(base_image) options = templates.init_options(package_type, runtime, base_image, dependency_manager) diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index d90ac00694..28b7bed019 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -12,7 +12,7 @@ import click from samcli.cli.main import global_cfg -from samcli.commands.exceptions import UserException +from samcli.commands.exceptions import UserException, AppTemplateUpdateException from samcli.lib.utils.git_repo import GitRepo, CloneRepoException, CloneRepoUnstableStateException from samcli.lib.utils.packagetype import IMAGE from samcli.local.common.runtime_template import RUNTIME_DEP_TEMPLATE_MAPPING, get_local_lambda_images_location @@ -25,12 +25,11 @@ class InvalidInitTemplateError(UserException): class InitTemplates: - def __init__(self, no_interactive=False, auto_clone=True): + def __init__(self, no_interactive=False): self._no_interactive = no_interactive self._git_repo: GitRepo = GitRepo( url="https://github.com/aws/aws-sam-cli-app-templates", name="aws-sam-cli-app-templates", - auto_clone=auto_clone, ) def prompt_for_location(self, package_type, runtime, base_image, dependency_manager): @@ -111,7 +110,7 @@ def init_options(self, package_type, runtime, base_image, dependency_manager): try: self._git_repo.clone(clone_dir=shared_dir, replace_existing=True) except CloneRepoUnstableStateException as ex: - raise UserException(str(ex)) from ex + raise AppTemplateUpdateException(str(ex)) from ex except (OSError, CloneRepoException): # If can't clone, try using an old clone from a previous run if already exist expected_previous_clone_local_path: Path = shared_dir.joinpath(self._git_repo.name) diff --git a/samcli/lib/utils/git_repo.py b/samcli/lib/utils/git_repo.py index 076f0c3046..c1fb828819 100644 --- a/samcli/lib/utils/git_repo.py +++ b/samcli/lib/utils/git_repo.py @@ -40,8 +40,6 @@ class GitRepo: The path of the local clone of this Git repository _clone_attempted: bool whether an attempt to clone this Git repository took place or not - _auto_clone: bool - Just for unit-testing, set to false to skip cloning Methods ------- @@ -49,12 +47,11 @@ class GitRepo: creates a local clone of this Git repository """ - def __init__(self, url: str, name: str, auto_clone: bool = True) -> None: + def __init__(self, url: str, name: str) -> None: self._url: str = url self._name: str = name self._local_path: Optional[Path] = None self._clone_attempted: bool = False - self._auto_clone: bool = auto_clone @property def url(self) -> str: @@ -80,10 +77,6 @@ def clone_attempted(self) -> bool: def clone_attempted(self, value: bool) -> None: self._clone_attempted = value - @property - def auto_clone(self) -> bool: - return self._auto_clone - @staticmethod def _ensure_clone_directory_exists(clone_dir: Path) -> None: try: @@ -137,8 +130,7 @@ def clone(self, clone_dir: Path, clone_name: Optional[str] = None, replace_exist when reaching unstable state, for example with replace_existing flag set, unstable state can happen if removed the current local clone but failed to copy the new one from the temp location to the destination """ - if not self._auto_clone: # Unit test escape hatch - return None # type: ignore + GitRepo._ensure_clone_directory_exists(clone_dir=clone_dir) # clone to temp then move to the destination(repo_local_path) with osutils.mkdir_temp(ignore_errors=True) as tempdir: diff --git a/tests/unit/commands/init/test_cli.py b/tests/unit/commands/init/test_cli.py index 3931c958ef..d935415700 100644 --- a/tests/unit/commands/init/test_cli.py +++ b/tests/unit/commands/init/test_cli.py @@ -16,12 +16,11 @@ class MockInitTemplates: - def __init__(self, no_interactive=False, auto_clone=True): + def __init__(self, no_interactive=False): self._no_interactive = no_interactive self._git_repo: GitRepo = GitRepo( url="https://github.com/awslabs/aws-sam-cli-app-templates.git", name="aws-sam-cli-app-templates", - auto_clone=auto_clone, ) self._git_repo.clone_attempted = True self._git_repo.local_path = Path("repository") @@ -44,9 +43,9 @@ def setUp(self): self.extra_context = '{"project_name": "testing project", "runtime": "python3.6"}' self.extra_context_as_json = {"project_name": "testing project", "runtime": "python3.6"} - @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") + @patch("samcli.lib.utils.git_repo.GitRepo.clone") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_cli(self, generate_project_patch, cd_mock): + def test_init_cli(self, generate_project_patch, git_repo_clone_mock): # GIVEN generate_project successfully created a project # WHEN a project name has been passed init_cli( @@ -63,7 +62,6 @@ def test_init_cli(self, generate_project_patch, cd_mock): app_template=self.app_template, no_input=self.no_input, extra_context=None, - auto_clone=False, ) # THEN we should receive no errors @@ -79,9 +77,9 @@ def test_init_cli(self, generate_project_patch, cd_mock): self.extra_context_as_json, ) - @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") + @patch("samcli.lib.utils.git_repo.GitRepo.clone") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_image_cli(self, generate_project_patch, cd_mock): + def test_init_image_cli(self, generate_project_patch, git_repo_clone_mock): # GIVEN generate_project successfully created a project # WHEN a project name has been passed init_cli( @@ -98,7 +96,6 @@ def test_init_image_cli(self, generate_project_patch, cd_mock): app_template=None, no_input=self.no_input, extra_context=None, - auto_clone=False, ) # THEN we should receive no errors @@ -114,9 +111,9 @@ def test_init_image_cli(self, generate_project_patch, cd_mock): {"runtime": "nodejs12.x", "project_name": "testing project"}, ) - @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") + @patch("samcli.lib.utils.git_repo.GitRepo.clone") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_image_java_cli(self, generate_project_patch, cd_mock): + def test_init_image_java_cli(self, generate_project_patch, git_repo_clone_mock): # GIVEN generate_project successfully created a project # WHEN a project name has been passed init_cli( @@ -133,7 +130,6 @@ def test_init_image_java_cli(self, generate_project_patch, cd_mock): app_template=None, no_input=self.no_input, extra_context=None, - auto_clone=False, ) # THEN we should receive no errors @@ -149,8 +145,8 @@ def test_init_image_java_cli(self, generate_project_patch, cd_mock): {"runtime": "java11", "project_name": "testing project"}, ) - @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") - def test_init_fails_invalid_template(self, cd_mock): + @patch("samcli.lib.utils.git_repo.GitRepo.clone") + def test_init_fails_invalid_template(self, git_repo_clone_mock): # WHEN an unknown app template is passed in # THEN an exception should be raised with self.assertRaises(UserException): @@ -168,11 +164,10 @@ def test_init_fails_invalid_template(self, cd_mock): app_template="wrong-and-bad", no_input=self.no_input, extra_context=None, - auto_clone=False, ) - @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") - def test_init_fails_invalid_dep_mgr(self, cd_mock): + @patch("samcli.lib.utils.git_repo.GitRepo.clone") + def test_init_fails_invalid_dep_mgr(self, git_repo_clone_mock): # WHEN an unknown app template is passed in # THEN an exception should be raised with self.assertRaises(UserException): @@ -190,12 +185,11 @@ def test_init_fails_invalid_dep_mgr(self, cd_mock): app_template=self.app_template, no_input=self.no_input, extra_context=None, - auto_clone=False, ) - @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") + @patch("samcli.lib.utils.git_repo.GitRepo.clone") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_cli_generate_project_fails(self, generate_project_patch, cd_mock): + def test_init_cli_generate_project_fails(self, generate_project_patch, git_repo_clone_mock): # GIVEN generate_project fails to create a project generate_project_patch.side_effect = GenerateProjectFailedError( project=self.name, provider_error="Something wrong happened" @@ -218,16 +212,15 @@ def test_init_cli_generate_project_fails(self, generate_project_patch, cd_mock): app_template=None, no_input=self.no_input, extra_context=None, - auto_clone=False, ) generate_project_patch.assert_called_with( self.location, self.runtime, self.dependency_manager, self.output_dir, self.name, self.no_input ) - @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") + @patch("samcli.lib.utils.git_repo.GitRepo.clone") @patch("samcli.commands.init.init_generator.generate_project") - def test_init_cli_generate_project_image_fails(self, generate_project_patch, cd_mock): + def test_init_cli_generate_project_image_fails(self, generate_project_patch, git_repo_clone_mock): # GIVEN generate_project fails to create a project generate_project_patch.side_effect = GenerateProjectFailedError( project=self.name, provider_error="Something wrong happened" @@ -250,7 +243,6 @@ def test_init_cli_generate_project_image_fails(self, generate_project_patch, cd_ app_template=None, no_input=self.no_input, extra_context=None, - auto_clone=False, ) generate_project_patch.assert_called_with( @@ -275,7 +267,6 @@ def test_init_cli_with_extra_context_parameter_not_passed(self, generate_project app_template=self.app_template, no_input=self.no_input, extra_context=None, - auto_clone=False, ) # THEN we should receive no errors @@ -301,7 +292,6 @@ def test_init_cli_with_extra_context_parameter_passed(self, generate_project_pat app_template=self.app_template, no_input=self.no_input, extra_context='{"schema_name":"events", "schema_type":"aws"}', - auto_clone=False, ) # THEN we should receive no errors and right extra_context should be passed @@ -334,7 +324,6 @@ def test_init_cli_with_extra_context_not_overriding_default_parameter(self, gene app_template=self.app_template, no_input=self.no_input, extra_context='{"project_name": "my_project", "runtime": "java8", "schema_name":"events", "schema_type": "aws"}', - auto_clone=False, ) # THEN extra_context should have not overridden default_parameters(name, runtime) @@ -367,7 +356,6 @@ def test_init_cli_with_extra_context_input_as_wrong_json_raises_exception(self): app_template=self.app_template, no_input=self.no_input, extra_context='{"project_name", "my_project", "runtime": "java8", "schema_name":"events", "schema_type": "aws"}', - auto_clone=False, ) @patch("samcli.commands.init.init_generator.generate_project") @@ -388,7 +376,6 @@ def test_init_cli_must_set_default_context_when_location_is_provided(self, gener app_template=None, no_input=None, extra_context='{"schema_name":"events", "schema_type": "aws"}', - auto_clone=False, ) # THEN should set default parameter(name, runtime) as extra_context @@ -421,7 +408,6 @@ def test_init_cli_must_only_set_passed_project_name_when_location_is_provided(se app_template=None, no_input=None, extra_context='{"schema_name":"events", "schema_type": "aws"}', - auto_clone=False, ) # THEN extra_context should be without runtime @@ -454,7 +440,6 @@ def test_init_cli_must_only_set_passed_runtime_when_location_is_provided(self, g app_template=None, no_input=None, extra_context='{"schema_name":"events", "schema_type": "aws"}', - auto_clone=False, ) # THEN extra_context should be without name @@ -489,7 +474,6 @@ def test_init_cli_with_extra_context_parameter_passed_as_escaped(self, generate_ # fmt: off extra_context='{\"schema_name\":\"events\", \"schema_type\":\"aws\"}', # fmt: on - auto_clone=False, ) # THEN we should receive no errors and right extra_context should be passed @@ -1039,7 +1023,6 @@ def test_init_passes_dynamic_event_bridge_template(self, generate_project_patch, app_template="eventBridge-schema-app", no_input=self.no_input, extra_context=None, - auto_clone=False, ) generate_project_patch.assert_called_once_with( diff --git a/tests/unit/lib/utils/test_git_repo.py b/tests/unit/lib/utils/test_git_repo.py index 8a905ba742..259277012f 100644 --- a/tests/unit/lib/utils/test_git_repo.py +++ b/tests/unit/lib/utils/test_git_repo.py @@ -2,20 +2,20 @@ from pathlib import Path from unittest import TestCase from unittest.mock import patch, MagicMock, ANY, call - +import os from samcli.lib.utils.git_repo import GitRepo, rmtree_callback, CloneRepoException, CloneRepoUnstableStateException REPO_URL = "REPO URL" REPO_NAME = "REPO NAME" -CLONE_DIR = "/tmp/local/clone/dir" +CLONE_DIR = os.path.normpath("/tmp/local/clone/dir") EXPECTED_DEFAULT_CLONE_PATH = f"{CLONE_DIR}/{REPO_NAME}" class TestGitRepo(TestCase): def setUp(self): - self.repo = GitRepo(url=REPO_URL, name=REPO_NAME, auto_clone=True) + self.repo = GitRepo(url=REPO_URL, name=REPO_NAME) self.local_clone_dir = MagicMock() - self.local_clone_dir.joinpath.side_effect = lambda sub_dir: f"{CLONE_DIR}/{sub_dir}" + self.local_clone_dir.joinpath.side_effect = lambda sub_dir: os.path.normpath(os.path.join(CLONE_DIR, sub_dir)) def test_ensure_clone_directory_exists(self): self.repo._ensure_clone_directory_exists(self.local_clone_dir) # No exception is thrown @@ -46,14 +46,6 @@ def test_git_executable_fails(self, mock_popen): with self.assertRaises(OSError): self.repo._git_executable() - @patch("samcli.lib.utils.git_repo.osutils") - @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") - def test_clone_will_return_immediately_if_auto_clone_flag_is_set_to_false(self, cd_mock, osutils_mock): - repo = GitRepo(url=REPO_URL, name=REPO_NAME, auto_clone=False) - repo.clone(clone_dir=self.local_clone_dir) - cd_mock.assert_not_called() - osutils_mock.mkdir_temp.assert_not_called() - @patch("samcli.lib.utils.git_repo.shutil") @patch("samcli.lib.utils.git_repo.subprocess.check_output") @patch("samcli.lib.utils.git_repo.subprocess.Popen") From 3fb3a3e41a1f14884176db1e48fcee6ee7926fd2 Mon Sep 17 00:00:00 2001 From: Ahmed Elbayaa Date: Thu, 22 Apr 2021 12:52:32 -0700 Subject: [PATCH 3/4] typo --- samcli/lib/utils/git_repo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samcli/lib/utils/git_repo.py b/samcli/lib/utils/git_repo.py index c1fb828819..2afa171160 100644 --- a/samcli/lib/utils/git_repo.py +++ b/samcli/lib/utils/git_repo.py @@ -176,5 +176,5 @@ def _persist_local_repo(temp_path: str, dest_dir: Path, dest_name: str, replace_ raise CloneRepoUnstableStateException( "Unstable state when updating repo. " f"Check that you have permissions to create/delete files in {dest_dir} directory " - "or file an issue at https://github.com/awslabs/aws-sam-cli/issues" + "or file an issue at https://github.com/aws/aws-sam-cli/issues" ) from ex From 2a35819042ab4491f3248c21e1ba1e15a3a01b16 Mon Sep 17 00:00:00 2001 From: Ahmed Elbayaa Date: Thu, 22 Apr 2021 16:46:56 -0700 Subject: [PATCH 4/4] apply review comments --- samcli/commands/init/init_templates.py | 13 ++--- samcli/lib/utils/git_repo.py | 80 ++++++++++---------------- tests/unit/commands/init/test_cli.py | 5 +- tests/unit/lib/utils/test_git_repo.py | 36 +++++------- 4 files changed, 51 insertions(+), 83 deletions(-) diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index 28b7bed019..fee1a22ce6 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -18,6 +18,8 @@ from samcli.local.common.runtime_template import RUNTIME_DEP_TEMPLATE_MAPPING, get_local_lambda_images_location LOG = logging.getLogger(__name__) +APP_TEMPLATES_REPO_URL = "https://github.com/aws/aws-sam-cli-app-templates" +APP_TEMPLATES_REPO_NAME = "aws-sam-cli-app-templates" class InvalidInitTemplateError(UserException): @@ -27,10 +29,7 @@ class InvalidInitTemplateError(UserException): class InitTemplates: def __init__(self, no_interactive=False): self._no_interactive = no_interactive - self._git_repo: GitRepo = GitRepo( - url="https://github.com/aws/aws-sam-cli-app-templates", - name="aws-sam-cli-app-templates", - ) + self._git_repo: GitRepo = GitRepo(url=APP_TEMPLATES_REPO_URL) def prompt_for_location(self, package_type, runtime, base_image, dependency_manager): """ @@ -82,7 +81,7 @@ def prompt_for_location(self, package_type, runtime, base_image, dependency_mana if template_md.get("init_location") is not None: return (template_md["init_location"], template_md["appTemplate"]) if template_md.get("directory") is not None: - return (os.path.join(self._git_repo.local_path, template_md["directory"]), template_md["appTemplate"]) + return os.path.join(self._git_repo.local_path, template_md["directory"]), template_md["appTemplate"] raise InvalidInitTemplateError("Invalid template. This should not be possible, please raise an issue.") def location_from_app_template(self, package_type, runtime, base_image, dependency_manager, app_template): @@ -108,12 +107,12 @@ def init_options(self, package_type, runtime, base_image, dependency_manager): if not self._git_repo.clone_attempted: shared_dir: Path = global_cfg.config_dir try: - self._git_repo.clone(clone_dir=shared_dir, replace_existing=True) + self._git_repo.clone(clone_dir=shared_dir, clone_name=APP_TEMPLATES_REPO_NAME, replace_existing=True) except CloneRepoUnstableStateException as ex: raise AppTemplateUpdateException(str(ex)) from ex except (OSError, CloneRepoException): # If can't clone, try using an old clone from a previous run if already exist - expected_previous_clone_local_path: Path = shared_dir.joinpath(self._git_repo.name) + expected_previous_clone_local_path: Path = shared_dir.joinpath(APP_TEMPLATES_REPO_NAME) if expected_previous_clone_local_path.exists(): self._git_repo.local_path = expected_previous_clone_local_path if self._git_repo.local_path is None: diff --git a/samcli/lib/utils/git_repo.py b/samcli/lib/utils/git_repo.py index 2afa171160..33e4597726 100644 --- a/samcli/lib/utils/git_repo.py +++ b/samcli/lib/utils/git_repo.py @@ -32,50 +32,25 @@ class GitRepo: Attributes ---------- - _url: str + url: str The URL of this Git repository, example "https://github.com/aws/aws-sam-cli" - _name: str - The name of this Git repository, example "aws-sam-cli" - _local_path: Path - The path of the local clone of this Git repository - _clone_attempted: bool - whether an attempt to clone this Git repository took place or not + local_path: Path + The path of the last local clone of this Git repository. Can be used in conjunction with clone_attempted + to avoid unnecessary multiple cloning of the repository. + clone_attempted: bool + whether an attempt to clone this Git repository took place or not. Can be used in conjunction with local_path + to avoid unnecessary multiple cloning of the repository Methods ------- - clone(self, clone_dir: Path, clone_name=None, replace_existing=False) -> Path: - creates a local clone of this Git repository + clone(self, clone_dir: Path, clone_name, replace_existing=False) -> Path: + creates a local clone of this Git repository. (more details in the method documentation). """ - def __init__(self, url: str, name: str) -> None: - self._url: str = url - self._name: str = name - self._local_path: Optional[Path] = None - self._clone_attempted: bool = False - - @property - def url(self) -> str: - return self._url - - @property - def name(self) -> str: - return self._name - - @property - def local_path(self) -> Optional[Path]: - return self._local_path - - @local_path.setter - def local_path(self, value: Path) -> None: - self._local_path = value - - @property - def clone_attempted(self) -> bool: - return self._clone_attempted - - @clone_attempted.setter - def clone_attempted(self, value: bool) -> None: - self._clone_attempted = value + def __init__(self, url: str) -> None: + self.url: str = url + self.local_path: Optional[Path] = None + self.clone_attempted: bool = False @staticmethod def _ensure_clone_directory_exists(clone_dir: Path) -> None: @@ -102,16 +77,21 @@ def _git_executable() -> str: raise OSError("Cannot find git, was looking at executables: {}".format(executables)) - def clone(self, clone_dir: Path, clone_name: Optional[str] = None, replace_existing: bool = False) -> Path: + def clone(self, clone_dir: Path, clone_name: str, replace_existing: bool = False) -> Path: """ - creates a local clone of this Git repository + creates a local clone of this Git repository. + This method is different from the standard Git clone in the following: + 1. It accepts the path to clone into as a clone_dir (the parent directory to clone in) and a clone_name (The + name of the local folder) instead of accepting the full path (the join of both) in one parameter + 2. It removes the "*.git" files/directories so the clone is not a GitRepo any more + 3. It has the option to replace the local folder(destination) if already exists Parameters ---------- clone_dir: Path The directory to create the local clone inside - clone_name: Optional[str] - The dirname of the local clone, if not provided, repository's name (self._name) will be used + clone_name: str + The dirname of the local clone replace_existing: bool Whether to replace the current local clone directory if already exists or not @@ -135,27 +115,26 @@ def clone(self, clone_dir: Path, clone_name: Optional[str] = None, replace_exist # clone to temp then move to the destination(repo_local_path) with osutils.mkdir_temp(ignore_errors=True) as tempdir: try: - clone_name = clone_name if clone_name else self._name temp_path = os.path.normpath(os.path.join(tempdir, clone_name)) git_executable: str = GitRepo._git_executable() - LOG.info("\nCloning from %s", self._url) + LOG.info("\nCloning from %s", self.url) subprocess.check_output( - [git_executable, "clone", self._url, clone_name], + [git_executable, "clone", self.url, clone_name], cwd=tempdir, stderr=subprocess.STDOUT, ) - self._local_path = self._persist_local_repo(temp_path, clone_dir, clone_name, replace_existing) - return self._local_path + self.local_path = self._persist_local_repo(temp_path, clone_dir, clone_name, replace_existing) + return self.local_path except OSError as ex: - LOG.warning("WARN: Could not clone repo %s", self._url, exc_info=ex) + LOG.warning("WARN: Could not clone repo %s", self.url, exc_info=ex) raise except subprocess.CalledProcessError as clone_error: output = clone_error.output.decode("utf-8") if "not found" in output.lower(): - LOG.warning("WARN: Could not clone repo %s", self._url, exc_info=clone_error) + LOG.warning("WARN: Could not clone repo %s", self.url, exc_info=clone_error) raise CloneRepoException from clone_error finally: - self._clone_attempted = True + self.clone_attempted = True @staticmethod def _persist_local_repo(temp_path: str, dest_dir: Path, dest_name: str, replace_existing: bool) -> Path: @@ -168,6 +147,7 @@ def _persist_local_repo(temp_path: str, dest_dir: Path, dest_name: str, replace_ shutil.rmtree(dest_path, onerror=rmtree_callback) LOG.debug("Copying from %s to %s", temp_path, dest_path) + # Todo consider not removing the .git files/directories shutil.copytree(temp_path, dest_path, ignore=shutil.ignore_patterns("*.git")) return Path(dest_path) except (OSError, shutil.Error) as ex: diff --git a/tests/unit/commands/init/test_cli.py b/tests/unit/commands/init/test_cli.py index d935415700..a8ee9a82c1 100644 --- a/tests/unit/commands/init/test_cli.py +++ b/tests/unit/commands/init/test_cli.py @@ -9,7 +9,7 @@ from samcli.commands.exceptions import UserException from samcli.commands.init import cli as init_cmd from samcli.commands.init import do_cli as init_cli -from samcli.commands.init.init_templates import InitTemplates +from samcli.commands.init.init_templates import InitTemplates, APP_TEMPLATES_REPO_URL, APP_TEMPLATES_REPO_NAME from samcli.lib.init import GenerateProjectFailedError from samcli.lib.utils.git_repo import GitRepo from samcli.lib.utils.packagetype import IMAGE, ZIP @@ -19,8 +19,7 @@ class MockInitTemplates: def __init__(self, no_interactive=False): self._no_interactive = no_interactive self._git_repo: GitRepo = GitRepo( - url="https://github.com/awslabs/aws-sam-cli-app-templates.git", - name="aws-sam-cli-app-templates", + url=APP_TEMPLATES_REPO_URL, ) self._git_repo.clone_attempted = True self._git_repo.local_path = Path("repository") diff --git a/tests/unit/lib/utils/test_git_repo.py b/tests/unit/lib/utils/test_git_repo.py index 259277012f..645dc5c2de 100644 --- a/tests/unit/lib/utils/test_git_repo.py +++ b/tests/unit/lib/utils/test_git_repo.py @@ -8,12 +8,12 @@ REPO_URL = "REPO URL" REPO_NAME = "REPO NAME" CLONE_DIR = os.path.normpath("/tmp/local/clone/dir") -EXPECTED_DEFAULT_CLONE_PATH = f"{CLONE_DIR}/{REPO_NAME}" +EXPECTED_DEFAULT_CLONE_PATH = os.path.normpath(os.path.join(CLONE_DIR, REPO_NAME)) class TestGitRepo(TestCase): def setUp(self): - self.repo = GitRepo(url=REPO_URL, name=REPO_NAME) + self.repo = GitRepo(url=REPO_URL) self.local_clone_dir = MagicMock() self.local_clone_dir.joinpath.side_effect = lambda sub_dir: os.path.normpath(os.path.join(CLONE_DIR, sub_dir)) @@ -46,16 +46,6 @@ def test_git_executable_fails(self, mock_popen): with self.assertRaises(OSError): self.repo._git_executable() - @patch("samcli.lib.utils.git_repo.shutil") - @patch("samcli.lib.utils.git_repo.subprocess.check_output") - @patch("samcli.lib.utils.git_repo.subprocess.Popen") - @patch("samcli.lib.utils.git_repo.platform.system") - def test_clone_will_use_repo_name_by_default(self, platform_mock, popen_mock, check_output_mock, shutil_mock): - local_path: Path = self.repo.clone(clone_dir=self.local_clone_dir) - self.assertTrue(REPO_NAME in str(local_path)) - local_path = self.repo.clone(clone_dir=self.local_clone_dir, clone_name="REPO NOT NAME") - self.assertFalse(REPO_NAME in str(local_path)) - @patch("samcli.lib.utils.git_repo.Path.exists") @patch("samcli.lib.utils.git_repo.shutil") @patch("samcli.lib.utils.git_repo.subprocess.check_output") @@ -63,11 +53,11 @@ def test_clone_will_use_repo_name_by_default(self, platform_mock, popen_mock, ch @patch("samcli.lib.utils.git_repo.platform.system") def test_clone_happy_case(self, platform_mock, popen_mock, check_output_mock, shutil_mock, path_exist_mock): path_exist_mock.return_value = False - self.repo.clone(clone_dir=self.local_clone_dir) + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME) self.local_clone_dir.mkdir.assert_called_once_with(mode=0o700, parents=True, exist_ok=True) popen_mock.assert_called_once_with(["git"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) check_output_mock.assert_has_calls( - [call(["git", "clone", self.repo.url, self.repo.name], cwd=ANY, stderr=subprocess.STDOUT)] + [call(["git", "clone", self.repo.url, REPO_NAME], cwd=ANY, stderr=subprocess.STDOUT)] ) shutil_mock.rmtree.assert_not_called() shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY) @@ -82,7 +72,7 @@ def test_clone_create_new_local_repo( self, platform_mock, popen_mock, check_output_mock, shutil_mock, path_exist_mock ): path_exist_mock.return_value = False - self.repo.clone(clone_dir=self.local_clone_dir) + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME) shutil_mock.rmtree.assert_not_called() shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY) shutil_mock.ignore_patterns.assert_called_with("*.git") @@ -96,7 +86,7 @@ def test_clone_replace_current_local_repo_if_replace_existing_flag_is_set( self, platform_mock, popen_mock, check_output_mock, shutil_mock, path_exist_mock ): path_exist_mock.return_value = True - self.repo.clone(clone_dir=self.local_clone_dir, replace_existing=True) + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME, replace_existing=True) self.local_clone_dir.mkdir.assert_called_once_with(mode=0o700, parents=True, exist_ok=True) shutil_mock.rmtree.assert_called_with(EXPECTED_DEFAULT_CLONE_PATH, onerror=rmtree_callback) shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY) @@ -111,7 +101,7 @@ def test_clone_fail_if_current_local_repo_exists_and_replace_existing_flag_is_no ): path_exist_mock.return_value = True with self.assertRaises(CloneRepoException): - self.repo.clone(clone_dir=self.local_clone_dir) # replace_existing=False by default + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME) # replace_existing=False by default @patch("samcli.lib.utils.git_repo.shutil") @patch("samcli.lib.utils.git_repo.subprocess.check_output") @@ -119,7 +109,7 @@ def test_clone_fail_if_current_local_repo_exists_and_replace_existing_flag_is_no @patch("samcli.lib.utils.git_repo.platform.system") def test_clone_attempt_is_set_to_true_after_clone(self, platform_mock, popen_mock, check_output_mock, shutil_mock): self.assertFalse(self.repo.clone_attempted) - self.repo.clone(clone_dir=self.local_clone_dir) + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME) self.assertTrue(self.repo.clone_attempted) @patch("samcli.lib.utils.git_repo.shutil") @@ -133,7 +123,7 @@ def test_clone_attempt_is_set_to_true_even_if_clone_failed( self.assertFalse(self.repo.clone_attempted) try: with self.assertRaises(CloneRepoException): - self.repo.clone(clone_dir=self.local_clone_dir) + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME) except: pass self.assertTrue(self.repo.clone_attempted) @@ -148,7 +138,7 @@ def test_clone_failed_to_create_the_clone_directory( self.local_clone_dir.mkdir.side_effect = OSError try: with self.assertRaises(OSError): - self.repo.clone(clone_dir=self.local_clone_dir) + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME) except: pass self.local_clone_dir.mkdir.assert_called_once_with(mode=0o700, parents=True, exist_ok=True) @@ -163,7 +153,7 @@ def test_clone_failed_to_create_the_clone_directory( def test_clone_when_the_subprocess_fail(self, platform_mock, popen_mock, check_output_mock, shutil_mock): check_output_mock.side_effect = subprocess.CalledProcessError("fail", "fail", "any reason".encode("utf-8")) with self.assertRaises(CloneRepoException): - self.repo.clone(clone_dir=self.local_clone_dir) + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME) @patch("samcli.lib.utils.git_repo.LOG") @patch("samcli.lib.utils.git_repo.subprocess.check_output") @@ -173,7 +163,7 @@ def test_clone_when_the_git_repo_not_found(self, platform_mock, popen_mock, chec check_output_mock.side_effect = subprocess.CalledProcessError("fail", "fail", "not found".encode("utf-8")) try: with self.assertRaises(CloneRepoException): - self.repo.clone(clone_dir=self.local_clone_dir) + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME) except Exception: pass log_mock.warning.assert_called() @@ -190,7 +180,7 @@ def test_clone_when_failed_to_move_cloned_repo_from_temp_to_final_destination( shutil_mock.copytree.side_effect = OSError try: with self.assertRaises(CloneRepoUnstableStateException): - self.repo.clone(clone_dir=self.local_clone_dir, replace_existing=True) + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME, replace_existing=True) except Exception: pass shutil_mock.rmtree.assert_called_once_with(EXPECTED_DEFAULT_CLONE_PATH, onerror=rmtree_callback)