pipeline init UX: Ask to confirm when file exists

samcli/commands/exceptions.py

@@ -2,8 +2,6 @@
 Class containing error conditions that are exposed to the user.
 """
 
-import os
-
 import click
 
 
@@ -80,15 +78,3 @@ class AppPipelineTemplateMetadataException(UserException):
     """
     Exception class when SAM is not able to parse the "metadata.json" file located in the SAM pipeline templates
     """
-
-
-class PipelineFileAlreadyExistsError(UserException):
-    """
-    Exception class when the files to-be-generated by the pipeline template already exists on the SAM project. Instead
-    of overriding, the user need to manually remove the old files.
-    """
-
-    def __init__(self, file_path: os.PathLike) -> None:
-        super().__init__(
-            f'Pipeline file "{file_path}" already exists in project root directory, please remove it first.'
-        )

samcli/commands/pipeline/init/interactive_init_flow.py

@@ -14,9 +14,8 @@
 
 from samcli.cli.main import global_cfg
 from samcli.commands.exceptions import (
-    PipelineTemplateCloneException,
-    PipelineFileAlreadyExistsError,
     AppPipelineTemplateMetadataException,
+    PipelineTemplateCloneException,
 )
 from samcli.lib.config.samconfig import SamConfig
 from samcli.lib.cookiecutter.interactive_flow import InteractiveFlow
@@ -220,7 +219,7 @@ def _generate_from_pipeline_template(self, pipeline_template_dir: Path) -> List[
             LOG.debug("Generating pipeline files into %s", generate_dir)
             context["outputDir"] = "."  # prevent cookiecutter from generating a sub-folder
             pipeline_template.generate_project(context, generate_dir)
-            return _copy_dir_contents_to_cwd_fail_on_exist(generate_dir)
+            return _copy_dir_contents_to_cwd(generate_dir)
 
 
 def _load_pipeline_bootstrap_resources() -> Tuple[List[str], Dict[str, str]]:
@@ -254,19 +253,35 @@ def _load_pipeline_bootstrap_resources() -> Tuple[List[str], Dict[str, str]]:
     return stage_names, context
 
 
-def _copy_dir_contents_to_cwd_fail_on_exist(source_dir: str) -> List[str]:
-    copied_file_paths: List[str] = []
+def _copy_dir_contents_to_cwd(source_dir: str) -> List[str]:
+    """
+    Copy the contents of source_dir into the current cwd.
+    If existing files are encountered, ask for confirmation.
+    If not confirmed, all files will be written to
+    .aws-sam/pipeline/generated-files/
+    """
+    file_paths: List[str] = []
+    existing_file_paths: List[str] = []
     for root, _, files in os.walk(source_dir):
         for filename in files:
             file_path = Path(root, filename)
             target_file_path = Path(".").joinpath(file_path.relative_to(source_dir))
             LOG.debug("Verify %s does not exist", target_file_path)
             if target_file_path.exists():
-                raise PipelineFileAlreadyExistsError(target_file_path)
-            copied_file_paths.append(str(target_file_path))
+                existing_file_paths.append(str(target_file_path))
+            file_paths.append(str(target_file_path))
+    if existing_file_paths:
+        click.echo("\nThe following files already exist:")
+        for existing_file_path in existing_file_paths:
+            click.echo(f"\t- {existing_file_path}")
+        if not click.confirm("Do you want to override them?"):
+            target_dir = str(Path(PIPELINE_CONFIG_DIR, "generated-files"))
+            osutils.copytree(source_dir, target_dir)
+            click.echo(f"All files are saved to {target_dir}.")
+            return [str(Path(target_dir, path)) for path in file_paths]
     LOG.debug("Copy contents of %s to cwd", source_dir)
     osutils.copytree(source_dir, ".")
-    return copied_file_paths
+    return file_paths
 
 
 def _clone_app_pipeline_templates() -> Path:

tests/integration/pipeline/test_init_command.py

@@ -1,3 +1,4 @@
+import os.path
 import shutil
 from pathlib import Path
 from textwrap import dedent
@@ -64,20 +65,44 @@ def test_quick_start(self):
         with open(expected_file_path, "r") as expected, open(generated_jenkinsfile_path, "r") as output:
             self.assertEqual(expected.read(), output.read())
 
-    def test_failed_when_generated_file_already_exist(self):
+    def test_failed_when_generated_file_already_exist_override(self):
         generated_jenkinsfile_path = Path("Jenkinsfile")
         generated_jenkinsfile_path.touch()  # the file now pre-exists
         self.generated_files.append(generated_jenkinsfile_path)
 
         init_command_list = self.get_init_command_list()
-        init_process_execute = run_command_with_inputs(init_command_list, QUICK_START_JENKINS_INPUTS_WITHOUT_AUTO_FILL)
+        init_process_execute = run_command_with_inputs(
+            init_command_list, [*QUICK_START_JENKINS_INPUTS_WITHOUT_AUTO_FILL, "y"]
+        )
+
+        self.assertEqual(init_process_execute.process.returncode, 0)
+        self.assertTrue(Path("Jenkinsfile").exists())
+
+        expected_file_path = Path(__file__).parent.parent.joinpath(Path("testdata", "pipeline", "expected_jenkinsfile"))
+        with open(expected_file_path, "r") as expected, open(generated_jenkinsfile_path, "r") as output:
+            self.assertEqual(expected.read(), output.read())
+
+    def test_failed_when_generated_file_already_exist_not_override(self):
+        generated_jenkinsfile_path = Path("Jenkinsfile")
+        generated_jenkinsfile_path.touch()  # the file now pre-exists
+        self.generated_files.append(generated_jenkinsfile_path)
 
-        self.assertEqual(init_process_execute.process.returncode, 1)
-        stderr = init_process_execute.stderr.decode()
-        self.assertIn(
-            'Pipeline file "Jenkinsfile" already exists in project root directory, please remove it first.', stderr
+        init_command_list = self.get_init_command_list()
+        init_process_execute = run_command_with_inputs(
+            init_command_list, [*QUICK_START_JENKINS_INPUTS_WITHOUT_AUTO_FILL, ""]
         )
 
+        self.assertEqual(init_process_execute.process.returncode, 0)
+
+        expected_file_path = Path(__file__).parent.parent.joinpath(Path("testdata", "pipeline", "expected_jenkinsfile"))
+        with open(expected_file_path, "r") as expected, open(
+            os.path.join(".aws-sam", "pipeline", "generated-files", "Jenkinsfile"), "r"
+        ) as output:
+            self.assertEqual(expected.read(), output.read())
+
+        # also check the Jenkinsfile is not overridden
+        self.assertEqual("", open("Jenkinsfile", "r").read())
+
     def test_custom_template(self):
         generated_file = Path("weather")
         self.generated_files.append(generated_file)

tests/unit/commands/pipeline/init/test_initeractive_init_flow.py

@@ -1,7 +1,8 @@
 import json
+import shutil
 import tempfile
 from unittest import TestCase
-from unittest.mock import patch, Mock, ANY, call
+from unittest.mock import patch, Mock, call
 import os
 from pathlib import Path
 
@@ -17,6 +18,7 @@
     _prompt_cicd_provider,
     _prompt_provider_pipeline_template,
     _get_pipeline_template_metadata,
+    _copy_dir_contents_to_cwd,
 )
 from samcli.commands.pipeline.init.pipeline_templates_manifest import AppPipelineTemplateManifestException
 from samcli.lib.utils.git_repo import CloneRepoException
@@ -114,14 +116,14 @@ def test_app_pipeline_templates_with_invalid_manifest(
     @patch("samcli.commands.pipeline.init.interactive_init_flow.InteractiveFlowCreator.create_flow")
     @patch("samcli.commands.pipeline.init.interactive_init_flow.PipelineTemplatesManifest")
     @patch("samcli.commands.pipeline.init.interactive_init_flow.GitRepo.clone")
-    @patch("samcli.commands.pipeline.init.interactive_init_flow._copy_dir_contents_to_cwd_fail_on_exist")
+    @patch("samcli.commands.pipeline.init.interactive_init_flow._copy_dir_contents_to_cwd")
     @patch("samcli.commands.pipeline.init.interactive_init_flow._get_pipeline_template_metadata")
     @patch("samcli.lib.cookiecutter.question.click")
     def test_generate_pipeline_configuration_file_from_app_pipeline_template_happy_case(
         self,
         click_mock,
         _get_pipeline_template_metadata_mock,
-        _copy_dir_contents_to_cwd_fail_on_exist_mock,
+        _copy_dir_contents_to_cwd_mock,
         clone_mock,
         PipelineTemplatesManifest_mock,
         create_interactive_flow_mock,
@@ -260,14 +262,14 @@ def test_generate_pipeline_configuration_file_from_custom_local_existing_path_wi
     @patch("samcli.commands.pipeline.init.interactive_init_flow.InteractiveFlowCreator.create_flow")
     @patch("samcli.commands.pipeline.init.interactive_init_flow.GitRepo.clone")
     @patch("samcli.commands.pipeline.init.interactive_init_flow.click")
-    @patch("samcli.commands.pipeline.init.interactive_init_flow._copy_dir_contents_to_cwd_fail_on_exist")
+    @patch("samcli.commands.pipeline.init.interactive_init_flow._copy_dir_contents_to_cwd")
     @patch("samcli.commands.pipeline.init.interactive_init_flow._get_pipeline_template_metadata")
     @patch("samcli.lib.cookiecutter.question.click")
     def test_generate_pipeline_configuration_file_from_custom_remote_pipeline_template_happy_case(
         self,
         questions_click_mock,
         _get_pipeline_template_metadata_mock,
-        _copy_dir_contents_to_cwd_fail_on_exist_mock,
+        _copy_dir_contents_to_cwd_mock,
         init_click_mock,
         clone_mock,
         create_interactive_flow_mock,
@@ -285,7 +287,7 @@ def test_generate_pipeline_configuration_file_from_custom_remote_pipeline_templa
         create_interactive_flow_mock.return_value = interactive_flow_mock
         cookiecutter_context_mock = {"key": "value"}
         interactive_flow_mock.run.return_value = cookiecutter_context_mock
-        _copy_dir_contents_to_cwd_fail_on_exist_mock.return_value = ["file1"]
+        _copy_dir_contents_to_cwd_mock.return_value = ["file1"]
 
         questions_click_mock.prompt.return_value = "2"  # Custom pipeline templates
         init_click_mock.prompt.return_value = "https://github.com/any-custom-pipeline-template-repo.git"
@@ -382,14 +384,14 @@ class TestInteractiveInitFlowWithBootstrap(TestCase):
     )
     @patch("samcli.commands.pipeline.init.interactive_init_flow.PipelineTemplatesManifest")
     @patch("samcli.commands.pipeline.init.interactive_init_flow.GitRepo.clone")
-    @patch("samcli.commands.pipeline.init.interactive_init_flow._copy_dir_contents_to_cwd_fail_on_exist")
+    @patch("samcli.commands.pipeline.init.interactive_init_flow._copy_dir_contents_to_cwd")
     @patch("samcli.commands.pipeline.init.interactive_init_flow._get_pipeline_template_metadata")
     @patch("samcli.lib.cookiecutter.question.click")
     def test_with_bootstrap_but_answer_no(
         self,
         click_mock,
         _get_pipeline_template_metadata_mock,
-        _copy_dir_contents_to_cwd_fail_on_exist_mock,
+        _copy_dir_contents_to_cwd_mock,
         clone_mock,
         PipelineTemplatesManifest_mock,
         _prompt_run_bootstrap_within_pipeline_init_mock,
@@ -458,7 +460,7 @@ def test_with_bootstrap_but_answer_no(
     )
     @patch("samcli.commands.pipeline.init.interactive_init_flow.PipelineTemplatesManifest")
     @patch("samcli.commands.pipeline.init.interactive_init_flow.GitRepo.clone")
-    @patch("samcli.commands.pipeline.init.interactive_init_flow._copy_dir_contents_to_cwd_fail_on_exist")
+    @patch("samcli.commands.pipeline.init.interactive_init_flow._copy_dir_contents_to_cwd")
     @patch("samcli.commands.pipeline.init.interactive_init_flow._get_pipeline_template_metadata")
     @patch("samcli.lib.cookiecutter.question.click")
     def test_with_bootstrap_answer_yes(
@@ -467,7 +469,7 @@ def test_with_bootstrap_answer_yes(
         _prompt_run_bootstrap_expected_calls,
         click_mock,
         _get_pipeline_template_metadata_mock,
-        _copy_dir_contents_to_cwd_fail_on_exist_mock,
+        _copy_dir_contents_to_cwd_mock,
         clone_mock,
         PipelineTemplatesManifest_mock,
         _prompt_run_bootstrap_within_pipeline_init_mock,
@@ -520,3 +522,45 @@ def test_with_bootstrap_answer_yes(
 
         # verify
         _prompt_run_bootstrap_within_pipeline_init_mock.assert_has_calls(_prompt_run_bootstrap_expected_calls)
+
+
+class TestInteractiveInitFlow_copy_dir_contents_to_cwd(TestCase):
+    def tearDown(self) -> None:
+        if Path("file").exists():
+            Path("file").unlink()
+        shutil.rmtree(os.path.join(".aws-sam", "pipeline"), ignore_errors=True)
+
+    @patch("samcli.commands.pipeline.init.interactive_init_flow.click.confirm")
+    def test_copy_dir_contents_to_cwd_no_need_override(self, confirm_mock):
+        with tempfile.TemporaryDirectory() as source:
+            confirm_mock.return_value = True
+            Path(source, "file").touch()
+            Path(source, "file").write_text("hi")
+            file_paths = _copy_dir_contents_to_cwd(source)
+            confirm_mock.assert_not_called()
+            self.assertEqual("hi", Path("file").read_text(encoding="utf-8"))
+            self.assertEqual([str(Path(".", "file"))], file_paths)
+
+    @patch("samcli.commands.pipeline.init.interactive_init_flow.click.confirm")
+    def test_copy_dir_contents_to_cwd_override(self, confirm_mock):
+        with tempfile.TemporaryDirectory() as source:
+            confirm_mock.return_value = True
+            Path(source, "file").touch()
+            Path(source, "file").write_text("hi")
+            Path("file").touch()
+            file_paths = _copy_dir_contents_to_cwd(source)
+            confirm_mock.assert_called_once()
+            self.assertEqual("hi", Path("file").read_text(encoding="utf-8"))
+            self.assertEqual([str(Path(".", "file"))], file_paths)
+
+    @patch("samcli.commands.pipeline.init.interactive_init_flow.click.confirm")
+    def test_copy_dir_contents_to_cwd_not_override(self, confirm_mock):
+        with tempfile.TemporaryDirectory() as source:
+            confirm_mock.return_value = False
+            Path(source, "file").touch()
+            Path(source, "file").write_text("hi")
+            Path("file").touch()
+            file_paths = _copy_dir_contents_to_cwd(source)
+            confirm_mock.assert_called_once()
+            self.assertEqual("", Path("file").read_text(encoding="utf-8"))
+            self.assertEqual([str(Path(".aws-sam", "pipeline", "generated-files", "file"))], file_paths)