From 64f478f6b4c4ef3482222bbd5b1588402cab5635 Mon Sep 17 00:00:00 2001 From: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Date: Fri, 8 Jul 2022 11:54:03 -0700 Subject: [PATCH] feat: Esbuild Generic Command Builder (#372) * Cleanup esbuild actions, remove external node action * Update existing tests * Black reformat * Add type hints and docstrings * Add unit tests * Remove unused import * Update variable naming, add docstring, add format option --- .../workflows/nodejs_npm_esbuild/actions.py | 194 ++++------------ .../esbuild-plugin.js.template | 19 -- .../workflows/nodejs_npm_esbuild/esbuild.py | 216 +++++++++++++++++- .../nodejs_npm_esbuild/exceptions.py | 21 ++ .../workflows/nodejs_npm_esbuild/node.py | 104 --------- .../workflows/nodejs_npm_esbuild/workflow.py | 15 +- .../nodejs_npm_esbuild/test_actions.py | 178 +++++++-------- .../nodejs_npm_esbuild/test_esbuild.py | 168 +++++++++++++- .../workflows/nodejs_npm_esbuild/test_node.py | 77 ------- .../nodejs_npm_esbuild/test_workflow.py | 4 +- 10 files changed, 528 insertions(+), 468 deletions(-) delete mode 100644 aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild-plugin.js.template create mode 100644 aws_lambda_builders/workflows/nodejs_npm_esbuild/exceptions.py delete mode 100644 aws_lambda_builders/workflows/nodejs_npm_esbuild/node.py delete mode 100644 tests/unit/workflows/nodejs_npm_esbuild/test_node.py diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py index 76e104193..810b242dc 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py @@ -2,15 +2,17 @@ Actions specific to the esbuild bundler """ import logging -from tempfile import NamedTemporaryFile - -from pathlib import Path +from typing import Any, Dict from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError -from .esbuild import EsbuildExecutionError +from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils +from aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild import EsbuildCommandBuilder, SubprocessEsbuild +from aws_lambda_builders.workflows.nodejs_npm_esbuild.exceptions import EsbuildExecutionError LOG = logging.getLogger(__name__) +EXTERNAL_KEY = "external" + class EsbuildBundleAction(BaseAction): @@ -23,16 +25,14 @@ class EsbuildBundleAction(BaseAction): DESCRIPTION = "Packaging source using Esbuild" PURPOSE = Purpose.COPY_SOURCE - ENTRY_POINTS = "entry_points" - def __init__( self, - scratch_dir, - artifacts_dir, - bundler_config, - osutils, - subprocess_esbuild, - subprocess_nodejs=None, + scratch_dir: str, + artifacts_dir: str, + bundler_config: Dict[str, Any], + osutils: OSUtils, + subprocess_esbuild: SubprocessEsbuild, + manifest: str, skip_deps=False, ): """ @@ -49,168 +49,58 @@ def __init__( :type subprocess_esbuild: aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild.SubprocessEsbuild :param subprocess_esbuild: An instance of the Esbuild process wrapper - :type subprocess_nodejs: aws_lambda_builders.workflows.nodejs_npm_esbuild.node.SubprocessNodejs - :param subprocess_nodejs: An instance of the nodejs process wrapper - :type skip_deps: bool :param skip_deps: if dependencies should be omitted from bundling :type bundler_config: Dict[str,Any] :param bundler_config: the bundler configuration + + :type manifest: str + :param manifest: path to package.json file contents to read """ super(EsbuildBundleAction, self).__init__() - self.scratch_dir = scratch_dir - self.artifacts_dir = artifacts_dir - self.bundler_config = bundler_config - self.osutils = osutils - self.subprocess_esbuild = subprocess_esbuild - self.skip_deps = skip_deps - self.subprocess_nodejs = subprocess_nodejs + self._scratch_dir = scratch_dir + self._artifacts_dir = artifacts_dir + self._bundler_config = bundler_config + self._osutils = osutils + self._subprocess_esbuild = subprocess_esbuild + self._skip_deps = skip_deps + self._manifest = manifest - def execute(self): + def execute(self) -> None: """ Runs the action. :raises lambda_builders.actions.ActionFailedError: when esbuild packaging fails """ + esbuild_command = EsbuildCommandBuilder( + self._scratch_dir, self._artifacts_dir, self._bundler_config, self._osutils, self._manifest + ) - explicit_entry_points = self._construct_esbuild_entry_points() - - args = explicit_entry_points + ["--bundle", "--platform=node", "--format=cjs"] - minify = self.bundler_config.get("minify", True) - sourcemap = self.bundler_config.get("sourcemap", True) - target = self.bundler_config.get("target", "es2020") - external = self.bundler_config.get("external", []) - loader = self.bundler_config.get("loader", []) - if minify: - args.append("--minify") - if sourcemap: - args.append("--sourcemap") - if external: - args.extend(map(lambda x: f"--external:{x}", external)) - if loader: - args.extend(map(lambda x: f"--loader:{x}", loader)) - - args.append("--target={}".format(target)) - args.append("--outdir={}".format(self.artifacts_dir)) - - if self.skip_deps: - LOG.info("Running custom esbuild using Node.js") - # Don't pass externals because the esbuild.js template makes everything external - script = EsbuildBundleAction._get_node_esbuild_template( - explicit_entry_points, target, self.artifacts_dir, minify, sourcemap - ) - self._run_external_esbuild_in_nodejs(script) - return + if self._should_bundle_deps_externally(): + esbuild_command.build_with_no_dependencies() + if EXTERNAL_KEY in self._bundler_config: + # Already marking everything as external, + # shouldn't attempt to do it again when building args from config + self._bundler_config.pop(EXTERNAL_KEY) + + args = ( + esbuild_command.build_entry_points().build_default_values().build_esbuild_args_from_config().get_command() + ) try: - self.subprocess_esbuild.run(args, cwd=self.scratch_dir) + self._subprocess_esbuild.run(args, cwd=self._scratch_dir) except EsbuildExecutionError as ex: raise ActionFailedError(str(ex)) - def _run_external_esbuild_in_nodejs(self, script): - """ - Run esbuild in a separate process through Node.js - Workaround for https://github.com/evanw/esbuild/issues/1958 - - :type script: str - :param script: Node.js script to execute - - :raises lambda_builders.actions.ActionFailedError: when esbuild packaging fails - """ - with NamedTemporaryFile(dir=self.scratch_dir, mode="w") as tmp: - tmp.write(script) - tmp.flush() - try: - self.subprocess_nodejs.run([tmp.name], cwd=self.scratch_dir) - except EsbuildExecutionError as ex: - raise ActionFailedError(str(ex)) - - def _construct_esbuild_entry_points(self): - """ - Construct the list of explicit entry points - """ - if self.ENTRY_POINTS not in self.bundler_config: - raise ActionFailedError(f"{self.ENTRY_POINTS} not set ({self.bundler_config})") - - entry_points = self.bundler_config[self.ENTRY_POINTS] - - if not isinstance(entry_points, list): - raise ActionFailedError(f"{self.ENTRY_POINTS} must be a list ({self.bundler_config})") - - if not entry_points: - raise ActionFailedError(f"{self.ENTRY_POINTS} must not be empty ({self.bundler_config})") - - entry_paths = [self.osutils.joinpath(self.scratch_dir, entry_point) for entry_point in entry_points] - - LOG.debug("NODEJS building %s using esbuild to %s", entry_paths, self.artifacts_dir) - - explicit_entry_points = [] - for entry_path, entry_point in zip(entry_paths, entry_points): - explicit_entry_points.append(self._get_explicit_file_type(entry_point, entry_path)) - return explicit_entry_points - - @staticmethod - def _get_node_esbuild_template(entry_points, target, out_dir, minify, sourcemap): + def _should_bundle_deps_externally(self) -> bool: """ - Get the esbuild nodejs plugin template - - :type entry_points: List[str] - :param entry_points: list of entry points - - :type target: str - :param target: target version - - :type out_dir: str - :param out_dir: output directory to bundle into - - :type minify: bool - :param minify: if bundled code should be minified - - :type sourcemap: bool - :param sourcemap: if esbuild should produce a sourcemap + Checks if all dependencies should be marked as external and not bundled with source code - :rtype: str - :return: formatted template + :rtype: boolean + :return: True if all dependencies should be marked as external """ - curr_dir = Path(__file__).resolve().parent - with open(str(Path(curr_dir, "esbuild-plugin.js.template")), "r") as f: - input_str = f.read() - result = input_str.format( - target=target, - minify="true" if minify else "false", - sourcemap="true" if sourcemap else "false", - out_dir=repr(out_dir), - entry_points=entry_points, - ) - return result - - def _get_explicit_file_type(self, entry_point, entry_path): - """ - Get an entry point with an explicit .ts or .js suffix. - - :type entry_point: str - :param entry_point: path to entry file from code uri - - :type entry_path: str - :param entry_path: full path of entry file - - :rtype: str - :return: entry point with appropriate file extension - - :raises lambda_builders.actions.ActionFailedError: when esbuild packaging fails - """ - if Path(entry_point).suffix: - if self.osutils.file_exists(entry_path): - return entry_point - raise ActionFailedError("entry point {} does not exist".format(entry_path)) - - for ext in [".ts", ".js"]: - entry_path_with_ext = entry_path + ext - if self.osutils.file_exists(entry_path_with_ext): - return entry_point + ext - - raise ActionFailedError("entry point {} does not exist".format(entry_path)) + return self._skip_deps or "./node_modules/*" in self._bundler_config.get(EXTERNAL_KEY, []) class EsbuildCheckVersionAction(BaseAction): diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild-plugin.js.template b/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild-plugin.js.template deleted file mode 100644 index f3d2beb99..000000000 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild-plugin.js.template +++ /dev/null @@ -1,19 +0,0 @@ -let skipBundleNodeModules = {{ - name: 'make-all-packages-external', - setup(build) {{ - let filter = /^[^.\/]|^\.[^.\/]|^\.\.[^\/]/ // Must not start with "/" or "./" or "../" - build.onResolve({{ filter }}, args => ({{ path: args.path, external: true }})) - }}, -}} - -require('esbuild').build({{ - entryPoints: {entry_points}, - bundle: true, - platform: 'node', - format: 'cjs', - target: '{target}', - sourcemap: {sourcemap}, - outdir: {out_dir}, - minify: {minify}, - plugins: [skipBundleNodeModules], -}}).catch(() => process.exit(1)) diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild.py index d1bf92b25..9f569abb3 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild.py +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild.py @@ -1,26 +1,19 @@ """ Wrapper around calling esbuild through a subprocess. """ +from pathlib import Path import logging +from typing import Dict, Any, List -from aws_lambda_builders.exceptions import LambdaBuilderError +from aws_lambda_builders.actions import ActionFailedError +from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils +from aws_lambda_builders.workflows.nodejs_npm_esbuild.exceptions import EsbuildCommandError, EsbuildExecutionError LOG = logging.getLogger(__name__) -class EsbuildExecutionError(LambdaBuilderError): - - """ - Exception raised in case esbuild execution fails. - It will pass on the standard error output from the esbuild console. - """ - - MESSAGE = "Esbuild Failed: {message}" - - class SubprocessEsbuild(object): - """ Wrapper around the Esbuild command line utility, making it easy to consume execution results. @@ -102,3 +95,202 @@ def run(self, args, cwd=None): raise EsbuildExecutionError(message=err.decode("utf8").strip()) return out.decode("utf8").strip() + + +# The esbuild API flags are broken up into three forms (https://esbuild.github.io/api/): +# Boolean types (--minify) +SUPPORTED_ESBUILD_APIS_BOOLEAN = [ + "minify", + "sourcemap", +] + +# single value types (--target=es2020) +SUPPORTED_ESBUILD_APIS_SINGLE_VALUE = [ + "target", + "format", +] + +# Multi-value types (--external:axios --external:aws-sdk) +SUPPORTED_ESBUILD_APIS_MULTI_VALUE = [ + "external", + "loader", +] + + +class EsbuildCommandBuilder: + ENTRY_POINTS = "entry_points" + + def __init__( + self, scratch_dir: str, artifacts_dir: str, bundler_config: Dict[Any, Any], osutils: OSUtils, manifest: str + ): + self._scratch_dir = scratch_dir + self._artifacts_dir = artifacts_dir + self._bundler_config = bundler_config + self._osutils = osutils + self._manifest = manifest + self._command: List[str] = [] + + def get_command(self) -> List[str]: + """ + Get all of the commands flags created by the command builder + + :rtype: List[str] + :return: List of esbuild commands to be executed + """ + return self._command + + def build_esbuild_args_from_config(self) -> "EsbuildCommandBuilder": + """ + Build arguments configured in the command config (e.g. template.yaml) + + :rtype: EsbuildCommandBuilder + :return: An instance of the command builder + """ + args = [] + + args.extend(self._get_boolean_args()) + args.extend(self._get_single_value_args()) + args.extend(self._get_multi_value_args()) + + LOG.debug("Found the following args in the config: %s", str(args)) + + self._command.extend(args) + return self + + def build_entry_points(self) -> "EsbuildCommandBuilder": + """ + Build the entry points to the command + + :rtype: EsbuildCommandBuilder + :return: An instance of the command builder + """ + if self.ENTRY_POINTS not in self._bundler_config: + raise EsbuildCommandError(f"{self.ENTRY_POINTS} not set ({self._bundler_config})") + + entry_points = self._bundler_config[self.ENTRY_POINTS] + + if not isinstance(entry_points, list): + raise EsbuildCommandError(f"{self.ENTRY_POINTS} must be a list ({self._bundler_config})") + + if not entry_points: + raise EsbuildCommandError(f"{self.ENTRY_POINTS} must not be empty ({self._bundler_config})") + + entry_paths = [self._osutils.joinpath(self._scratch_dir, entry_point) for entry_point in entry_points] + + LOG.debug("NODEJS building %s using esbuild to %s", entry_paths, self._artifacts_dir) + + for entry_path, entry_point in zip(entry_paths, entry_points): + self._command.append(self._get_explicit_file_type(entry_point, entry_path)) + + return self + + def build_default_values(self) -> "EsbuildCommandBuilder": + """ + Build the default values that each call to esbuild should contain + + :rtype: EsbuildCommandBuilder + :return: An instance of the command builder + """ + args = ["--bundle", "--platform=node", "--outdir={}".format(self._artifacts_dir)] + + if "target" not in self._bundler_config: + args.append("--target=es2020") + + if "format" not in self._bundler_config: + args.append("--format=cjs") + + if "minify" not in self._bundler_config: + args.append("--minify") + + if "sourcemap" not in self._bundler_config: + args.append("--sourcemap") + + LOG.debug("Using the following default args: %s", str(args)) + + self._command.extend(args) + return self + + def build_with_no_dependencies(self) -> "EsbuildCommandBuilder": + """ + Set all dependencies located in the package.json to + external so as to not bundle them with the source code + + :rtype: EsbuildCommandBuilder + :return: An instance of the command builder + """ + package = self._osutils.parse_json(self._manifest) + dependencies = package.get("dependencies", {}).keys() + args = ["--external:{}".format(dep) for dep in dependencies] + self._command.extend(args) + return self + + def _get_boolean_args(self) -> List[str]: + """ + Get a list of all the boolean value flag types (e.g. --minify) + + :rtype: List[str] + :return: Arguments to be appended to the command list + """ + args = [] + for param in SUPPORTED_ESBUILD_APIS_BOOLEAN: + if param in self._bundler_config and self._bundler_config[param] is True: + args.append(f"--{param}") + return args + + def _get_single_value_args(self) -> List[str]: + """ + Get a list of all the single value flag types (e.g. --target=es2020) + + :rtype: List[str] + :return: Arguments to be appended to the command list + """ + args = [] + for param in SUPPORTED_ESBUILD_APIS_SINGLE_VALUE: + if param in self._bundler_config: + value = self._bundler_config.get(param) + args.append(f"--{param}={value}") + return args + + def _get_multi_value_args(self) -> List[str]: + """ + Get a list of all the multi-value flag types (e.g. --external:aws-sdk) + + :rtype: List[str] + :return: Arguments to be appended to the command list + """ + args = [] + for param in SUPPORTED_ESBUILD_APIS_MULTI_VALUE: + if param in self._bundler_config: + values = self._bundler_config.get(param) + if not isinstance(values, list): + raise EsbuildCommandError(f"Invalid type for property {param}, must be a dict.") + for param_item in values: + args.append(f"--{param}:{param_item}") + return args + + def _get_explicit_file_type(self, entry_point, entry_path): + """ + Get an entry point with an explicit .ts or .js suffix. + + :type entry_point: str + :param entry_point: path to entry file from code uri + + :type entry_path: str + :param entry_path: full path of entry file + + :rtype: str + :return: entry point with appropriate file extension + + :raises lambda_builders.actions.ActionFailedError: when esbuild packaging fails + """ + if Path(entry_point).suffix: + if self._osutils.file_exists(entry_path): + return entry_point + raise ActionFailedError("entry point {} does not exist".format(entry_path)) + + for ext in [".ts", ".js"]: + entry_path_with_ext = entry_path + ext + if self._osutils.file_exists(entry_path_with_ext): + return entry_point + ext + + raise ActionFailedError("entry point {} does not exist".format(entry_path)) diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/exceptions.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/exceptions.py new file mode 100644 index 000000000..56bbf1f4a --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/exceptions.py @@ -0,0 +1,21 @@ +""" +Esbuild specific exceptions +""" +from aws_lambda_builders.actions import ActionFailedError +from aws_lambda_builders.exceptions import LambdaBuilderError + + +class EsbuildExecutionError(LambdaBuilderError): + """ + Exception raised in case esbuild execution fails. + It will pass on the standard error output from the esbuild console. + """ + + MESSAGE = "Esbuild Failed: {message}" + + +class EsbuildCommandError(ActionFailedError): + """ + Exception raised in case esbuild can't build a valid esbuild command from the given config. + It will pass on the standard error output from the esbuild console. + """ diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/node.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/node.py deleted file mode 100644 index 6a50ea495..000000000 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/node.py +++ /dev/null @@ -1,104 +0,0 @@ -""" -Wrapper around calling nodejs through a subprocess. -""" - -import logging - -from aws_lambda_builders.exceptions import LambdaBuilderError - -LOG = logging.getLogger(__name__) - - -class NodejsExecutionError(LambdaBuilderError): - - """ - Exception raised in case nodejs execution fails. - It will pass on the standard error output from the Node.js console. - """ - - MESSAGE = "Nodejs Failed: {message}" - - -class SubprocessNodejs(object): - - """ - Wrapper around the nodejs command line utility, making it - easy to consume execution results. - """ - - def __init__(self, osutils, executable_search_paths, which): - """ - :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils - :param osutils: An instance of OS Utilities for file manipulation - - :type executable_search_paths: list - :param executable_search_paths: List of paths to the node package binary utilities. This will - be used to find embedded Nodejs at runtime if present in the package - - :type which: aws_lambda_builders.utils.which - :param which: Function to get paths which conform to the given mode on the PATH - with the prepended additional search paths - """ - self.osutils = osutils - self.executable_search_paths = executable_search_paths - self.which = which - - def nodejs_binary(self): - """ - Finds the Nodejs binary at runtime. - - The utility may be present as a package dependency of the Lambda project, - or in the global path. If there is one in the Lambda project, it should - be preferred over a global utility. The check has to be executed - at runtime, since nodejs dependencies will be installed by the workflow - using one of the previous actions. - """ - - LOG.debug("checking for nodejs in: %s", self.executable_search_paths) - binaries = self.which("node", executable_search_paths=self.executable_search_paths) - LOG.debug("potential nodejs binaries: %s", binaries) - - if binaries: - return binaries[0] - else: - raise NodejsExecutionError(message="cannot find nodejs") - - def run(self, args, cwd=None): - - """ - Runs the action. - - :type args: list - :param args: Command line arguments to pass to Nodejs - - :type cwd: str - :param cwd: Directory where to execute the command (defaults to current dir) - - :rtype: str - :return: text of the standard output from the command - - :raises aws_lambda_builders.workflows.nodejs_npm.npm.NodejsExecutionError: - when the command executes with a non-zero return code. The exception will - contain the text of the standard error output from the command. - - :raises ValueError: if arguments are not provided, or not a list - """ - - if not isinstance(args, list): - raise ValueError("args must be a list") - - if not args: - raise ValueError("requires at least one arg") - - invoke_nodejs = [self.nodejs_binary()] + args - - LOG.debug("executing Nodejs: %s", invoke_nodejs) - - p = self.osutils.popen(invoke_nodejs, stdout=self.osutils.pipe, stderr=self.osutils.pipe, cwd=cwd) - - out, err = p.communicate() - - if p.returncode != 0: - raise NodejsExecutionError(message=err.decode("utf8").strip()) - - return out.decode("utf8").strip() diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py index fcd5de062..d5cdbb633 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py @@ -19,7 +19,6 @@ EsbuildBundleAction, EsbuildCheckVersionAction, ) -from .node import SubprocessNodejs from .utils import is_experimental_esbuild_scope from .esbuild import SubprocessEsbuild, EsbuildExecutionError from ..nodejs_npm import NodejsNpmWorkflow @@ -61,7 +60,11 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim if not osutils.file_exists(manifest_path): LOG.warning("package.json file not found. Bundling source without dependencies.") - self.actions = [EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild)] + self.actions = [ + EsbuildBundleAction( + source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild, self.manifest_path + ) + ] return if not is_experimental_esbuild_scope(self.experimental_flags): @@ -105,8 +108,6 @@ def actions_with_bundler( CopySourceAction(source_dir, scratch_dir, excludes=self.EXCLUDED_FILES + tuple(["node_modules"])) ] - subprocess_node = SubprocessNodejs(osutils, self.executable_search_paths, which=which) - # Bundle dependencies separately in a dependency layer. We need to check the esbuild # version here to ensure that it supports skipping dependency bundling esbuild_no_deps = [ @@ -117,11 +118,13 @@ def actions_with_bundler( bundler_config, osutils, subprocess_esbuild, - subprocess_node, + self.manifest_path, skip_deps=True, ), ] - esbuild_with_deps = EsbuildBundleAction(scratch_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) + esbuild_with_deps = EsbuildBundleAction( + scratch_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild, self.manifest_path + ) install_action = NodejsNpmWorkflow.get_install_action( source_dir, scratch_dir, subprocess_npm, osutils, self.options, is_production=False diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py b/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py index c6751a6e4..fd0a35877 100644 --- a/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py @@ -1,5 +1,3 @@ -import tempfile -from pathlib import Path from unittest import TestCase from unittest.mock import Mock @@ -13,16 +11,16 @@ class TestEsbuildBundleAction(TestCase): @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") @patch("aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild.SubprocessEsbuild") - @patch("aws_lambda_builders.workflows.nodejs_npm_esbuild.node.SubprocessNodejs") - def setUp(self, OSUtilMock, SubprocessEsbuildMock, SubprocessNodejsMock): + def setUp(self, OSUtilMock, SubprocessEsbuildMock): self.osutils = OSUtilMock.return_value self.subprocess_esbuild = SubprocessEsbuildMock.return_value - self.subprocess_nodejs = SubprocessNodejsMock.return_value self.osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) self.osutils.file_exists.side_effect = [True, True] def test_raises_error_if_entrypoints_not_specified(self): - action = EsbuildBundleAction("source", "artifacts", {"config": "param"}, self.osutils, self.subprocess_esbuild) + action = EsbuildBundleAction( + "source", "artifacts", {"config": "param"}, self.osutils, self.subprocess_esbuild, "package.json" + ) with self.assertRaises(ActionFailedError) as raised: action.execute() @@ -30,7 +28,12 @@ def test_raises_error_if_entrypoints_not_specified(self): def test_raises_error_if_entrypoints_not_a_list(self): action = EsbuildBundleAction( - "source", "artifacts", {"config": "param", "entry_points": "abc"}, self.osutils, self.subprocess_esbuild + "source", + "artifacts", + {"config": "param", "entry_points": "abc"}, + self.osutils, + self.subprocess_esbuild, + "package.json", ) with self.assertRaises(ActionFailedError) as raised: action.execute() @@ -41,7 +44,12 @@ def test_raises_error_if_entrypoints_not_a_list(self): def test_raises_error_if_entrypoints_empty_list(self): action = EsbuildBundleAction( - "source", "artifacts", {"config": "param", "entry_points": []}, self.osutils, self.subprocess_esbuild + "source", + "artifacts", + {"config": "param", "entry_points": []}, + self.osutils, + self.subprocess_esbuild, + "package.json", ) with self.assertRaises(ActionFailedError) as raised: action.execute() @@ -52,7 +60,7 @@ def test_raises_error_if_entrypoints_empty_list(self): def test_packages_javascript_with_minification_and_sourcemap(self): action = EsbuildBundleAction( - "source", "artifacts", {"entry_points": ["x.js"]}, self.osutils, self.subprocess_esbuild + "source", "artifacts", {"entry_points": ["x.js"]}, self.osutils, self.subprocess_esbuild, "package.json" ) action.execute() @@ -61,11 +69,11 @@ def test_packages_javascript_with_minification_and_sourcemap(self): "x.js", "--bundle", "--platform=node", + "--outdir=artifacts", + "--target=es2020", "--format=cjs", "--minify", "--sourcemap", - "--target=es2020", - "--outdir=artifacts", ], cwd="source", ) @@ -77,6 +85,7 @@ def test_packages_with_externals(self): {"entry_points": ["x.js"], "external": ["fetch", "aws-sdk"]}, self.osutils, self.subprocess_esbuild, + "", ) action.execute() self.subprocess_esbuild.run.assert_called_with( @@ -84,13 +93,13 @@ def test_packages_with_externals(self): "x.js", "--bundle", "--platform=node", + "--outdir=artifacts", + "--target=es2020", "--format=cjs", "--minify", "--sourcemap", "--external:fetch", "--external:aws-sdk", - "--target=es2020", - "--outdir=artifacts", ], cwd="source", ) @@ -102,6 +111,7 @@ def test_packages_with_custom_loaders(self): {"entry_points": ["x.js"], "loader": [".proto=text", ".json=js"]}, self.osutils, self.subprocess_esbuild, + "", ) action.execute() self.subprocess_esbuild.run.assert_called_with( @@ -109,13 +119,13 @@ def test_packages_with_custom_loaders(self): "x.js", "--bundle", "--platform=node", + "--outdir=artifacts", + "--target=es2020", "--format=cjs", "--minify", "--sourcemap", "--loader:.proto=text", "--loader:.json=js", - "--target=es2020", - "--outdir=artifacts", ], cwd="source", ) @@ -123,7 +133,7 @@ def test_packages_with_custom_loaders(self): def test_checks_if_single_entrypoint_exists(self): action = EsbuildBundleAction( - "source", "artifacts", {"entry_points": ["x.js"]}, self.osutils, self.subprocess_esbuild + "source", "artifacts", {"entry_points": ["x.js"]}, self.osutils, self.subprocess_esbuild, "package.json" ) self.osutils.file_exists.side_effect = [False] @@ -138,7 +148,12 @@ def test_checks_if_multiple_entrypoints_exist(self): self.osutils.file_exists.side_effect = [True, False] action = EsbuildBundleAction( - "source", "artifacts", {"entry_points": ["x.js", "y.js"]}, self.osutils, self.subprocess_esbuild + "source", + "artifacts", + {"entry_points": ["x.js", "y.js"]}, + self.osutils, + self.subprocess_esbuild, + "package.json", ) with self.assertRaises(ActionFailedError) as raised: @@ -152,7 +167,12 @@ def test_checks_if_multiple_entrypoints_exist(self): def test_excludes_sourcemap_if_requested(self): action = EsbuildBundleAction( - "source", "artifacts", {"entry_points": ["x.js"], "sourcemap": False}, self.osutils, self.subprocess_esbuild + "source", + "artifacts", + {"entry_points": ["x.js"], "sourcemap": False}, + self.osutils, + self.subprocess_esbuild, + "package.json", ) action.execute() self.subprocess_esbuild.run.assert_called_with( @@ -160,17 +180,22 @@ def test_excludes_sourcemap_if_requested(self): "x.js", "--bundle", "--platform=node", + "--outdir=artifacts", + "--target=es2020", "--format=cjs", "--minify", - "--target=es2020", - "--outdir=artifacts", ], cwd="source", ) def test_does_not_minify_if_requested(self): action = EsbuildBundleAction( - "source", "artifacts", {"entry_points": ["x.js"], "minify": False}, self.osutils, self.subprocess_esbuild + "source", + "artifacts", + {"entry_points": ["x.js"], "minify": False}, + self.osutils, + self.subprocess_esbuild, + "package.json", ) action.execute() self.subprocess_esbuild.run.assert_called_with( @@ -178,17 +203,22 @@ def test_does_not_minify_if_requested(self): "x.js", "--bundle", "--platform=node", + "--outdir=artifacts", + "--target=es2020", "--format=cjs", "--sourcemap", - "--target=es2020", - "--outdir=artifacts", ], cwd="source", ) def test_uses_specified_target(self): action = EsbuildBundleAction( - "source", "artifacts", {"entry_points": ["x.js"], "target": "node14"}, self.osutils, self.subprocess_esbuild + "source", + "artifacts", + {"entry_points": ["x.js"], "target": "node14"}, + self.osutils, + self.subprocess_esbuild, + "package.json", ) action.execute() self.subprocess_esbuild.run.assert_called_with( @@ -196,11 +226,11 @@ def test_uses_specified_target(self): "x.js", "--bundle", "--platform=node", + "--outdir=artifacts", "--format=cjs", "--minify", "--sourcemap", "--target=node14", - "--outdir=artifacts", ], cwd="source", ) @@ -212,6 +242,7 @@ def test_includes_multiple_entry_points_if_requested(self): {"entry_points": ["x.js", "y.js"], "target": "node14"}, self.osutils, self.subprocess_esbuild, + "package.json", ) action.execute() self.subprocess_esbuild.run.assert_called_with( @@ -220,91 +251,48 @@ def test_includes_multiple_entry_points_if_requested(self): "y.js", "--bundle", "--platform=node", + "--outdir=artifacts", "--format=cjs", "--minify", "--sourcemap", "--target=node14", - "--outdir=artifacts", ], cwd="source", ) - def test_runs_node_subprocess_if_deps_skipped(self): + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_includes_building_with_external_dependencies(self, osutils_mock): + osutils_mock.parse_json.return_value = { + "dependencies": {"@faker-js/faker": "7.1.0", "uuidv4": "^6.2.12", "axios": "0.0.0"} + } action = EsbuildBundleAction( - tempfile.mkdtemp(), + "source", "artifacts", - {"entry_points": ["app.ts"]}, - self.osutils, + {"entry_points": ["x.js", "y.js"], "target": "node14", "external": "./node_modules/*"}, + osutils_mock, self.subprocess_esbuild, - self.subprocess_nodejs, - True, + "package.json", ) action.execute() - self.subprocess_nodejs.run.assert_called() - - def test_reads_nodejs_bundle_template_file(self): - template = EsbuildBundleAction._get_node_esbuild_template(["app.ts"], "es2020", "outdir", False, True) - expected_template = """let skipBundleNodeModules = { - name: 'make-all-packages-external', - setup(build) { - let filter = /^[^.\/]|^\.[^.\/]|^\.\.[^\/]/ // Must not start with "/" or "./" or "../" - build.onResolve({ filter }, args => ({ path: args.path, external: true })) - }, -} - -require('esbuild').build({ - entryPoints: ['app.ts'], - bundle: true, - platform: 'node', - format: 'cjs', - target: 'es2020', - sourcemap: true, - outdir: 'outdir', - minify: false, - plugins: [skipBundleNodeModules], -}).catch(() => process.exit(1)) -""" - self.assertEqual(template, expected_template) - - -class TestImplicitFileTypeResolution(TestCase): - @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") - @patch("aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild.SubprocessEsbuild") - def setUp(self, OSUtilMock, SubprocessEsbuildMock): - self.osutils = OSUtilMock.return_value - self.subprocess_esbuild = SubprocessEsbuildMock.return_value - self.action = EsbuildBundleAction( - "source", - "artifacts", - {}, - self.osutils, - self.subprocess_esbuild, + self.assertNotIn("external", action._bundler_config) + self.subprocess_esbuild.run.assert_called_with( + [ + "--external:@faker-js/faker", + "--external:uuidv4", + "--external:axios", + "x.js", + "y.js", + "--bundle", + "--platform=node", + "--outdir=artifacts", + "--format=cjs", + "--minify", + "--sourcemap", + "--target=node14", + ], + cwd="source", ) - @parameterized.expand( - [ - ([True], "file.ts", "file.ts"), - ([False, True], "file", "file.js"), - ([True], "file", "file.ts"), - ] - ) - def test_implicit_and_explicit_file_types(self, file_exists, entry_point, expected): - self.osutils.file_exists.side_effect = file_exists - explicit_entry_point = self.action._get_explicit_file_type(entry_point, "") - self.assertEqual(expected, explicit_entry_point) - - @parameterized.expand( - [ - ([False], "file.ts"), - ([False, False], "file"), - ] - ) - def test_throws_exception_entry_point_not_found(self, file_exists, entry_point): - self.osutils.file_exists.side_effect = file_exists - with self.assertRaises(ActionFailedError) as context: - self.action._get_explicit_file_type(entry_point, "invalid") - self.assertEqual(str(context.exception), "entry point invalid does not exist") - class TestEsbuildVersionCheckerAction(TestCase): @parameterized.expand(["0.14.0", "0.0.0", "0.14.12"]) diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_esbuild.py b/tests/unit/workflows/nodejs_npm_esbuild/test_esbuild.py index 44f94387e..a402acd5f 100644 --- a/tests/unit/workflows/nodejs_npm_esbuild/test_esbuild.py +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_esbuild.py @@ -1,7 +1,13 @@ from unittest import TestCase from mock import patch +from parameterized import parameterized -from aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild import SubprocessEsbuild, EsbuildExecutionError +from aws_lambda_builders.actions import ActionFailedError +from aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild import ( + SubprocessEsbuild, + EsbuildExecutionError, + EsbuildCommandBuilder, +) class FakePopen: @@ -77,3 +83,163 @@ def test_raises_ValueError_if_args_empty(self): self.under_test.run([]) self.assertEqual(raised.exception.args[0], "requires at least one arg") + + +class TestImplicitFileTypeResolution(TestCase): + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + @patch("aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild.SubprocessEsbuild") + def setUp(self, OSUtilMock, SubprocessEsbuildMock): + self.osutils = OSUtilMock.return_value + self.subprocess_esbuild = SubprocessEsbuildMock.return_value + self.esbuild_command_builder = EsbuildCommandBuilder( + "source", + "artifacts", + {}, + self.osutils, + "package.json", + ) + + @parameterized.expand( + [ + ([True], "file.ts", "file.ts"), + ([False, True], "file", "file.js"), + ([True], "file", "file.ts"), + ] + ) + def test_implicit_and_explicit_file_types(self, file_exists, entry_point, expected): + self.osutils.file_exists.side_effect = file_exists + explicit_entry_point = self.esbuild_command_builder._get_explicit_file_type(entry_point, "") + self.assertEqual(expected, explicit_entry_point) + + @parameterized.expand( + [ + ([False], "file.ts"), + ([False, False], "file"), + ] + ) + def test_throws_exception_entry_point_not_found(self, file_exists, entry_point): + self.osutils.file_exists.side_effect = file_exists + with self.assertRaises(ActionFailedError) as context: + self.esbuild_command_builder._get_explicit_file_type(entry_point, "invalid") + self.assertEqual(str(context.exception), "entry point invalid does not exist") + + +class TestEsbuildCommandBuilder(TestCase): + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_builds_entry_points(self, osutils_mock): + bundler_config = {"entry_points": ["x.js", "y.ts"]} + args = ( + EsbuildCommandBuilder("scratch", "artifacts", bundler_config, osutils_mock, "") + .build_entry_points() + .get_command() + ) + self.assertEqual(args, ["x.js", "y.ts"]) + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_builds_default_values(self, osutils_mock): + bundler_config = {} + args = ( + EsbuildCommandBuilder("scratch", "artifacts", bundler_config, osutils_mock, "") + .build_default_values() + .get_command() + ) + self.assertEqual( + args, + [ + "--bundle", + "--platform=node", + "--outdir=artifacts", + "--target=es2020", + "--format=cjs", + "--minify", + "--sourcemap", + ], + ) + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_combined_builder_exclude_all_dependencies(self, osutils_mock): + bundler_config = {"entry_points": ["x.js"], "loader": [".proto=text", ".json=js"]} + osutils_mock.parse_json.return_value = { + "dependencies": {"@faker-js/faker": "7.1.0", "uuidv4": "^6.2.12", "axios": "0.0.0"} + } + args = ( + EsbuildCommandBuilder("scratch", "artifacts", bundler_config, osutils_mock, "") + .build_entry_points() + .build_default_values() + .build_esbuild_args_from_config() + .build_with_no_dependencies() + .get_command() + ) + self.assertEqual( + args, + [ + "x.js", + "--bundle", + "--platform=node", + "--outdir=artifacts", + "--target=es2020", + "--format=cjs", + "--minify", + "--sourcemap", + "--loader:.proto=text", + "--loader:.json=js", + "--external:@faker-js/faker", + "--external:uuidv4", + "--external:axios", + ], + ) + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_builds_args_from_config(self, osutils_mock): + bundler_config = { + "minify": True, + "sourcemap": False, + "format": "esm", + "target": "node14", + "loader": [".proto=text", ".json=js"], + "external": ["aws-sdk", "axios"], + } + + args = ( + EsbuildCommandBuilder("scratch", "artifacts", bundler_config, osutils_mock, "") + .build_esbuild_args_from_config() + .get_command() + ) + self.assertEqual( + args, + [ + "--minify", + "--target=node14", + "--format=esm", + "--external:aws-sdk", + "--external:axios", + "--loader:.proto=text", + "--loader:.json=js", + ], + ) + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_combined_builder_with_dependencies(self, osutils_mock): + bundler_config = {"entry_points": ["x.js"], "loader": [".proto=text", ".json=js"], "format": "esm"} + args = ( + EsbuildCommandBuilder("scratch", "artifacts", bundler_config, osutils_mock, "") + .build_entry_points() + .build_default_values() + .build_esbuild_args_from_config() + .get_command() + ) + self.assertEqual( + args, + [ + "x.js", + "--bundle", + "--platform=node", + "--outdir=artifacts", + "--target=es2020", + "--minify", + "--sourcemap", + "--format=esm", + "--loader:.proto=text", + "--loader:.json=js", + ], + ) diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_node.py b/tests/unit/workflows/nodejs_npm_esbuild/test_node.py deleted file mode 100644 index d9a7c89c2..000000000 --- a/tests/unit/workflows/nodejs_npm_esbuild/test_node.py +++ /dev/null @@ -1,77 +0,0 @@ -from unittest import TestCase -from mock import patch - -from aws_lambda_builders.workflows.nodejs_npm_esbuild.node import SubprocessNodejs, NodejsExecutionError - - -class FakePopen: - def __init__(self, out=b"out", err=b"err", retcode=0): - self.out = out - self.err = err - self.returncode = retcode - - def communicate(self): - return self.out, self.err - - -class TestSubprocessNodejs(TestCase): - @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") - def setUp(self, OSUtilMock): - self.osutils = OSUtilMock.return_value - self.osutils.pipe = "PIPE" - self.popen = FakePopen() - self.osutils.popen.side_effect = [self.popen] - - which = lambda cmd, executable_search_paths: ["{}/{}".format(executable_search_paths[0], cmd)] - - self.under_test = SubprocessNodejs(self.osutils, ["/a/b", "/c/d"], which) - - def test_run_executes_binary_found_in_exec_paths(self): - - self.under_test.run(["arg-a", "arg-b"]) - - self.osutils.popen.assert_called_with(["/a/b/node", "arg-a", "arg-b"], cwd=None, stderr="PIPE", stdout="PIPE") - - def test_uses_cwd_if_supplied(self): - self.under_test.run(["arg-a", "arg-b"], cwd="/a/cwd") - - self.osutils.popen.assert_called_with( - ["/a/b/node", "arg-a", "arg-b"], cwd="/a/cwd", stderr="PIPE", stdout="PIPE" - ) - - def test_returns_popen_out_decoded_if_retcode_is_0(self): - self.popen.out = b"some encoded text\n\n" - - result = self.under_test.run(["pack"]) - - self.assertEqual(result, "some encoded text") - - def test_raises_NodejsExecutionError_with_err_text_if_retcode_is_not_0(self): - self.popen.returncode = 1 - self.popen.err = b"some error text\n\n" - - with self.assertRaises(NodejsExecutionError) as raised: - self.under_test.run(["pack"]) - - self.assertEqual(raised.exception.args[0], "Nodejs Failed: some error text") - - def test_raises_NodejsExecutionError_if_which_returns_no_results(self): - - which = lambda cmd, executable_search_paths: [] - self.under_test = SubprocessNodejs(self.osutils, ["/a/b", "/c/d"], which) - with self.assertRaises(NodejsExecutionError) as raised: - self.under_test.run(["pack"]) - - self.assertEqual(raised.exception.args[0], "Nodejs Failed: cannot find nodejs") - - def test_raises_ValueError_if_args_not_a_list(self): - with self.assertRaises(ValueError) as raised: - self.under_test.run(("pack")) - - self.assertEqual(raised.exception.args[0], "args must be a list") - - def test_raises_ValueError_if_args_empty(self): - with self.assertRaises(ValueError) as raised: - self.under_test.run([]) - - self.assertEqual(raised.exception.args[0], "requires at least one arg") diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py index 493318e52..fe37bc48a 100644 --- a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py @@ -71,7 +71,7 @@ def test_sets_up_esbuild_search_path_from_npm_bin(self): ) self.osutils.popen.assert_called_with(["npm", "bin"], stdout="PIPE", stderr="PIPE", cwd="scratch_dir") - esbuild = workflow.actions[2].subprocess_esbuild + esbuild = workflow.actions[2]._subprocess_esbuild self.assertIsInstance(esbuild, SubprocessEsbuild) self.assertEqual(esbuild.executable_search_paths, ["project/bin"]) @@ -91,7 +91,7 @@ def test_sets_up_esbuild_search_path_with_workflow_executable_search_paths_after ) self.osutils.popen.assert_called_with(["npm", "bin"], stdout="PIPE", stderr="PIPE", cwd="scratch_dir") - esbuild = workflow.actions[2].subprocess_esbuild + esbuild = workflow.actions[2]._subprocess_esbuild self.assertIsInstance(esbuild, SubprocessEsbuild) self.assertEqual(esbuild.executable_search_paths, ["project/bin", "other/bin"])