From cbd9cbd2f594ad4b842a6fbb11199e8081fe18c7 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Fri, 3 Mar 2023 19:41:37 -0800 Subject: [PATCH 1/9] feat: only clone init templates when required --- samcli/commands/init/init_templates.py | 27 +++++++++++++++++----- samcli/lib/utils/git_repo.py | 5 +++- tests/unit/commands/init/test_templates.py | 5 +++- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index 78e3d386f8..d9e7e48c52 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -8,6 +8,7 @@ import re from enum import Enum from pathlib import Path +from subprocess import STDOUT, CalledProcessError, check_output from typing import Dict, Optional import requests @@ -87,13 +88,16 @@ def clone_templates_repo(self): os_name = system().lower() cloned_folder_name = APP_TEMPLATES_REPO_NAME_WINDOWS if os_name == "windows" else APP_TEMPLATES_REPO_NAME + to_upsert = self._check_upsert_templates(shared_dir, cloned_folder_name) + try: - self._git_repo.clone( - clone_dir=shared_dir, - clone_name=cloned_folder_name, - replace_existing=True, - commit=APP_TEMPLATES_REPO_COMMIT, - ) + if to_upsert: + self._git_repo.clone( + clone_dir=shared_dir, + clone_name=cloned_folder_name, + replace_existing=True, + commit=APP_TEMPLATES_REPO_COMMIT, + ) except CloneRepoUnstableStateException as ex: raise AppTemplateUpdateException(str(ex)) from ex except (OSError, CloneRepoException): @@ -102,6 +106,17 @@ def clone_templates_repo(self): if expected_previous_clone_local_path.exists(): self._git_repo.local_path = expected_previous_clone_local_path + def _check_upsert_templates(self, shared_dir, cloned_folder_name): + cache_dir = shared_dir / cloned_folder_name + git_executable = self._git_repo.get_git_executable() + command = [git_executable, "rev-parse", "--verify", "HEAD"] + try: + existing_hash = check_output(command, cwd=cache_dir, stderr=STDOUT).decode("utf-8").strip() + except CalledProcessError: + LOG.error("Unable to check existing cache hash") + return True + return not existing_hash == APP_TEMPLATES_REPO_COMMIT + def _init_options_from_manifest(self, package_type, runtime, base_image, dependency_manager): manifest_path = self.get_manifest_path() with open(str(manifest_path)) as fp: diff --git a/samcli/lib/utils/git_repo.py b/samcli/lib/utils/git_repo.py index c2f30914c3..3ed73d4ade 100644 --- a/samcli/lib/utils/git_repo.py +++ b/samcli/lib/utils/git_repo.py @@ -70,6 +70,9 @@ def _ensure_clone_directory_exists(clone_dir: Path) -> None: LOG.warning("WARN: Unable to create clone directory.", exc_info=ex) raise + def get_git_executable(self): + return self._git_executable() + @staticmethod def _git_executable() -> str: if platform.system().lower() == "windows": @@ -172,7 +175,7 @@ def _persist_local_repo(temp_path: str, dest_dir: Path, dest_name: str, replace_ 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")) + shutil.copytree(temp_path, dest_path) return Path(dest_path) except (OSError, shutil.Error) as ex: # UNSTABLE STATE diff --git a/tests/unit/commands/init/test_templates.py b/tests/unit/commands/init/test_templates.py index e39b451eac..6cef2b67d5 100644 --- a/tests/unit/commands/init/test_templates.py +++ b/tests/unit/commands/init/test_templates.py @@ -2,7 +2,7 @@ from pathlib import Path from re import search from unittest import TestCase -from unittest.mock import mock_open, patch, PropertyMock +from unittest.mock import mock_open, patch, PropertyMock, Mock from samcli.commands.init.init_templates import InitTemplates from samcli.lib.utils.packagetype import IMAGE, ZIP @@ -15,6 +15,7 @@ class TestTemplates(TestCase): @patch("shutil.copytree") def test_location_from_app_template_zip(self, subprocess_mock, git_exec_mock, cd_mock, copy_mock): it = InitTemplates() + it._check_upsert_templates = Mock() manifest = { "ruby2.7": [ @@ -36,12 +37,14 @@ def test_location_from_app_template_zip(self, subprocess_mock, git_exec_mock, cd location = it.location_from_app_template(ZIP, "ruby2.7", None, "bundler", "hello-world") self.assertTrue(search("mock-ruby-template", location)) + @patch("samcli.lib.utils.init_templates.") @patch("samcli.lib.utils.git_repo.check_output") @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, cd_mock, copy_mock): it = InitTemplates() + it._check_upsert_templates = Mock() manifest = { "ruby2.7-image": [ From 4eecd6b997afdb8fbdd27bae9bff1b62d38ee07e Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Mon, 6 Mar 2023 15:13:21 -0800 Subject: [PATCH 2/9] Add unit and integration tests --- tests/integration/init/test_init_command.py | 57 +++++++++++++++++++++ tests/unit/commands/init/test_templates.py | 18 ++++++- tests/unit/lib/utils/test_git_repo.py | 18 +++---- 3 files changed, 80 insertions(+), 13 deletions(-) diff --git a/tests/integration/init/test_init_command.py b/tests/integration/init/test_init_command.py index e4cfa4d648..70b6ee44d3 100644 --- a/tests/integration/init/test_init_command.py +++ b/tests/integration/init/test_init_command.py @@ -4,6 +4,7 @@ from click.testing import CliRunner +from samcli.cli.global_config import GlobalConfig from samcli.commands.init import cli as init_cmd from unittest import TestCase @@ -838,6 +839,62 @@ def test_interactive_init_default_runtime(self): self.assertTrue(Path(expected_output_folder, "hello_world", "app.py").is_file()) +class TestSubsequentInitCaching(TestCase): + def test_subsequent_init_skips_cloning(self): + with tempfile.TemporaryDirectory() as temp: + project_directory = Path(temp, "sam-app") + cache_dir = GlobalConfig().config_dir / "aws-sam-cli-app-templates" + + # Run the first time, get cache last modification time + self._run_init(temp) + first_modification_time = self._last_modification_time(cache_dir) + self.assertTrue(project_directory.is_dir()) + shutil.rmtree(project_directory) + + # Run init again, get the second modification time + self._run_init(temp) + second_modification_time = self._last_modification_time(cache_dir) + self.assertTrue(project_directory.is_dir()) + + self.assertEqual(first_modification_time, second_modification_time) + + def _run_init(self, cwd): + process = Popen( + [ + get_sam_command(), + "init", + "--runtime", + "nodejs14.x", + "--dependency-manager", + "npm", + "--architecture", + "arm64", + "--app-template", + "hello-world", + "--name", + "sam-app", + "--no-interactive", + "-o", + cwd, + ], + stdout=PIPE, + stderr=PIPE, + ) + try: + stdout_data, stderr_data = process.communicate(timeout=TIMEOUT) + stderr = stderr_data.decode("utf-8") + except TimeoutExpired: + process.kill() + raise + + self.assertEqual(process.returncode, 0) + self.assertNotIn(COMMIT_ERROR, stderr) + + @staticmethod + def _last_modification_time(file): + return os.path.getmtime(file) + + class TestInitProducesSamconfigFile(TestCase): def test_zip_template_config(self): with tempfile.TemporaryDirectory() as temp: diff --git a/tests/unit/commands/init/test_templates.py b/tests/unit/commands/init/test_templates.py index 6cef2b67d5..1dd8d33218 100644 --- a/tests/unit/commands/init/test_templates.py +++ b/tests/unit/commands/init/test_templates.py @@ -1,4 +1,7 @@ +from subprocess import STDOUT + import json +from parameterized import parameterized from pathlib import Path from re import search from unittest import TestCase @@ -37,7 +40,6 @@ def test_location_from_app_template_zip(self, subprocess_mock, git_exec_mock, cd location = it.location_from_app_template(ZIP, "ruby2.7", None, "bundler", "hello-world") self.assertTrue(search("mock-ruby-template", location)) - @patch("samcli.lib.utils.init_templates.") @patch("samcli.lib.utils.git_repo.check_output") @patch("samcli.lib.utils.git_repo.GitRepo._git_executable") @patch("samcli.lib.utils.git_repo.GitRepo._ensure_clone_directory_exists") @@ -67,3 +69,17 @@ def test_location_from_app_template_image(self, subprocess_mock, git_exec_mock, IMAGE, None, "ruby2.7-image", "bundler", "hello-world-lambda-image" ) self.assertTrue(search("mock-ruby-image-template", location)) + + @parameterized.expand([("hash_a", "hash_a", False), ("hash_a", "hash_b", True)]) + @patch("samcli.lib.utils.git_repo.GitRepo.get_git_executable") + @patch("samcli.commands.init.init_templates.check_output") + def test_check_upsert_templates(self, first_hash, second_hash, expected_value, check_output_mock, git_exec_mock): + it = InitTemplates() + git_exec_mock.return_value = "git" + check_output_mock.return_value = second_hash.encode("utf-8") + with patch("samcli.commands.init.init_templates.APP_TEMPLATES_REPO_COMMIT", first_hash): + return_value = it._check_upsert_templates(Path("shared_dir"), Path("cloned_folder_dir")) + check_output_mock.assert_called_once_with( + ["git", "rev-parse", "--verify", "HEAD"], cwd=Path("shared_dir/cloned_folder_dir"), stderr=STDOUT + ) + self.assertEqual(return_value, expected_value) diff --git a/tests/unit/lib/utils/test_git_repo.py b/tests/unit/lib/utils/test_git_repo.py index d56213f018..f505159be1 100644 --- a/tests/unit/lib/utils/test_git_repo.py +++ b/tests/unit/lib/utils/test_git_repo.py @@ -62,8 +62,7 @@ def test_clone_happy_case(self, platform_mock, popen_mock, check_output_mock, sh [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) - shutil_mock.ignore_patterns.assert_called_with("*.git") + shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH) @patch("samcli.lib.utils.git_repo.Path.exists") @patch("samcli.lib.utils.git_repo.shutil") @@ -76,8 +75,7 @@ def test_clone_create_new_local_repo( path_exist_mock.return_value = False 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") + shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH) @patch("samcli.lib.utils.git_repo.Path.exists") @patch("samcli.lib.utils.git_repo.shutil") @@ -91,8 +89,7 @@ def test_clone_replace_current_local_repo_if_replace_existing_flag_is_set( 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) - shutil_mock.ignore_patterns.assert_called_with("*.git") + shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH) @patch("samcli.lib.utils.git_repo.Path.exists") @patch("samcli.lib.utils.git_repo.check_output") @@ -186,8 +183,7 @@ def test_clone_when_failed_to_move_cloned_repo_from_temp_to_final_destination( 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") + shutil_mock.copytree.assert_called_once_with(ANY, EXPECTED_DEFAULT_CLONE_PATH) @patch("samcli.lib.utils.git_repo.LOG") @patch("samcli.lib.utils.git_repo.check_output") @@ -221,8 +217,7 @@ def test_clone_with_commit(self, platform_mock, popen_mock, check_output_mock, s [call(["git", "checkout", COMMIT], 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") + shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH) @patch("samcli.lib.utils.git_repo.Path.exists") @patch("samcli.lib.utils.git_repo.shutil") @@ -247,8 +242,7 @@ def test_clone_with_longpaths_configured_in_windows( ] ) 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") + shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH) @patch("samcli.lib.utils.git_repo.Path") @patch("samcli.lib.utils.git_repo.platform.system") From 2823ecab550c2fee540344997c72495eb504614e Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Mon, 6 Mar 2023 15:22:21 -0800 Subject: [PATCH 3/9] Add type hints and docstrings --- samcli/commands/init/init_templates.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index d9e7e48c52..a56ee15103 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -106,7 +106,24 @@ def clone_templates_repo(self): if expected_previous_clone_local_path.exists(): self._git_repo.local_path = expected_previous_clone_local_path - def _check_upsert_templates(self, shared_dir, cloned_folder_name): + def _check_upsert_templates(self, shared_dir: Path, cloned_folder_name: str) -> bool: + """ + Check if the app templates repository should be cloned, or if cloning should be skipped. + + Parameters + ---------- + shared_dir: Path + Folder containing the aws-sam-cli shared data + + cloned_folder_name: str + Name of the directory into which the app templates will be copied + + Returns + ------- + bool + True if the cache should be updated, False otherwise + + """ cache_dir = shared_dir / cloned_folder_name git_executable = self._git_repo.get_git_executable() command = [git_executable, "rev-parse", "--verify", "HEAD"] From 7ac7d29e4e610a30751f51a78de6298f4efb998e Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Mon, 6 Mar 2023 15:59:37 -0800 Subject: [PATCH 4/9] Catch file not found exception --- samcli/commands/init/init_templates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index a56ee15103..88e96d57da 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -129,7 +129,7 @@ def _check_upsert_templates(self, shared_dir: Path, cloned_folder_name: str) -> command = [git_executable, "rev-parse", "--verify", "HEAD"] try: existing_hash = check_output(command, cwd=cache_dir, stderr=STDOUT).decode("utf-8").strip() - except CalledProcessError: + except (CalledProcessError, FileNotFoundError): LOG.error("Unable to check existing cache hash") return True return not existing_hash == APP_TEMPLATES_REPO_COMMIT From 42bde5d8cccfeed1c8e2c515caf270dedfc3a048 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Mon, 6 Mar 2023 16:10:28 -0800 Subject: [PATCH 5/9] Catch file not found exception --- samcli/commands/init/init_templates.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index 88e96d57da..b83fb22974 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -129,8 +129,11 @@ def _check_upsert_templates(self, shared_dir: Path, cloned_folder_name: str) -> command = [git_executable, "rev-parse", "--verify", "HEAD"] try: existing_hash = check_output(command, cwd=cache_dir, stderr=STDOUT).decode("utf-8").strip() - except (CalledProcessError, FileNotFoundError): - LOG.error("Unable to check existing cache hash") + except CalledProcessError: + LOG.debug("Unable to check existing cache hash") + return True + except FileNotFoundError: + LOG.debug("Cache directory does not yet exist, creating one.") return True return not existing_hash == APP_TEMPLATES_REPO_COMMIT From 03a6e6c8701d0417357322949fcaa12a1aa78119 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Mon, 6 Mar 2023 16:42:20 -0800 Subject: [PATCH 6/9] Fix Windows tests --- samcli/commands/init/init_templates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index b83fb22974..5c0847b206 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -132,7 +132,7 @@ def _check_upsert_templates(self, shared_dir: Path, cloned_folder_name: str) -> except CalledProcessError: LOG.debug("Unable to check existing cache hash") return True - except FileNotFoundError: + except (FileNotFoundError, NotADirectoryError): LOG.debug("Cache directory does not yet exist, creating one.") return True return not existing_hash == APP_TEMPLATES_REPO_COMMIT From a1e0e053abaed427c7e7cfd38529f91409879e1e Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Mon, 6 Mar 2023 17:09:47 -0800 Subject: [PATCH 7/9] Address comments --- samcli/commands/init/init_templates.py | 24 +++++++++++----------- samcli/lib/utils/git_repo.py | 9 +++----- tests/unit/commands/init/test_templates.py | 6 +++--- tests/unit/lib/utils/test_git_repo.py | 6 +++--- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index 5c0847b206..534d9b9804 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -88,16 +88,16 @@ def clone_templates_repo(self): os_name = system().lower() cloned_folder_name = APP_TEMPLATES_REPO_NAME_WINDOWS if os_name == "windows" else APP_TEMPLATES_REPO_NAME - to_upsert = self._check_upsert_templates(shared_dir, cloned_folder_name) + if not self._check_upsert_templates(shared_dir, cloned_folder_name): + return try: - if to_upsert: - self._git_repo.clone( - clone_dir=shared_dir, - clone_name=cloned_folder_name, - replace_existing=True, - commit=APP_TEMPLATES_REPO_COMMIT, - ) + self._git_repo.clone( + clone_dir=shared_dir, + clone_name=cloned_folder_name, + replace_existing=True, + commit=APP_TEMPLATES_REPO_COMMIT, + ) except CloneRepoUnstableStateException as ex: raise AppTemplateUpdateException(str(ex)) from ex except (OSError, CloneRepoException): @@ -124,13 +124,13 @@ def _check_upsert_templates(self, shared_dir: Path, cloned_folder_name: str) -> True if the cache should be updated, False otherwise """ - cache_dir = shared_dir / cloned_folder_name - git_executable = self._git_repo.get_git_executable() + cache_dir = Path(shared_dir, cloned_folder_name) + git_executable = self._git_repo.git_executable() command = [git_executable, "rev-parse", "--verify", "HEAD"] try: existing_hash = check_output(command, cwd=cache_dir, stderr=STDOUT).decode("utf-8").strip() - except CalledProcessError: - LOG.debug("Unable to check existing cache hash") + except CalledProcessError as ex: + LOG.debug(f"Unable to check existing cache hash\n{ex.output.decode('utf-8')}") return True except (FileNotFoundError, NotADirectoryError): LOG.debug("Cache directory does not yet exist, creating one.") diff --git a/samcli/lib/utils/git_repo.py b/samcli/lib/utils/git_repo.py index 3ed73d4ade..0aed88837a 100644 --- a/samcli/lib/utils/git_repo.py +++ b/samcli/lib/utils/git_repo.py @@ -70,11 +70,8 @@ def _ensure_clone_directory_exists(clone_dir: Path) -> None: LOG.warning("WARN: Unable to create clone directory.", exc_info=ex) raise - def get_git_executable(self): - return self._git_executable() - @staticmethod - def _git_executable() -> str: + def git_executable() -> str: if platform.system().lower() == "windows": executables = ["git", "git.cmd", "git.exe", "git.bat"] else: @@ -130,7 +127,7 @@ def clone(self, clone_dir: Path, clone_name: str, replace_existing: bool = False with osutils.mkdir_temp(ignore_errors=True) as tempdir: try: temp_path = os.path.normpath(os.path.join(tempdir, clone_name)) - git_executable: str = GitRepo._git_executable() + git_executable: str = GitRepo.git_executable() LOG.info("\nCloning from %s (process may take a moment)", self.url) command = [git_executable, "clone", self.url, clone_name] if platform.system().lower() == "windows": @@ -201,7 +198,7 @@ def _persist_local_repo(temp_path: str, dest_dir: Path, dest_name: str, replace_ def _checkout_commit(repo_dir: str, commit: str): try: # if the checkout commit failed, it will use the latest commit instead - git_executable = GitRepo._git_executable() + git_executable = GitRepo.git_executable() check_output( [git_executable, "checkout", commit], cwd=repo_dir, diff --git a/tests/unit/commands/init/test_templates.py b/tests/unit/commands/init/test_templates.py index 1dd8d33218..2a18f358f9 100644 --- a/tests/unit/commands/init/test_templates.py +++ b/tests/unit/commands/init/test_templates.py @@ -13,7 +13,7 @@ class TestTemplates(TestCase): @patch("samcli.lib.utils.git_repo.check_output") - @patch("samcli.lib.utils.git_repo.GitRepo._git_executable") + @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, cd_mock, copy_mock): @@ -41,7 +41,7 @@ def test_location_from_app_template_zip(self, subprocess_mock, git_exec_mock, cd self.assertTrue(search("mock-ruby-template", location)) @patch("samcli.lib.utils.git_repo.check_output") - @patch("samcli.lib.utils.git_repo.GitRepo._git_executable") + @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, cd_mock, copy_mock): @@ -71,7 +71,7 @@ def test_location_from_app_template_image(self, subprocess_mock, git_exec_mock, self.assertTrue(search("mock-ruby-image-template", location)) @parameterized.expand([("hash_a", "hash_a", False), ("hash_a", "hash_b", True)]) - @patch("samcli.lib.utils.git_repo.GitRepo.get_git_executable") + @patch("samcli.lib.utils.git_repo.GitRepo.git_executable") @patch("samcli.commands.init.init_templates.check_output") def test_check_upsert_templates(self, first_hash, second_hash, expected_value, check_output_mock, git_exec_mock): it = InitTemplates() diff --git a/tests/unit/lib/utils/test_git_repo.py b/tests/unit/lib/utils/test_git_repo.py index f505159be1..67760e435a 100644 --- a/tests/unit/lib/utils/test_git_repo.py +++ b/tests/unit/lib/utils/test_git_repo.py @@ -31,21 +31,21 @@ def test_ensure_clone_directory_exists_fail(self): @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() + 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() + 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() + self.repo.git_executable() @patch("samcli.lib.utils.git_repo.Path.exists") @patch("samcli.lib.utils.git_repo.shutil") From beceab1a3d4694cabf92e081f6f54cbe2824d703 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Mon, 6 Mar 2023 17:59:54 -0800 Subject: [PATCH 8/9] Fix image templates not being found --- samcli/commands/init/init_templates.py | 1 + 1 file changed, 1 insertion(+) diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index 534d9b9804..8e56ab47ce 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -135,6 +135,7 @@ def _check_upsert_templates(self, shared_dir: Path, cloned_folder_name: str) -> except (FileNotFoundError, NotADirectoryError): LOG.debug("Cache directory does not yet exist, creating one.") return True + self._git_repo.local_path = cache_dir return not existing_hash == APP_TEMPLATES_REPO_COMMIT def _init_options_from_manifest(self, package_type, runtime, base_image, dependency_manager): From 04814ee24ce37a5ba8aa44bf4d9710bdcce5d11e Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Tue, 7 Mar 2023 09:20:07 -0800 Subject: [PATCH 9/9] Use correct folder name per OS --- tests/integration/init/test_init_command.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/integration/init/test_init_command.py b/tests/integration/init/test_init_command.py index 70b6ee44d3..e952de2f73 100644 --- a/tests/integration/init/test_init_command.py +++ b/tests/integration/init/test_init_command.py @@ -14,6 +14,7 @@ import shutil import tempfile +from samcli.commands.init.init_templates import APP_TEMPLATES_REPO_NAME_WINDOWS, APP_TEMPLATES_REPO_NAME from samcli.lib.config.samconfig import DEFAULT_CONFIG_FILE_NAME from samcli.lib.utils.packagetype import IMAGE, ZIP @@ -841,9 +842,14 @@ def test_interactive_init_default_runtime(self): class TestSubsequentInitCaching(TestCase): def test_subsequent_init_skips_cloning(self): + from platform import system + + os_name = system().lower() + cloned_folder_name = APP_TEMPLATES_REPO_NAME_WINDOWS if os_name == "windows" else APP_TEMPLATES_REPO_NAME + with tempfile.TemporaryDirectory() as temp: project_directory = Path(temp, "sam-app") - cache_dir = GlobalConfig().config_dir / "aws-sam-cli-app-templates" + cache_dir = GlobalConfig().config_dir / cloned_folder_name # Run the first time, get cache last modification time self._run_init(temp)