From aca8e9a9faee83befee81f88b6c29624703f04fd Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Fri, 26 Mar 2021 16:06:33 -0700 Subject: [PATCH 01/24] Added integration tests --- samcli/lib/build/app_builder.py | 4 +- .../integration/buildcmd/build_integ_base.py | 4 + tests/integration/buildcmd/test_build_cmd.py | 109 ++++++++++++++++++ .../buildcmd/ProvidedWithBuildImage/main.js | 4 + .../ProvidedWithBuildImage/package.json | 11 ++ .../ProvidedWithBuildImage/requirements.txt | 0 .../buildcmd/provided_image_function.yaml | 15 +++ 7 files changed, 145 insertions(+), 2 deletions(-) create mode 100644 tests/integration/testdata/buildcmd/ProvidedWithBuildImage/main.js create mode 100644 tests/integration/testdata/buildcmd/ProvidedWithBuildImage/package.json create mode 100644 tests/integration/testdata/buildcmd/ProvidedWithBuildImage/requirements.txt create mode 100644 tests/integration/testdata/buildcmd/provided_image_function.yaml diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index 3f8eef806d..a481d44f5c 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -67,7 +67,7 @@ def __init__( docker_client: Optional[docker.DockerClient] = None, container_env_var: Optional[Dict] = None, container_env_var_file: Optional[str] = None, - build_images: Optional[dict] = None, + build_images: Optional[Dict] = None, ) -> None: """ Initialize the class @@ -577,7 +577,7 @@ def _build_function_in_process( scratch_dir: str, manifest_path: str, runtime: str, - options: Optional[dict], + options: Optional[Dict], ) -> str: builder = LambdaBuilder( diff --git a/tests/integration/buildcmd/build_integ_base.py b/tests/integration/buildcmd/build_integ_base.py index b2211e53b4..54e122d04f 100644 --- a/tests/integration/buildcmd/build_integ_base.py +++ b/tests/integration/buildcmd/build_integ_base.py @@ -71,6 +71,7 @@ def get_command_list( parallel=False, container_env_var=None, container_env_var_file=None, + build_image=None, ): command_list = [self.cmd, "build"] @@ -113,6 +114,9 @@ def get_command_list( if container_env_var_file: command_list += ["--container-env-var-file", container_env_var_file] + if build_image: + command_list += ["--build-image", build_image] + return command_list def verify_docker_container_cleanedup(self, runtime): diff --git a/tests/integration/buildcmd/test_build_cmd.py b/tests/integration/buildcmd/test_build_cmd.py index 2fea281f7d..cd684518f2 100644 --- a/tests/integration/buildcmd/test_build_cmd.py +++ b/tests/integration/buildcmd/test_build_cmd.py @@ -7,6 +7,7 @@ from unittest import skipIf from pathlib import Path from parameterized import parameterized, parameterized_class +from subprocess import Popen, PIPE, TimeoutExpired import pytest @@ -1837,3 +1838,111 @@ def test_nested_build(self, use_container, cached, parallel): ("LocalNestedStack/Function2", {"pi": "3.14"}), ], ) + +@parameterized_class( + ("template", "stack_paths", "layer_full_path", "function_full_paths", "invoke_error_message"), + [ + ( + os.path.join("nested-with-intrinsic-functions", "template-pass-down.yaml"), + ["", "AppUsingRef", "AppUsingJoin"], + "MyLayerVersion", + ["AppUsingRef/FunctionInChild", "AppUsingJoin/FunctionInChild"], + # Note(xinhol), intrinsic function passed by parameter are resolved as string, + # therefore it is being treated as an Arn, it is a bug in intrinsic resolver + "Invalid Layer Arn", + ), + ( + os.path.join("nested-with-intrinsic-functions", "template-pass-up.yaml"), + ["", "ChildApp"], + "ChildApp/MyLayerVersion", + ["FunctionInRoot"], + # for this pass-up use case, since we are not sure whether there are valid local invoke cases out there, + # so we don't want to block customers from local invoking it. + None, + ), + ], +) +class TestBuildPassingLayerAcrossStacks(IntrinsicIntegBase): + @pytest.mark.flaky(reruns=3) + def test_nested_build(self): + if SKIP_DOCKER_TESTS: + self.skipTest(SKIP_DOCKER_MESSAGE) + + """ + Build template above and verify that each function call returns as expected + """ + cmdlist = self.get_command_list( + use_container=True, + cached=True, + parallel=True, + ) + + LOG.info("Running Command: %s", cmdlist) + LOG.info(self.working_dir) + + command_result = run_command(cmdlist, cwd=self.working_dir) + + if not SKIP_DOCKER_TESTS: + self._verify_build( + self.function_full_paths, + self.layer_full_path, + self.stack_paths, + command_result, + ) + + self._verify_invoke_built_functions( + self.built_template, self.function_full_paths, self.invoke_error_message + ) + + +@skipIf( + ((IS_WINDOWS and RUNNING_ON_CI) and not CI_OVERRIDE), + "Skip build tests on windows when running in CI unless overridden", +) +class TestBuildWithCustomBuildImage(BuildIntegBase): + template = "provided_image_function.yaml" + + @parameterized.expand( + [ + ("use_container", None), + ("use_container", "amazon/aws-sam-cli-build-image-nodejs10.x"), + ] + ) + @pytest.mark.flaky(reruns=3) + def test_custom_build_image_succeeds(self, use_container, build_image): + if use_container and SKIP_DOCKER_TESTS: + self.skipTest(SKIP_DOCKER_MESSAGE) + + cmdlist = self.get_command_list(use_container=use_container, build_image=build_image) + + LOG.info("Running Command: {}".format(cmdlist)) + process = Popen(cmdlist, cwd=self.working_dir, stdout=PIPE, stderr=PIPE) + try: + stdout, stderr = process.communicate(timeout=TIMEOUT) + LOG.info(f"Stdout: {stdout.decode('utf-8')}") + LOG.info(f"Stderr: {stderr.decode('utf-8')}") + except TimeoutExpired: + LOG.error(f"Command: {command_list}, TIMED OUT") + LOG.error(f"Return Code: {process_execute.returncode}") + process.kill() + raise + process_stderr = stderr.strip() + + self._verify_right_image_pulled(build_image, process_stderr) + self._verify_build_succeeds(self.default_build_dir) + + self.verify_docker_container_cleanedup("nodejs10.x") + + def _verify_right_image_pulled(self, build_image, process_stderr): + image_name = build_image if build_image is not None else "public.ecr.aws/sam/build-nodejs10.x" + processed_name = bytes(image_name, encoding="utf-8") + self.assertIn( + processed_name, + process_stderr, + ) + + def _verify_build_succeeds(self, build_dir): + self.assertTrue(build_dir.exists(), "Build directory should be created") + + build_dir_files = os.listdir(str(build_dir)) + self.assertIn("BuildImageFunction", build_dir_files) diff --git a/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/main.js b/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/main.js new file mode 100644 index 0000000000..3e0a1b1f38 --- /dev/null +++ b/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/main.js @@ -0,0 +1,4 @@ + +exports.lambdaHandler = async (event, context) => { + return 'Hello World' +}; diff --git a/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/package.json b/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/package.json new file mode 100644 index 0000000000..c95b709aec --- /dev/null +++ b/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/package.json @@ -0,0 +1,11 @@ +{ + "name": "npmdeps", + "version": "1.0.0", + "description": "", + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "*" + } +} \ No newline at end of file diff --git a/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/requirements.txt b/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/requirements.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integration/testdata/buildcmd/provided_image_function.yaml b/tests/integration/testdata/buildcmd/provided_image_function.yaml new file mode 100644 index 0000000000..999c843564 --- /dev/null +++ b/tests/integration/testdata/buildcmd/provided_image_function.yaml @@ -0,0 +1,15 @@ +AWSTemplateFormatVersion : '2010-09-09' +Transform: AWS::Serverless-2016-10-31 + +Globals: + Function: + Timeout: 20 + MemorySize: 512 + +Resources: + BuildImageFunction: + Type: AWS::Serverless::Function + Properties: + CodeUri: ProvidedWithBuildImage + Handler: main.lambdaHandler + Runtime: nodejs10.x \ No newline at end of file From 555802dbdd4a63635bc644b1a7b7aba4bc84f930 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Mon, 29 Mar 2021 11:01:53 -0700 Subject: [PATCH 02/24] Added click requirement on -u presence for container based options --- samcli/commands/build/command.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index 4c2a7e2654..c726823db2 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -20,6 +20,7 @@ from samcli.cli.cli_config_file import configuration_option, TomlProvider from samcli.lib.utils.version_checker import check_newer_version from samcli.commands.build.exceptions import InvalidBuildImageException +from samcli.commands.local.cli_common.click_container import Container LOG = logging.getLogger(__name__) @@ -117,6 +118,8 @@ help="Input environment variables through command line to pass into build containers, you can either " "input function specific format (FuncName.VarName=Value) or global format (VarName=Value). e.g., " "sam build --use-container --container-env-var Func1.VAR1=value1 --container-env-var VAR2=value2", + cls=Container, + require_container=True, ) @click.option( "--container-env-var-file", @@ -124,6 +127,8 @@ default=None, type=click.Path(), # Must be a json file help="Path to environment variable json file (e.g., env_vars.json) to pass into build containers", + cls=Container, + require_container=True, ) @click.option( "--build-image", @@ -138,6 +143,8 @@ "(--build-image FunctionLogicalID=public.ecr.aws/sam/build-nodejs14.x:latest). " "A combination of the two can be used. If a function does not have build image specified or " "an image URI for all functions, the default SAM CLI build images will be used.", + cls=Container, + require_container=True, ) @click.option( "--parallel", From 644a1daf5399df8ab53b355a1bf3810d4e07e2e7 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Mon, 29 Mar 2021 11:03:12 -0700 Subject: [PATCH 03/24] Added click class that checks -u presence for container based options --- .../local/cli_common/click_container.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 samcli/commands/local/cli_common/click_container.py diff --git a/samcli/commands/local/cli_common/click_container.py b/samcli/commands/local/cli_common/click_container.py new file mode 100644 index 0000000000..c614499bd6 --- /dev/null +++ b/samcli/commands/local/cli_common/click_container.py @@ -0,0 +1,26 @@ +from typing import List + +import click + + +class Container(click.Option): + """ + Preprocessing checks for presence of --use-container flag for container based options. + """ + + def __init__(self, *args, **kwargs): + self.require_container: bool = kwargs.pop("require_container", None) + + super().__init__(*args, **kwargs) + + def handle_parse_result(self, ctx, opts, args): + if self.require_container: + if "use_container" not in opts: + msg = f""" +Missing required parameter, with --{self.name.replace("_", "-")} set. + +Must provide the --use-container flag in order to use --{self.name.replace("_", "-")} flag. + """ + raise click.UsageError(msg) + self.prompt = None + return super().handle_parse_result(ctx, opts, args) From 12c6cf29e6a468c55f210aa4ed4871d713b56346 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Mon, 29 Mar 2021 14:31:02 -0700 Subject: [PATCH 04/24] Reformatting and fixing test cases --- samcli/commands/local/cli_common/click_container.py | 5 +++-- tests/unit/commands/samconfig/test_samconfig.py | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/samcli/commands/local/cli_common/click_container.py b/samcli/commands/local/cli_common/click_container.py index c614499bd6..6fb7d4d879 100644 --- a/samcli/commands/local/cli_common/click_container.py +++ b/samcli/commands/local/cli_common/click_container.py @@ -1,5 +1,6 @@ -from typing import List - +""" +Module to check container based cli parameters +""" import click diff --git a/tests/unit/commands/samconfig/test_samconfig.py b/tests/unit/commands/samconfig/test_samconfig.py index c5fcb47fd5..bfadecddd2 100644 --- a/tests/unit/commands/samconfig/test_samconfig.py +++ b/tests/unit/commands/samconfig/test_samconfig.py @@ -122,7 +122,7 @@ def test_build(self, do_cli_mock): LOG.debug(Path(config_path).read_text()) runner = CliRunner() - result = runner.invoke(cli, []) + result = runner.invoke(cli, ["--use-container"]) LOG.info(result.output) LOG.info(result.exception) @@ -174,7 +174,7 @@ def test_build_with_container_env_vars(self, do_cli_mock): LOG.debug(Path(config_path).read_text()) runner = CliRunner() - result = runner.invoke(cli, []) + result = runner.invoke(cli, ["--use-container"]) LOG.info(result.output) LOG.info(result.exception) @@ -225,7 +225,7 @@ def test_build_with_build_images(self, do_cli_mock): LOG.debug(Path(config_path).read_text()) runner = CliRunner() - result = runner.invoke(cli, []) + result = runner.invoke(cli, ["--use-container"]) LOG.info(result.output) LOG.info(result.exception) From 4c851b737250aeedfd8c45ba53f8177b9562f3fc Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Mon, 29 Mar 2021 15:29:43 -0700 Subject: [PATCH 05/24] Fix for integration test failures --- samcli/commands/local/cli_common/click_container.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samcli/commands/local/cli_common/click_container.py b/samcli/commands/local/cli_common/click_container.py index 6fb7d4d879..d28cf5857e 100644 --- a/samcli/commands/local/cli_common/click_container.py +++ b/samcli/commands/local/cli_common/click_container.py @@ -16,7 +16,7 @@ def __init__(self, *args, **kwargs): def handle_parse_result(self, ctx, opts, args): if self.require_container: - if "use_container" not in opts: + if "use_container" not in opts and opts.get(self.name) is not None: msg = f""" Missing required parameter, with --{self.name.replace("_", "-")} set. From 7c83d7ec91fd793017560107a9f553b9eaeb5a43 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Tue, 30 Mar 2021 12:15:13 -0700 Subject: [PATCH 06/24] Added support for layers --- samcli/lib/build/app_builder.py | 8 +++++++- tests/unit/lib/build_module/test_app_builder.py | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index a481d44f5c..8ef4ba0189 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -441,8 +441,14 @@ def _build_layer( # Only set to this value if specified workflow is makefile # which will result in config language as provided build_runtime = compatible_runtimes[0] + image = None + if self._build_images is not None: + if layer_name in self._build_images: + image = self._build_images[layer_name] + elif None in self._build_images: + image = self._build_images[None] self._build_function_on_container( - config, code_dir, artifact_subdir, manifest_path, build_runtime, options, container_env_vars + config, code_dir, artifact_subdir, manifest_path, build_runtime, options, container_env_vars, image ) else: self._build_function_in_process( diff --git a/tests/unit/lib/build_module/test_app_builder.py b/tests/unit/lib/build_module/test_app_builder.py index 4fd7acb823..03a40f7d87 100644 --- a/tests/unit/lib/build_module/test_app_builder.py +++ b/tests/unit/lib/build_module/test_app_builder.py @@ -419,6 +419,7 @@ def test_must_build_layer_in_container(self, get_layer_subfolder_mock, osutils_m "python3.8", None, None, + None, ) From f023b88119f996527c86ad42cc490c7e17f756e4 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Tue, 30 Mar 2021 14:03:40 -0700 Subject: [PATCH 07/24] Resolve conflict --- tests/integration/buildcmd/test_build_cmd.py | 55 -------------------- 1 file changed, 55 deletions(-) diff --git a/tests/integration/buildcmd/test_build_cmd.py b/tests/integration/buildcmd/test_build_cmd.py index cd684518f2..443168ff60 100644 --- a/tests/integration/buildcmd/test_build_cmd.py +++ b/tests/integration/buildcmd/test_build_cmd.py @@ -1839,61 +1839,6 @@ def test_nested_build(self, use_container, cached, parallel): ], ) -@parameterized_class( - ("template", "stack_paths", "layer_full_path", "function_full_paths", "invoke_error_message"), - [ - ( - os.path.join("nested-with-intrinsic-functions", "template-pass-down.yaml"), - ["", "AppUsingRef", "AppUsingJoin"], - "MyLayerVersion", - ["AppUsingRef/FunctionInChild", "AppUsingJoin/FunctionInChild"], - # Note(xinhol), intrinsic function passed by parameter are resolved as string, - # therefore it is being treated as an Arn, it is a bug in intrinsic resolver - "Invalid Layer Arn", - ), - ( - os.path.join("nested-with-intrinsic-functions", "template-pass-up.yaml"), - ["", "ChildApp"], - "ChildApp/MyLayerVersion", - ["FunctionInRoot"], - # for this pass-up use case, since we are not sure whether there are valid local invoke cases out there, - # so we don't want to block customers from local invoking it. - None, - ), - ], -) -class TestBuildPassingLayerAcrossStacks(IntrinsicIntegBase): - @pytest.mark.flaky(reruns=3) - def test_nested_build(self): - if SKIP_DOCKER_TESTS: - self.skipTest(SKIP_DOCKER_MESSAGE) - - """ - Build template above and verify that each function call returns as expected - """ - cmdlist = self.get_command_list( - use_container=True, - cached=True, - parallel=True, - ) - - LOG.info("Running Command: %s", cmdlist) - LOG.info(self.working_dir) - - command_result = run_command(cmdlist, cwd=self.working_dir) - - if not SKIP_DOCKER_TESTS: - self._verify_build( - self.function_full_paths, - self.layer_full_path, - self.stack_paths, - command_result, - ) - - self._verify_invoke_built_functions( - self.built_template, self.function_full_paths, self.invoke_error_message - ) - @skipIf( ((IS_WINDOWS and RUNNING_ON_CI) and not CI_OVERRIDE), From f38adba9145d2eb476bf8dc31848fb79d3f16566 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Tue, 30 Mar 2021 15:36:57 -0700 Subject: [PATCH 08/24] Refactoring file location --- samcli/commands/{local/cli_common => build}/click_container.py | 0 samcli/commands/build/command.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename samcli/commands/{local/cli_common => build}/click_container.py (100%) diff --git a/samcli/commands/local/cli_common/click_container.py b/samcli/commands/build/click_container.py similarity index 100% rename from samcli/commands/local/cli_common/click_container.py rename to samcli/commands/build/click_container.py diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index c726823db2..fa0a167bb4 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -20,7 +20,7 @@ from samcli.cli.cli_config_file import configuration_option, TomlProvider from samcli.lib.utils.version_checker import check_newer_version from samcli.commands.build.exceptions import InvalidBuildImageException -from samcli.commands.local.cli_common.click_container import Container +from samcli.commands.build.click_container import Container LOG = logging.getLogger(__name__) From 5e6b4801a4bf8edfb6aef26246446525ad0011b9 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Wed, 31 Mar 2021 15:12:39 -0700 Subject: [PATCH 09/24] Addressing review comments, refactoring --- samcli/commands/build/click_container.py | 8 ++++---- samcli/commands/build/command.py | 8 ++++---- samcli/lib/build/app_builder.py | 20 +++++++++---------- tests/integration/buildcmd/test_build_cmd.py | 8 ++++---- .../buildcmd/ProvidedWithBuildImage/main.js | 4 ---- .../ProvidedWithBuildImage/package.json | 11 ---------- .../ProvidedWithBuildImage/requirements.txt | 0 ...unction.yaml => build_image_function.yaml} | 6 +++--- 8 files changed, 24 insertions(+), 41 deletions(-) delete mode 100644 tests/integration/testdata/buildcmd/ProvidedWithBuildImage/main.js delete mode 100644 tests/integration/testdata/buildcmd/ProvidedWithBuildImage/package.json delete mode 100644 tests/integration/testdata/buildcmd/ProvidedWithBuildImage/requirements.txt rename tests/integration/testdata/buildcmd/{provided_image_function.yaml => build_image_function.yaml} (69%) diff --git a/samcli/commands/build/click_container.py b/samcli/commands/build/click_container.py index d28cf5857e..6a4fd0a7f5 100644 --- a/samcli/commands/build/click_container.py +++ b/samcli/commands/build/click_container.py @@ -4,23 +4,23 @@ import click -class Container(click.Option): +class ContainerOptions(click.Option): """ Preprocessing checks for presence of --use-container flag for container based options. """ def __init__(self, *args, **kwargs): - self.require_container: bool = kwargs.pop("require_container", None) + self.require_container: bool = kwargs.pop("require_container", False) super().__init__(*args, **kwargs) def handle_parse_result(self, ctx, opts, args): if self.require_container: if "use_container" not in opts and opts.get(self.name) is not None: - msg = f""" + msg = f"""\ Missing required parameter, with --{self.name.replace("_", "-")} set. -Must provide the --use-container flag in order to use --{self.name.replace("_", "-")} flag. +Must provide the --use-container flag in order to use --{self.name.replace("_", "-")} flag.\ """ raise click.UsageError(msg) self.prompt = None diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index fa0a167bb4..76a8216541 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -20,7 +20,7 @@ from samcli.cli.cli_config_file import configuration_option, TomlProvider from samcli.lib.utils.version_checker import check_newer_version from samcli.commands.build.exceptions import InvalidBuildImageException -from samcli.commands.build.click_container import Container +from samcli.commands.build.click_container import ContainerOptions LOG = logging.getLogger(__name__) @@ -118,7 +118,7 @@ help="Input environment variables through command line to pass into build containers, you can either " "input function specific format (FuncName.VarName=Value) or global format (VarName=Value). e.g., " "sam build --use-container --container-env-var Func1.VAR1=value1 --container-env-var VAR2=value2", - cls=Container, + cls=ContainerOptions, require_container=True, ) @click.option( @@ -127,7 +127,7 @@ default=None, type=click.Path(), # Must be a json file help="Path to environment variable json file (e.g., env_vars.json) to pass into build containers", - cls=Container, + cls=ContainerOptions, require_container=True, ) @click.option( @@ -143,7 +143,7 @@ "(--build-image FunctionLogicalID=public.ecr.aws/sam/build-nodejs14.x:latest). " "A combination of the two can be used. If a function does not have build image specified or " "an image URI for all functions, the default SAM CLI build images will be used.", - cls=Container, + cls=ContainerOptions, require_container=True, ) @click.option( diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index 8ef4ba0189..529f696742 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -125,7 +125,7 @@ def __init__( self._colored = Colored() self._container_env_var = container_env_var self._container_env_var_file = container_env_var_file - self._build_images = build_images + self._build_images = build_images or {} def build(self) -> Dict[str, str]: """ @@ -442,11 +442,10 @@ def _build_layer( # which will result in config language as provided build_runtime = compatible_runtimes[0] image = None - if self._build_images is not None: - if layer_name in self._build_images: - image = self._build_images[layer_name] - elif None in self._build_images: - image = self._build_images[None] + if layer_name in self._build_images: + image = self._build_images.get(layer_name) + elif None in self._build_images: + image = self._build_images.get(None) self._build_function_on_container( config, code_dir, artifact_subdir, manifest_path, build_runtime, options, container_env_vars, image ) @@ -527,11 +526,10 @@ def _build_function( # pylint: disable=R1710 # By default prefer to build in-process for speed if self._container_manager: image = None - if self._build_images is not None: - if function_name in self._build_images: - image = self._build_images[function_name] - elif None in self._build_images: - image = self._build_images[None] + if function_name in self._build_images: + image = self._build_images.get(function_name) + elif None in self._build_images: + image = self._build_images.get(None) return self._build_function_on_container( config, diff --git a/tests/integration/buildcmd/test_build_cmd.py b/tests/integration/buildcmd/test_build_cmd.py index 443168ff60..828ba6f7fe 100644 --- a/tests/integration/buildcmd/test_build_cmd.py +++ b/tests/integration/buildcmd/test_build_cmd.py @@ -1845,12 +1845,12 @@ def test_nested_build(self, use_container, cached, parallel): "Skip build tests on windows when running in CI unless overridden", ) class TestBuildWithCustomBuildImage(BuildIntegBase): - template = "provided_image_function.yaml" + template = "build_image_function.yaml" @parameterized.expand( [ ("use_container", None), - ("use_container", "amazon/aws-sam-cli-build-image-nodejs10.x"), + ("use_container", "amazon/aws-sam-cli-build-image-python3.7:latest"), ] ) @pytest.mark.flaky(reruns=3) @@ -1876,10 +1876,10 @@ def test_custom_build_image_succeeds(self, use_container, build_image): self._verify_right_image_pulled(build_image, process_stderr) self._verify_build_succeeds(self.default_build_dir) - self.verify_docker_container_cleanedup("nodejs10.x") + self.verify_docker_container_cleanedup("python3.7") def _verify_right_image_pulled(self, build_image, process_stderr): - image_name = build_image if build_image is not None else "public.ecr.aws/sam/build-nodejs10.x" + image_name = build_image if build_image is not None else "public.ecr.aws/sam/build-python3.7:latest" processed_name = bytes(image_name, encoding="utf-8") self.assertIn( processed_name, diff --git a/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/main.js b/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/main.js deleted file mode 100644 index 3e0a1b1f38..0000000000 --- a/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/main.js +++ /dev/null @@ -1,4 +0,0 @@ - -exports.lambdaHandler = async (event, context) => { - return 'Hello World' -}; diff --git a/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/package.json b/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/package.json deleted file mode 100644 index c95b709aec..0000000000 --- a/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/package.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "name": "npmdeps", - "version": "1.0.0", - "description": "", - "keywords": [], - "author": "", - "license": "APACHE2.0", - "dependencies": { - "minimal-request-promise": "*" - } -} \ No newline at end of file diff --git a/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/requirements.txt b/tests/integration/testdata/buildcmd/ProvidedWithBuildImage/requirements.txt deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/integration/testdata/buildcmd/provided_image_function.yaml b/tests/integration/testdata/buildcmd/build_image_function.yaml similarity index 69% rename from tests/integration/testdata/buildcmd/provided_image_function.yaml rename to tests/integration/testdata/buildcmd/build_image_function.yaml index 999c843564..5f335b9574 100644 --- a/tests/integration/testdata/buildcmd/provided_image_function.yaml +++ b/tests/integration/testdata/buildcmd/build_image_function.yaml @@ -10,6 +10,6 @@ Resources: BuildImageFunction: Type: AWS::Serverless::Function Properties: - CodeUri: ProvidedWithBuildImage - Handler: main.lambdaHandler - Runtime: nodejs10.x \ No newline at end of file + CodeUri: Python + Handler: main.handler + Runtime: python3.7 \ No newline at end of file From 4337ccd07c5f0ff362d89b91fe55d2c3e7fbaead Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Wed, 31 Mar 2021 17:14:31 -0700 Subject: [PATCH 10/24] Reformatting logging sentence --- samcli/commands/build/click_container.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/samcli/commands/build/click_container.py b/samcli/commands/build/click_container.py index 6a4fd0a7f5..fd369a501f 100644 --- a/samcli/commands/build/click_container.py +++ b/samcli/commands/build/click_container.py @@ -20,8 +20,7 @@ def handle_parse_result(self, ctx, opts, args): msg = f"""\ Missing required parameter, with --{self.name.replace("_", "-")} set. -Must provide the --use-container flag in order to use --{self.name.replace("_", "-")} flag.\ - """ +Must provide the --use-container flag in order to use --{self.name.replace("_", "-")} flag.""" raise click.UsageError(msg) self.prompt = None return super().handle_parse_result(ctx, opts, args) From 6970d945132f4e5a9a9f6fa3e8cc923a3c9be36d Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Wed, 31 Mar 2021 17:23:16 -0700 Subject: [PATCH 11/24] Refactoring container check for simplicity --- samcli/commands/build/click_container.py | 11 ++++------- samcli/commands/build/command.py | 3 --- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/samcli/commands/build/click_container.py b/samcli/commands/build/click_container.py index fd369a501f..2135389aaa 100644 --- a/samcli/commands/build/click_container.py +++ b/samcli/commands/build/click_container.py @@ -10,17 +10,14 @@ class ContainerOptions(click.Option): """ def __init__(self, *args, **kwargs): - self.require_container: bool = kwargs.pop("require_container", False) - super().__init__(*args, **kwargs) def handle_parse_result(self, ctx, opts, args): - if self.require_container: - if "use_container" not in opts and opts.get(self.name) is not None: - msg = f"""\ + if "use_container" not in opts and opts.get(self.name) is not None: + msg = f"""\ Missing required parameter, with --{self.name.replace("_", "-")} set. Must provide the --use-container flag in order to use --{self.name.replace("_", "-")} flag.""" - raise click.UsageError(msg) - self.prompt = None + raise click.UsageError(msg) + self.prompt = None return super().handle_parse_result(ctx, opts, args) diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index 76a8216541..8edb1e2b05 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -119,7 +119,6 @@ "input function specific format (FuncName.VarName=Value) or global format (VarName=Value). e.g., " "sam build --use-container --container-env-var Func1.VAR1=value1 --container-env-var VAR2=value2", cls=ContainerOptions, - require_container=True, ) @click.option( "--container-env-var-file", @@ -128,7 +127,6 @@ type=click.Path(), # Must be a json file help="Path to environment variable json file (e.g., env_vars.json) to pass into build containers", cls=ContainerOptions, - require_container=True, ) @click.option( "--build-image", @@ -144,7 +142,6 @@ "A combination of the two can be used. If a function does not have build image specified or " "an image URI for all functions, the default SAM CLI build images will be used.", cls=ContainerOptions, - require_container=True, ) @click.option( "--parallel", From e37cb02b86bd6789e26396fe140bfc0879ec70a2 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Wed, 31 Mar 2021 17:35:28 -0700 Subject: [PATCH 12/24] Clarifying test case --- tests/unit/commands/samconfig/test_samconfig.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/commands/samconfig/test_samconfig.py b/tests/unit/commands/samconfig/test_samconfig.py index bfadecddd2..85725eebb1 100644 --- a/tests/unit/commands/samconfig/test_samconfig.py +++ b/tests/unit/commands/samconfig/test_samconfig.py @@ -114,6 +114,9 @@ def test_build(self, do_cli_mock): "docker_network": "mynetwork", "skip_pull_image": True, "parameter_overrides": "ParameterKey=Key,ParameterValue=Value ParameterKey=Key2,ParameterValue=Value2", + "container_env_var": (""), + "container_env_var_file": "file", + "build_image": (""), } with samconfig_parameters(["build"], self.scratch_dir, **config_values) as config_path: @@ -146,7 +149,7 @@ def test_build(self, do_cli_mock): {"Key": "Value", "Key2": "Value2"}, None, (), - None, + "file", (), ) From 805d7476ffcc344711e4344bb99c93548dc3c961 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Wed, 31 Mar 2021 19:10:35 -0700 Subject: [PATCH 13/24] Added unit tests on layer builds --- .../unit/lib/build_module/test_app_builder.py | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/unit/lib/build_module/test_app_builder.py b/tests/unit/lib/build_module/test_app_builder.py index 03a40f7d87..8645a7c6c5 100644 --- a/tests/unit/lib/build_module/test_app_builder.py +++ b/tests/unit/lib/build_module/test_app_builder.py @@ -422,6 +422,72 @@ def test_must_build_layer_in_container(self, get_layer_subfolder_mock, osutils_m None, ) + @patch("samcli.lib.build.app_builder.get_workflow_config") + @patch("samcli.lib.build.app_builder.osutils") + @patch("samcli.lib.build.app_builder.get_layer_subfolder") + def test_must_build_layer_in_container_with_global_build_image( + self, get_layer_subfolder_mock, osutils_mock, get_workflow_config_mock + ): + self.builder._container_manager = self.container_manager + get_layer_subfolder_mock.return_value = "python" + config_mock = Mock() + config_mock.manifest_name = "manifest_name" + + scratch_dir = "scratch" + osutils_mock.mkdir_temp.return_value.__enter__ = Mock(return_value=scratch_dir) + osutils_mock.mkdir_temp.return_value.__exit__ = Mock() + + get_workflow_config_mock.return_value = config_mock + build_function_on_container_mock = Mock() + + build_images = {None: "test_image"} + self.builder._build_images = build_images + self.builder._build_function_on_container = build_function_on_container_mock + self.builder._build_layer("layer_name", "code_uri", "python3.8", ["python3.8"], "full_path") + build_function_on_container_mock.assert_called_once_with( + config_mock, + PathValidator("code_uri"), + PathValidator("python"), + PathValidator("manifest_name"), + "python3.8", + None, + None, + "test_image", + ) + + @patch("samcli.lib.build.app_builder.get_workflow_config") + @patch("samcli.lib.build.app_builder.osutils") + @patch("samcli.lib.build.app_builder.get_layer_subfolder") + def test_must_build_layer_in_container_with_specific_build_image( + self, get_layer_subfolder_mock, osutils_mock, get_workflow_config_mock + ): + self.builder._container_manager = self.container_manager + get_layer_subfolder_mock.return_value = "python" + config_mock = Mock() + config_mock.manifest_name = "manifest_name" + + scratch_dir = "scratch" + osutils_mock.mkdir_temp.return_value.__enter__ = Mock(return_value=scratch_dir) + osutils_mock.mkdir_temp.return_value.__exit__ = Mock() + + get_workflow_config_mock.return_value = config_mock + build_function_on_container_mock = Mock() + + build_images = {"layer_name": "test_image"} + self.builder._build_images = build_images + self.builder._build_function_on_container = build_function_on_container_mock + self.builder._build_layer("layer_name", "code_uri", "python3.8", ["python3.8"], "full_path") + build_function_on_container_mock.assert_called_once_with( + config_mock, + PathValidator("code_uri"), + PathValidator("python"), + PathValidator("manifest_name"), + "python3.8", + None, + None, + "test_image", + ) + class TestApplicationBuilder_update_template(TestCase): def make_root_template(self, resource_type, location_property_name): From b68e6f90edfeee066b42e6580ad24336e0e703fc Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Thu, 1 Apr 2021 11:05:49 -0700 Subject: [PATCH 14/24] Improving readability --- samcli/commands/build/click_container.py | 4 +--- samcli/lib/build/app_builder.py | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/samcli/commands/build/click_container.py b/samcli/commands/build/click_container.py index 2135389aaa..e4326117f3 100644 --- a/samcli/commands/build/click_container.py +++ b/samcli/commands/build/click_container.py @@ -9,9 +9,6 @@ class ContainerOptions(click.Option): Preprocessing checks for presence of --use-container flag for container based options. """ - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - def handle_parse_result(self, ctx, opts, args): if "use_container" not in opts and opts.get(self.name) is not None: msg = f"""\ @@ -19,5 +16,6 @@ def handle_parse_result(self, ctx, opts, args): Must provide the --use-container flag in order to use --{self.name.replace("_", "-")} flag.""" raise click.UsageError(msg) + # To make sure no unser input prompting happens self.prompt = None return super().handle_parse_result(ctx, opts, args) diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index 529f696742..aee76089bd 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -444,6 +444,7 @@ def _build_layer( image = None if layer_name in self._build_images: image = self._build_images.get(layer_name) + # None represents the global build image for all functions/layers elif None in self._build_images: image = self._build_images.get(None) self._build_function_on_container( @@ -528,6 +529,7 @@ def _build_function( # pylint: disable=R1710 image = None if function_name in self._build_images: image = self._build_images.get(function_name) + # None represents the global build image for all functions/layers elif None in self._build_images: image = self._build_images.get(None) From 83b391edcc37bc0dcddd901123582e3ba373a1a3 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Thu, 1 Apr 2021 13:42:27 -0700 Subject: [PATCH 15/24] Refactoring to simplify --- samcli/lib/build/app_builder.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index aee76089bd..345fbfe97a 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -442,11 +442,9 @@ def _build_layer( # which will result in config language as provided build_runtime = compatible_runtimes[0] image = None - if layer_name in self._build_images: - image = self._build_images.get(layer_name) - # None represents the global build image for all functions/layers - elif None in self._build_images: - image = self._build_images.get(None) + # None key represents the global build image for all functions/layers + global_image = self._build_images.get(None) + image = self._build_images.get(layer_name, global_image) self._build_function_on_container( config, code_dir, artifact_subdir, manifest_path, build_runtime, options, container_env_vars, image ) @@ -527,11 +525,9 @@ def _build_function( # pylint: disable=R1710 # By default prefer to build in-process for speed if self._container_manager: image = None - if function_name in self._build_images: - image = self._build_images.get(function_name) # None represents the global build image for all functions/layers - elif None in self._build_images: - image = self._build_images.get(None) + global_image = self._build_images.get(None) + image = self._build_images.get(function_name, global_image) return self._build_function_on_container( config, From 891250d917a55fe81995263cce4a0de212915795 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Thu, 1 Apr 2021 14:02:57 -0700 Subject: [PATCH 16/24] Removing unnecessary lines --- samcli/lib/build/app_builder.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index 345fbfe97a..0164ef9d59 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -441,7 +441,6 @@ def _build_layer( # Only set to this value if specified workflow is makefile # which will result in config language as provided build_runtime = compatible_runtimes[0] - image = None # None key represents the global build image for all functions/layers global_image = self._build_images.get(None) image = self._build_images.get(layer_name, global_image) @@ -524,7 +523,6 @@ def _build_function( # pylint: disable=R1710 options = ApplicationBuilder._get_build_options(function_name, config.language, handler) # By default prefer to build in-process for speed if self._container_manager: - image = None # None represents the global build image for all functions/layers global_image = self._build_images.get(None) image = self._build_images.get(function_name, global_image) From 694d5ab7cd87047e3774d025415c1cc62cead4c5 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Thu, 1 Apr 2021 14:20:53 -0700 Subject: [PATCH 17/24] Refactoring integration test --- tests/integration/buildcmd/test_build_cmd.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tests/integration/buildcmd/test_build_cmd.py b/tests/integration/buildcmd/test_build_cmd.py index 828ba6f7fe..714ce44934 100644 --- a/tests/integration/buildcmd/test_build_cmd.py +++ b/tests/integration/buildcmd/test_build_cmd.py @@ -1860,17 +1860,8 @@ def test_custom_build_image_succeeds(self, use_container, build_image): cmdlist = self.get_command_list(use_container=use_container, build_image=build_image) - LOG.info("Running Command: {}".format(cmdlist)) - process = Popen(cmdlist, cwd=self.working_dir, stdout=PIPE, stderr=PIPE) - try: - stdout, stderr = process.communicate(timeout=TIMEOUT) - LOG.info(f"Stdout: {stdout.decode('utf-8')}") - LOG.info(f"Stderr: {stderr.decode('utf-8')}") - except TimeoutExpired: - LOG.error(f"Command: {command_list}, TIMED OUT") - LOG.error(f"Return Code: {process_execute.returncode}") - process.kill() - raise + command_result = run_command(cmdlist, cwd=self.working_dir) + stderr = command_result.stderr process_stderr = stderr.strip() self._verify_right_image_pulled(build_image, process_stderr) From 54a5da25a2b3c0a3dac4e47dff7b9bbfdfedd5d9 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Thu, 1 Apr 2021 14:26:00 -0700 Subject: [PATCH 18/24] Shortening error message --- samcli/commands/build/click_container.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/samcli/commands/build/click_container.py b/samcli/commands/build/click_container.py index e4326117f3..ff359846ce 100644 --- a/samcli/commands/build/click_container.py +++ b/samcli/commands/build/click_container.py @@ -12,9 +12,7 @@ class ContainerOptions(click.Option): def handle_parse_result(self, ctx, opts, args): if "use_container" not in opts and opts.get(self.name) is not None: msg = f"""\ -Missing required parameter, with --{self.name.replace("_", "-")} set. - -Must provide the --use-container flag in order to use --{self.name.replace("_", "-")} flag.""" +Missing required parameter, must provide the --use-container flag in order to use --{self.name.replace("_", "-")} flag.""" raise click.UsageError(msg) # To make sure no unser input prompting happens self.prompt = None From fee15aea4a3e4b87dba8df5491da3e6b843deb9b Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Thu, 1 Apr 2021 15:34:24 -0700 Subject: [PATCH 19/24] Adjusting test behaviour --- samcli/commands/build/click_container.py | 6 +++--- tests/unit/commands/samconfig/test_samconfig.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/samcli/commands/build/click_container.py b/samcli/commands/build/click_container.py index ff359846ce..b6e27e7ffb 100644 --- a/samcli/commands/build/click_container.py +++ b/samcli/commands/build/click_container.py @@ -10,9 +10,9 @@ class ContainerOptions(click.Option): """ def handle_parse_result(self, ctx, opts, args): - if "use_container" not in opts and opts.get(self.name) is not None: - msg = f"""\ -Missing required parameter, must provide the --use-container flag in order to use --{self.name.replace("_", "-")} flag.""" + if "use_container" not in opts and self.name in opts: + opt_name = self.name.replace("_", "-") + msg = f"Missing required parameter, need the --use-container flag in order to use --{opt_name} flag." raise click.UsageError(msg) # To make sure no unser input prompting happens self.prompt = None diff --git a/tests/unit/commands/samconfig/test_samconfig.py b/tests/unit/commands/samconfig/test_samconfig.py index 85725eebb1..369156f014 100644 --- a/tests/unit/commands/samconfig/test_samconfig.py +++ b/tests/unit/commands/samconfig/test_samconfig.py @@ -125,7 +125,7 @@ def test_build(self, do_cli_mock): LOG.debug(Path(config_path).read_text()) runner = CliRunner() - result = runner.invoke(cli, ["--use-container"]) + result = runner.invoke(cli, []) LOG.info(result.output) LOG.info(result.exception) @@ -177,7 +177,7 @@ def test_build_with_container_env_vars(self, do_cli_mock): LOG.debug(Path(config_path).read_text()) runner = CliRunner() - result = runner.invoke(cli, ["--use-container"]) + result = runner.invoke(cli, []) LOG.info(result.output) LOG.info(result.exception) @@ -228,7 +228,7 @@ def test_build_with_build_images(self, do_cli_mock): LOG.debug(Path(config_path).read_text()) runner = CliRunner() - result = runner.invoke(cli, ["--use-container"]) + result = runner.invoke(cli, []) LOG.info(result.output) LOG.info(result.exception) From e99649d94557b10341f4ff89a04830231836cf77 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Mon, 5 Apr 2021 01:39:18 -0700 Subject: [PATCH 20/24] Added new unit tests --- tests/unit/commands/buildcmd/test_command.py | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/unit/commands/buildcmd/test_command.py b/tests/unit/commands/buildcmd/test_command.py index 773bab6743..4649061373 100644 --- a/tests/unit/commands/buildcmd/test_command.py +++ b/tests/unit/commands/buildcmd/test_command.py @@ -15,6 +15,7 @@ ) from samcli.lib.build.workflow_config import UnsupportedRuntimeException from samcli.local.lambdafn.exceptions import FunctionNotFound +from samcli.commands.build.click_container import ContainerOptions class DeepWrap(Exception): @@ -284,3 +285,29 @@ def test_two_functions_with_default_image(self): ["Function1=image1", "Function2=image2", "image3"], {"Function1": "image1", "Function2": "image2", None: "image3"}, ) + + +@patch("samcli.commands.build.click_container.ContainerOptions") +class TestContainerOptionsSucceeds(TestCase): + ctx_mock = Mock() + opts = {"container_env_var": ["hi=in"], "use_container": True, "resource_logical_id": None} + ContainerOptionsMock = Mock() + ContainerOptionsMock.handle_parse_result.return_value = "value" + + def test_container_options(self, ContainerOptionsMock): + self.assertEqual(self.ContainerOptionsMock.handle_parse_result(self.ctx_mock, self.opts, []), "value") + + +class TestContainerOptionsFails(TestCase): + ctx_mock = Mock() + opts = {"container_env_var": ["hi=in"], "resource_logical_id": None} + args = ["--container-env-var"] + container_opt = ContainerOptions(args) + + def test_container_options_failure(self): + with self.assertRaises(click.UsageError) as err: + self.container_opt.handle_parse_result(self.ctx_mock, self.opts, []) + self.assertEqual( + str(err.exception), + "Missing required parameter, need the --use-container flag in order to use --container-env-var flag.", + ) From 54013883bd830dcf2db2737bec300bd54c2d7df3 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Mon, 5 Apr 2021 12:07:37 -0700 Subject: [PATCH 21/24] Test refactoring --- tests/unit/commands/buildcmd/test_command.py | 26 -------------------- 1 file changed, 26 deletions(-) diff --git a/tests/unit/commands/buildcmd/test_command.py b/tests/unit/commands/buildcmd/test_command.py index 4649061373..0ea0c55d6b 100644 --- a/tests/unit/commands/buildcmd/test_command.py +++ b/tests/unit/commands/buildcmd/test_command.py @@ -285,29 +285,3 @@ def test_two_functions_with_default_image(self): ["Function1=image1", "Function2=image2", "image3"], {"Function1": "image1", "Function2": "image2", None: "image3"}, ) - - -@patch("samcli.commands.build.click_container.ContainerOptions") -class TestContainerOptionsSucceeds(TestCase): - ctx_mock = Mock() - opts = {"container_env_var": ["hi=in"], "use_container": True, "resource_logical_id": None} - ContainerOptionsMock = Mock() - ContainerOptionsMock.handle_parse_result.return_value = "value" - - def test_container_options(self, ContainerOptionsMock): - self.assertEqual(self.ContainerOptionsMock.handle_parse_result(self.ctx_mock, self.opts, []), "value") - - -class TestContainerOptionsFails(TestCase): - ctx_mock = Mock() - opts = {"container_env_var": ["hi=in"], "resource_logical_id": None} - args = ["--container-env-var"] - container_opt = ContainerOptions(args) - - def test_container_options_failure(self): - with self.assertRaises(click.UsageError) as err: - self.container_opt.handle_parse_result(self.ctx_mock, self.opts, []) - self.assertEqual( - str(err.exception), - "Missing required parameter, need the --use-container flag in order to use --container-env-var flag.", - ) From 0a8d29a85bf5d6e3134398e85c3884ad305198e8 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Mon, 5 Apr 2021 12:09:41 -0700 Subject: [PATCH 22/24] Added new test class --- .../buildcmd/test_container_options.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/unit/commands/buildcmd/test_container_options.py diff --git a/tests/unit/commands/buildcmd/test_container_options.py b/tests/unit/commands/buildcmd/test_container_options.py new file mode 100644 index 0000000000..e96c5ac1c4 --- /dev/null +++ b/tests/unit/commands/buildcmd/test_container_options.py @@ -0,0 +1,31 @@ +import click + +from unittest import TestCase +from unittest.mock import Mock, patch, call +from samcli.commands.build.click_container import ContainerOptions + + +@patch("samcli.commands.build.click_container.ContainerOptions") +class TestContainerOptionsSucceeds(TestCase): + ctx_mock = Mock() + opts = {"container_env_var": ["hi=in"], "use_container": True, "resource_logical_id": None} + ContainerOptionsMock = Mock() + ContainerOptionsMock.handle_parse_result.return_value = "value" + + def test_container_options(self, ContainerOptionsMock): + self.assertEqual(self.ContainerOptionsMock.handle_parse_result(self.ctx_mock, self.opts, []), "value") + + +class TestContainerOptionsFails(TestCase): + ctx_mock = Mock() + opts = {"container_env_var": ["hi=in"], "resource_logical_id": None} + args = ["--container-env-var"] + container_opt = ContainerOptions(args) + + def test_container_options_failure(self): + with self.assertRaises(click.UsageError) as err: + self.container_opt.handle_parse_result(self.ctx_mock, self.opts, []) + self.assertEqual( + str(err.exception), + "Missing required parameter, need the --use-container flag in order to use --container-env-var flag.", + ) From cf62ccf94205215d502c26429301c86f03dfa330 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Mon, 5 Apr 2021 12:11:05 -0700 Subject: [PATCH 23/24] Removed unused import --- tests/unit/commands/buildcmd/test_command.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/commands/buildcmd/test_command.py b/tests/unit/commands/buildcmd/test_command.py index 0ea0c55d6b..773bab6743 100644 --- a/tests/unit/commands/buildcmd/test_command.py +++ b/tests/unit/commands/buildcmd/test_command.py @@ -15,7 +15,6 @@ ) from samcli.lib.build.workflow_config import UnsupportedRuntimeException from samcli.local.lambdafn.exceptions import FunctionNotFound -from samcli.commands.build.click_container import ContainerOptions class DeepWrap(Exception): From e5499150880e8238175ec68d4791e2db36c20268 Mon Sep 17 00:00:00 2001 From: Qingchuan Ma Date: Mon, 5 Apr 2021 12:14:48 -0700 Subject: [PATCH 24/24] Improving help text --- samcli/commands/build/command.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index 8edb1e2b05..32988b1d9d 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -134,8 +134,8 @@ default=None, multiple=True, # Can pass in multiple build images required=False, - help="Container image URIs for building functions. " - "You can specify for all functions with just the image URI " + help="Container image URIs for building functions/layers. " + "You can specify for all functions/layers with just the image URI " "(--build-image public.ecr.aws/sam/build-nodejs14.x:latest). " "You can specify for each individual function with " "(--build-image FunctionLogicalID=public.ecr.aws/sam/build-nodejs14.x:latest). "