Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Speed up unit test by caching the git clone #3060

Merged
merged 4 commits into from
Jul 22, 2021
Merged

ci: Speed up unit test by caching the git clone #3060

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a7f3488
ci: Speed up unit test by caching the git clone
aahung Jul 15, 2021
2ee9f23
Add comments
aahung Jul 15, 2021
c0ef90d
Merge branch 'develop' into speed-up-init-test
aahung Jul 22, 2021
d83b599
Merge branch 'develop' into speed-up-init-test
aahung Jul 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion samcli/lib/utils/git_repo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import shutil
import subprocess
from pathlib import Path
from subprocess import check_output
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A "why" comment here will be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

from typing import Optional

from samcli.lib.utils import osutils
Expand Down Expand Up @@ -118,7 +119,7 @@ def clone(self, clone_dir: Path, clone_name: str, replace_existing: bool = False
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(
check_output(
[git_executable, "clone", self.url, clone_name],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we shallow-clone instead? So that everyone benefits from sam init and there's no risk to the patched test code diverging from prod code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, and it alone produced time ranging from 37s to 67s.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted, wrong testing data

cwd=tempdir,
stderr=subprocess.STDOUT,
Expand Down
37 changes: 37 additions & 0 deletions tests/unit/commands/init/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import os
import shutil
import subprocess
import tempfile
from pathlib import Path
from typing import Dict, Any
from unittest import TestCase
from unittest.mock import patch, ANY

Expand All @@ -12,6 +16,7 @@
from samcli.commands.init import do_cli as init_cli
from samcli.commands.init.init_templates import InitTemplates, APP_TEMPLATES_REPO_URL
from samcli.lib.init import GenerateProjectFailedError
from samcli.lib.utils import osutils
from samcli.lib.utils.git_repo import GitRepo
from samcli.lib.utils.packagetype import IMAGE, ZIP

Expand Down Expand Up @@ -43,6 +48,38 @@ 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"}

# setup cache for clone, so that if `git clone` is called multiple times on the same URL,
# only one clone will happen.
clone_cache: Dict[str, Path]
patcher: Any

@classmethod
def setUpClass(cls) -> None:
# Make (url -> directory) cache to avoid cloning the same thing twice
cls.clone_cache = {}
cls.patcher = patch("samcli.lib.utils.git_repo.check_output", side_effect=cls.check_output_mock)
cls.patcher.start()

@classmethod
def tearDownClass(cls) -> None:
cls.patcher.stop()
for _, directory in cls.clone_cache.items():
shutil.rmtree(directory.parent)

@classmethod
def check_output_mock(cls, commands, cwd, stderr):
git_executable, _, url, clone_name = commands
if url not in cls.clone_cache:
tempdir = tempfile.NamedTemporaryFile(delete=False).name
subprocess.check_output(
[git_executable, "clone", url, clone_name],
cwd=tempdir,
stderr=stderr,
)
cls.clone_cache[url] = Path(tempdir, clone_name)

osutils.copytree(str(cls.clone_cache[url]), str(Path(cwd, clone_name)))

@patch("samcli.lib.utils.git_repo.GitRepo.clone")
@patch("samcli.commands.init.init_generator.generate_project")
def test_init_cli(self, generate_project_patch, git_repo_clone_mock):
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/commands/init/test_templates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@


class TestTemplates(TestCase):
@patch("subprocess.check_output")
@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")
Expand All @@ -37,7 +37,7 @@ def test_location_from_app_template_zip(self, subprocess_mock, git_exec_mock, cd
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.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")
Expand Down Expand Up @@ -71,7 +71,7 @@ def test_location_from_app_template_image(self, subprocess_mock, git_exec_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.lib.utils.git_repo.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 = Path("/tmp/test-sam")
Expand All @@ -85,7 +85,7 @@ def test_fallback_options(self, git_exec_mock, prompt_mock, cd_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.lib.utils.git_repo.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 = Path("/tmp/test-sam")
Expand Down
20 changes: 10 additions & 10 deletions tests/unit/lib/utils/test_git_repo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test_git_executable_fails(self, mock_popen):

@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.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):
Expand All @@ -65,7 +65,7 @@ def test_clone_happy_case(self, platform_mock, popen_mock, check_output_mock, sh

@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.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(
Expand All @@ -79,7 +79,7 @@ def test_clone_create_new_local_repo(

@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.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(
Expand All @@ -93,7 +93,7 @@ def test_clone_replace_current_local_repo_if_replace_existing_flag_is_set(
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.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(
Expand All @@ -104,7 +104,7 @@ def test_clone_fail_if_current_local_repo_exists_and_replace_existing_flag_is_no
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")
@patch("samcli.lib.utils.git_repo.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):
Expand All @@ -113,7 +113,7 @@ def test_clone_attempt_is_set_to_true_after_clone(self, platform_mock, popen_moc
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.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(
Expand All @@ -129,7 +129,7 @@ def test_clone_attempt_is_set_to_true_even_if_clone_failed(
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.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(
Expand All @@ -147,7 +147,7 @@ def test_clone_failed_to_create_the_clone_directory(
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.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):
Expand All @@ -156,7 +156,7 @@ def test_clone_when_the_subprocess_fail(self, platform_mock, popen_mock, check_o
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")
@patch("samcli.lib.utils.git_repo.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):
Expand All @@ -170,7 +170,7 @@ def test_clone_when_the_git_repo_not_found(self, platform_mock, popen_mock, chec

@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.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(
Expand Down