From d2c1e25beec49e625213ba44f3d86bf69c007187 Mon Sep 17 00:00:00 2001 From: Gojko Adzic Date: Sun, 16 May 2021 09:48:03 +0200 Subject: [PATCH 01/17] upgrade integration tests to use nodejs14.x instead of obsolete 8.10; remove garbage lockfile left by npm 7 --- .../workflows/nodejs_npm/DESIGN.md | 133 ++++++++++++++++-- .../workflows/nodejs_npm/actions.py | 44 +++++- .../workflows/nodejs_npm/workflow.py | 4 +- .../workflows/nodejs_npm/test_nodejs_npm.py | 2 +- .../unit/workflows/nodejs_npm/test_actions.py | 25 ++++ .../workflows/nodejs_npm/test_workflow.py | 5 +- 6 files changed, 199 insertions(+), 14 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md index 803052ed7..e981e1d7b 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md +++ b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md @@ -2,9 +2,7 @@ ### Scope -This package is an effort to port the Claudia.JS packager to a library that can -be used to handle the dependency resolution portion of packaging NodeJS code -for use in AWS Lambda. The scope for this builder is to take an existing +The scope for this builder is to take an existing directory containing customer code, including a valid `package.json` manifest specifying third-party dependencies. The builder will use NPM to include production dependencies and exclude test resources in a way that makes them @@ -24,9 +22,16 @@ To speed up Lambda startup time and optimise usage costs, the correct thing to do in most cases is just to package up production dependencies. During development work we can expect that the local `node_modules` directory contains all the various dependency types, and NPM does not provide a way to directly identify -just the ones relevant for production. To identify production dependencies, -this packager needs to copy the source to a clean temporary directory and re-run -dependency installation there. +just the ones relevant for production. + +There are two ways to include only production dependencies in a package: + +1. **without a bundler**: Copy the source to a clean temporary directory and + re-run dependency installation there. + +2. **with a bundler**: Apply a javascript code bundler (such as `esbuild` or + `webpack`) to produce a single-file javascript bundle by recursively + resolving included dependencies, starting from the main lambda handler. A frequently used trick to speed up NodeJS Lambda deployment is to avoid bundling the `aws-sdk`, since it is already available on the Lambda VM. @@ -53,7 +58,9 @@ far from optimal to create a stand-alone module. Copying would lead to significa larger packages than necessary, as sub-modules might still have test resources, and common references from multiple projects would be duplicated. -NPM also uses a locking mechanism (`package-lock.json`) that's in many ways more +NPM also uses two locking mechanisms (`package-lock.json` and `npm-shrinkwrap.json`) +that can be used to freeze versions of dependencies recursively, and provide reproducible +builds. Before version 7, the locking mechanism was in many ways more broken than functional, as it in some cases hard-codes locks to local disk paths, and gets confused by including the same package as a dependency throughout the project tree in different dependency categories @@ -73,10 +80,96 @@ To fully deal with those cases, this packager may need to execute the dependency installation step on a Docker image compatible with the target Lambda environment. -### Implementation +### Choosing the packaging type + +For a large majority of projects, packaging is using a bundler has significant +advantages (speed and runtime package size, supporting local dependencies). + +However, there are also some drawbacks to using a bundler for a small set of +use cases (namely including packages with binary dependencies, such as `sharp`, a +popular image processing library). + +Because of this, it's important to support both ways of packaging. The version +without a bundler is slower, but will be correct in case of binary dependencies. +For backwards compatibility, this should be the default. + +Users should be able to activate packaging with a bundler for projects where that +is safe to do, such as those without any binary dependencies. + +The proposed approach is to use a "aws-sam" property in the package manifest +(`package.json`). If the `nodejs_npm` Lambda builder finds a matching property, it +knows that it is safe to use the bundler to package. + +The rest of this section outlines the major differences between packaging with +and without a bundler. + +#### packaging speed + +Packaging without a bundler is slower than using a bundler, as it +requires copying the project to a clean working directory, installing +dependencies and archiving into a single ZIP. + +Packaging with a bundler runs directly on files already on the disk, without +the need to copy or move files around. This approach can use the fast `npm ci` +command to just ensure that the dependencies are present on the disk instead of +always downloading all the dependencies. + +#### additional tools + +Packaging without a bundler does not require additional tools installed on the +development environment or CI systems, as it can just work with NPM. + +Packaging with a bundler requires installing additional tools (eg `esbuild`). + +#### handling local dependencies + +Packaging without a bundler requires complex +rewriting to handle local dependencies, and recursively packaging archives. In +theory, this was going to be implemented as a subsequent release after the +initial version of the `npm_nodejs` builder, but due to issues with container +environments and how `aws-lambda-builders` mounts the working directory, it was +not added for several years, and likely will not be implemented soon. + +Packaging with a bundler can handle local dependencies out of the box, since +it just traverses relative file liks. + +#### including non-javascript files + +Packaging without a bundler zips up entire contents of NPM packages. + +Packaging with a bundler only locates JavaScript files in the dependency tree. + +Some NPM packages include important binaries or resources in the NPM package, +which would not be included in the package without a bundler. This means that +packaging using a bundler is not universally applicable, and may never fully +replace packaging without a bundler. + +Some NPM packages include a lot of additional files not required at runtime. +`aws-sdk` for JavaScript (v2) is a good example, including TypeScript type +definitions, documentation and REST service definitions for automated code +generators. Packaging without a bundler includes these files as well, +unnecessarily increasing Lambda archive size. Packaging with a bundler just +ignores all these additional files out of the box. + +#### error reporting + +Packaging without a bundler leaves original file names and line numbers, ensuring +that any stack traces or exception reports correspond directly to the original +source files. + +Packaging with a bundler creates a single file from all the dependencies, so +stack traces on production no longer correspond to original source files. As a +workaround, bundlers can include a 'source map' file, to allow translating +production stack traces into source stack traces. Prior to Node 14, this +required including a separate NPM package, or additional tools. Since Node 14, +stack trace translation can be [activated using an environment +variable](https://serverless.pub/aws-lambda-node-sourcemaps/) + + +### Implementation without a bundler The general algorithm for preparing a node package for use on AWS Lambda -is as follows. +without a JavaScript bundler (`esbuild` or `webpack`) is as follows. #### Step 1: Prepare a clean copy of the project source files @@ -134,3 +227,25 @@ To fully support dependencies that download or compile binaries for a target pla needs to be executed inside a Docker image compatible with AWS Lambda. _(out of scope for the current version)_ +### Implementation with a bundler + +The general algorithm for preparing a node package for use on AWS Lambda +with a bundler (`esbuild` or `webpack`) is as follows. + +#### Step 1: ensure production dependencies are installed + +If the directory contains `package-lock.json` or `npm-shrinkwrap.json`, +execute [`npm ci`](https://docs.npmjs.com/cli/v7/commands/npm-ci). This +operation is designed to be faster than installing dependencies using `npm install` +in automated CI environments. + +If the directory does not contain lockfiles, but contains `package.json`, +execute [`npm install --production`] to download production dependencies. + +#### Step 2: bundle the main Lambda file + +Execute `esbuild` to produce a single JavaScript file by recursively resolving +included dependencies, and optionally a source map. + +Ensure that the target file name is the same as the entry point of the Lambda +function, so that there is no impact on the CloudFormation template. diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index 43259bd8c..7afa89dd2 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -9,7 +9,6 @@ LOG = logging.getLogger(__name__) - class NodejsNpmPackAction(BaseAction): """ @@ -185,7 +184,7 @@ def execute(self): """ Runs the action. - :raises lambda_builders.actions.ActionFailedError: when .npmrc copying fails + :raises lambda_builders.actions.ActionFailedError: when deleting .npmrc fails """ try: @@ -196,3 +195,44 @@ def execute(self): except OSError as ex: raise ActionFailedError(str(ex)) + + +class NodejsNpmLockFileCleanUpAction(BaseAction): + + """ + A Lambda Builder Action that cleans up garbage lockfile left by 7 in node_modules + """ + + NAME = "LockfileCleanUp" + DESCRIPTION = "Cleans garbage lockfiles dir" + PURPOSE = Purpose.COPY_SOURCE + + def __init__(self, artifacts_dir, osutils): + """ + :type artifacts_dir: str + :param artifacts_dir: an existing (writable) directory with project source files. + Dependencies will be installed in this directory. + + :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils + :param osutils: An instance of OS Utilities for file manipulation + """ + + super(NodejsNpmLockFileCleanUpAction, self).__init__() + self.artifacts_dir = artifacts_dir + self.osutils = osutils + + def execute(self): + """ + Runs the action. + + :raises lambda_builders.actions.ActionFailedError: when deleting the lockfile fails + """ + + try: + npmrc_path = self.osutils.joinpath(self.artifacts_dir, "node_modules", ".package-lock.json") + if self.osutils.file_exists(npmrc_path): + LOG.debug(".package-lock cleanup in: %s", self.artifacts_dir) + self.osutils.remove_file(npmrc_path) + + except OSError as ex: + raise ActionFailedError(str(ex)) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index b48ea4430..02fda7bcc 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -4,7 +4,8 @@ from aws_lambda_builders.path_resolver import PathResolver from aws_lambda_builders.workflow import BaseWorkflow, Capability from aws_lambda_builders.actions import CopySourceAction -from .actions import NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction +from .actions import NodejsNpmPackAction, NodejsNpmLockFileCleanUpAction, \ + NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction from .utils import OSUtils from .npm import SubprocessNpm @@ -50,6 +51,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES), npm_install, NodejsNpmrcCleanUpAction(artifacts_dir, osutils=osutils), + NodejsNpmLockFileCleanUpAction(artifacts_dir, osutils=osutils), ] def get_resolvers(self): diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index 83a8de6e0..c8fc0f8bc 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -22,7 +22,7 @@ def setUp(self): self.no_deps = os.path.join(self.TEST_DATA_FOLDER, "no-deps") self.builder = LambdaBuilder(language="nodejs", dependency_manager="npm", application_framework=None) - self.runtime = "nodejs8.10" + self.runtime = "nodejs14.x" def tearDown(self): shutil.rmtree(self.artifacts_dir) diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index 39007a59c..2da7054e0 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -7,6 +7,7 @@ NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction, + NodejsNpmLockFileCleanUpAction ) from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError @@ -140,3 +141,27 @@ def test_skips_npmrc_removal_if_npmrc_doesnt_exist(self, OSUtilMock): action.execute() osutils.remove_file.assert_not_called() + + +class TestNodejsNpmLockFileCleanUpAction(TestCase): + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_removes_dot_package_lock_if_exists(self, OSUtilMock): + osutils = OSUtilMock.return_value + osutils.joinpath.side_effect = lambda a, b, c: "{}/{}/{}".format(a, b, c) + + action = NodejsNpmLockFileCleanUpAction("artifacts", osutils=osutils) + osutils.file_exists.side_effect = [True] + action.execute() + + osutils.remove_file.assert_called_with("artifacts/node_modules/.package-lock.json") + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_skips_lockfile_removal_if_it_doesnt_exist(self, OSUtilMock): + osutils = OSUtilMock.return_value + osutils.joinpath.side_effect = lambda a, b, c: "{}/{}/{}".format(a, b, c) + + action = NodejsNpmLockFileCleanUpAction("artifacts", osutils=osutils) + osutils.file_exists.side_effect = [False] + action.execute() + + osutils.remove_file.assert_not_called() diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 3dac6d791..c4862b82e 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -7,6 +7,7 @@ NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction, + NodejsNpmLockFileCleanUpAction ) @@ -21,7 +22,7 @@ def test_workflow_sets_up_npm_actions(self): workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest") - self.assertEqual(len(workflow.actions), 5) + self.assertEqual(len(workflow.actions), 6) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) @@ -32,3 +33,5 @@ def test_workflow_sets_up_npm_actions(self): self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) + + self.assertIsInstance(workflow.actions[5], NodejsNpmLockFileCleanUpAction) From c8e32a4cffbfaf90879e8e288bfddb2f006b958f Mon Sep 17 00:00:00 2001 From: Gojko Adzic Date: Sun, 16 May 2021 22:30:46 +0200 Subject: [PATCH 02/17] esbuild bundling for javascript --- .../workflows/nodejs_npm/actions.py | 109 ++++++++++++++++++ .../workflows/nodejs_npm/esbuild.py | 87 ++++++++++++++ .../workflows/nodejs_npm/utils.py | 5 + .../workflows/nodejs_npm/workflow.py | 85 +++++++++++--- .../workflows/nodejs_npm/test_data/test.json | 1 + .../workflows/nodejs_npm/test_utils.py | 7 ++ .../workflows/nodejs_npm/test_nodejs_npm.py | 2 +- .../test_nodejs_npm_with_esbuild.py | 44 +++++++ .../testdata/no-deps-esbuild/excluded.js | 2 + .../testdata/no-deps-esbuild/included.js | 3 + .../testdata/no-deps-esbuild/package.json | 12 ++ .../unit/workflows/nodejs_npm/test_actions.py | 27 ++++- .../unit/workflows/nodejs_npm/test_esbuild.py | 82 +++++++++++++ .../workflows/nodejs_npm/test_workflow.py | 7 +- 14 files changed, 453 insertions(+), 20 deletions(-) create mode 100644 aws_lambda_builders/workflows/nodejs_npm/esbuild.py create mode 100644 tests/functional/workflows/nodejs_npm/test_data/test.json create mode 100644 tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py create mode 100644 tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/excluded.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/included.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json create mode 100644 tests/unit/workflows/nodejs_npm/test_esbuild.py diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index 7afa89dd2..1f939d038 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -6,9 +6,11 @@ from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError from .npm import NpmExecutionError +from .esbuild import EsbuildExecutionError LOG = logging.getLogger(__name__) + class NodejsNpmPackAction(BaseAction): """ @@ -111,6 +113,51 @@ def execute(self): raise ActionFailedError(str(ex)) +class NodejsNpmCIAction(BaseAction): + + """ + A Lambda Builder Action that installs NPM project dependencies + using the CI method - which is faster and better reproducible + for CI environments, but requires a lockfile (package-lock.json + or npm-shrinkwrap.json) + """ + + NAME = "NpmCI" + DESCRIPTION = "Installing dependencies from NPM using the CI method" + PURPOSE = Purpose.RESOLVE_DEPENDENCIES + + def __init__(self, artifacts_dir, subprocess_npm): + """ + :type artifacts_dir: str + :param artifacts_dir: an existing (writable) directory with project source files. + Dependencies will be installed in this directory. + + :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm + :param subprocess_npm: An instance of the NPM process wrapper + """ + + super(NodejsNpmCIAction, self).__init__() + self.artifacts_dir = artifacts_dir + self.subprocess_npm = subprocess_npm + + def execute(self): + """ + Runs the action. + + :raises lambda_builders.actions.ActionFailedError: when NPM execution fails + """ + + try: + LOG.debug("NODEJS installing ci in: %s", self.artifacts_dir) + + self.subprocess_npm.run( + ["ci"], cwd=self.artifacts_dir + ) + + except NpmExecutionError as ex: + raise ActionFailedError(str(ex)) + + class NodejsNpmrcCopyAction(BaseAction): """ @@ -236,3 +283,65 @@ def execute(self): except OSError as ex: raise ActionFailedError(str(ex)) + + +class EsbuildBundleAction(BaseAction): + + """ + A Lambda Builder Action that packages a Node.js package using esbuild into a single file + optionally transpiling TypeScript + """ + + NAME = "EsbuildBundle" + DESCRIPTION = "Packaging source using Esbuild" + PURPOSE = Purpose.COPY_SOURCE + + def __init__(self, source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild): + """ + :type source_dir: str + :param source_dir: an existing (readable) directory containing source files + + + :type artifacts_dir: str + :param artifacts_dir: an existing (writable) directory where to store the output. + Note that the actual result will be in the 'package' subdirectory here. + + :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils + :param osutils: An instance of OS Utilities for file manipulation + + :type subprocess_esbuild: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessEsbuild + :param subprocess_esbuild: An instance of the Esbuild process wrapper + """ + super(EsbuildBundleAction, self).__init__() + self.source_dir = source_dir + self.artifacts_dir = artifacts_dir + self.bundler_config = bundler_config + self.osutils = osutils + self.subprocess_esbuild = subprocess_esbuild + + def execute(self): + """ + Runs the action. + + :raises lambda_builders.actions.ActionFailedError: when esbuild packaging fails + """ + + if 'main' not in self.bundler_config: + raise ActionFailedError("main entry point not set %s" % self.bundler_config) + + entrypoint = self.bundler_config['main'] + entrypath = self.osutils.joinpath(self.source_dir, entrypoint) + LOG.debug("NODEJS bunlding %s using esbuild to %s", entrypath, self.artifacts_dir) + if not self.osutils.file_exists(entrypath): + raise ActionFailedError("main entry point %s does not exist" % entrypath) + + try: + self.subprocess_esbuild.run([ + entrypath, + "--bundle", "--platform=node", + "--minify", "--sourcemap", + ("--outfile=" + entrypoint) + ], cwd=self.artifacts_dir) + + except EsbuildExecutionError as ex: + raise ActionFailedError(str(ex)) diff --git a/aws_lambda_builders/workflows/nodejs_npm/esbuild.py b/aws_lambda_builders/workflows/nodejs_npm/esbuild.py new file mode 100644 index 000000000..ddd8f93c3 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm/esbuild.py @@ -0,0 +1,87 @@ +""" +Wrapper around calling esbuild through a subprocess. +""" + +import logging + +LOG = logging.getLogger(__name__) + + +class EsbuildExecutionError(Exception): + + """ + Exception raised in case NPM execution fails. + It will pass on the standard error output from the NPM console. + """ + + MESSAGE = "Esbuild Failed: {message}" + + def __init__(self, **kwargs): + Exception.__init__(self, self.MESSAGE.format(**kwargs)) + + +class SubprocessEsbuild(object): + + """ + Wrapper around the Esbuild command line utility, making it + easy to consume execution results. + """ + + def __init__(self, osutils, esbuild_exe=None): + """ + :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils + :param osutils: An instance of OS Utilities for file manipulation + + :type esbuild_exe: str + :param esbuild_exe: Path to the Esbuild binary. If not set, + the default executable path esbuild will be used + """ + self.osutils = osutils + + if esbuild_exe is None: + if osutils.is_windows(): + esbuild_exe = "esbuild.cmd" + else: + esbuild_exe = "esbuild" + + self.esbuild_exe = esbuild_exe + + def run(self, args, cwd=None): + + """ + Runs the action. + + :type args: list + :param args: Command line arguments to pass to Esbuild + + :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.EsbuildExecutionError: + 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_esbuild = [self.esbuild_exe] + args + + LOG.debug("executing Esbuild: %s", invoke_esbuild) + + p = self.osutils.popen(invoke_esbuild, stdout=self.osutils.pipe, stderr=self.osutils.pipe, cwd=cwd) + + out, err = p.communicate() + + if p.returncode != 0: + raise EsbuildExecutionError(message=err.decode("utf8").strip()) + + return out.decode("utf8").strip() diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py index ad92cfd23..aebfb3b7b 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/utils.py +++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py @@ -7,6 +7,7 @@ import tarfile import subprocess import shutil +import json class OSUtils(object): @@ -48,3 +49,7 @@ def abspath(self, path): def is_windows(self): return platform.system().lower() == "windows" + + def parse_json(self, path): + with open(path) as json_file: + return json.load(json_file) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 02fda7bcc..c2d7f030b 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -1,13 +1,22 @@ """ NodeJS NPM Workflow """ + +import logging +import json + from aws_lambda_builders.path_resolver import PathResolver from aws_lambda_builders.workflow import BaseWorkflow, Capability from aws_lambda_builders.actions import CopySourceAction +from aws_lambda_builders.exceptions import WorkflowFailedError from .actions import NodejsNpmPackAction, NodejsNpmLockFileCleanUpAction, \ - NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction + NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction, \ + NodejsNpmCIAction, EsbuildBundleAction from .utils import OSUtils from .npm import SubprocessNpm +from .esbuild import SubprocessEsbuild + +LOG = logging.getLogger(__name__) class NodejsNpmWorkflow(BaseWorkflow): @@ -23,29 +32,15 @@ class NodejsNpmWorkflow(BaseWorkflow): EXCLUDED_FILES = (".aws-sam", ".git") - def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): - - super(NodejsNpmWorkflow, self).__init__( - source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=runtime, **kwargs - ) - - if osutils is None: - osutils = OSUtils() - - subprocess_npm = SubprocessNpm(osutils) - + def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm): tar_dest_dir = osutils.joinpath(scratch_dir, "unpacked") tar_package_dir = osutils.joinpath(tar_dest_dir, "package") - npm_pack = NodejsNpmPackAction( tar_dest_dir, scratch_dir, manifest_path, osutils=osutils, subprocess_npm=subprocess_npm ) - npm_install = NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm) - npm_copy_npmrc = NodejsNpmrcCopyAction(tar_package_dir, source_dir, osutils=osutils) - - self.actions = [ + return [ npm_pack, npm_copy_npmrc, CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES), @@ -54,6 +49,62 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim NodejsNpmLockFileCleanUpAction(artifacts_dir, osutils=osutils), ] + def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutils, subprocess_npm): + lockfile_path = osutils.joinpath(source_dir, "package-lock.json") + shrinkwrap_path = osutils.joinpath(source_dir, "npm-shrinkwrap.json") + subprocess_esbuild = SubprocessEsbuild(osutils) + + if (osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path)): + install_action = NodejsNpmCIAction(source_dir, subprocess_npm=subprocess_npm) + else: + install_action = NodejsNpmInstallAction(source_dir, subprocess_npm=subprocess_npm) + + esbuild_action = EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) + return [ + install_action, + esbuild_action + ] + + def get_manifest_config(self, osutils, manifest_path): + LOG.debug("NODEJS reading manifest from %s", manifest_path) + try: + manifest = osutils.parse_json(manifest_path) + if 'aws-sam' in manifest and isinstance(manifest['aws-sam'], dict): + return manifest['aws-sam'] + else: + return {'bundler': ''} + except (OSError, json.decoder.JSONDecodeError) as ex: + raise WorkflowFailedError(workflow_name=self.NAME, action_name="ParseManifest", reason=str(ex)) + + def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): + + super(NodejsNpmWorkflow, self).__init__( + source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=runtime, **kwargs + ) + + if osutils is None: + osutils = OSUtils() + + subprocess_npm = SubprocessNpm(osutils) + + manifest_config = self.get_manifest_config(osutils, manifest_path) + + if manifest_config['bundler'] == 'esbuild': + self.actions = self.actions_with_bundler( + source_dir, + artifacts_dir, + manifest_config, + osutils, + subprocess_npm) + else: + self.actions = self.actions_without_bundler( + source_dir, + artifacts_dir, + scratch_dir, + manifest_path, + osutils, + subprocess_npm) + def get_resolvers(self): """ specialized path resolver that just returns the list of executable for the runtime on the path. diff --git a/tests/functional/workflows/nodejs_npm/test_data/test.json b/tests/functional/workflows/nodejs_npm/test_data/test.json new file mode 100644 index 000000000..a6867ce02 --- /dev/null +++ b/tests/functional/workflows/nodejs_npm/test_data/test.json @@ -0,0 +1 @@ +{"a":1,"b":{"c":2}} diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py index bd39e0ff3..fcaf4cd28 100644 --- a/tests/functional/workflows/nodejs_npm/test_utils.py +++ b/tests/functional/workflows/nodejs_npm/test_utils.py @@ -123,3 +123,10 @@ def test_popen_can_accept_cwd(self): self.assertEqual(p.returncode, 0) self.assertEqual(out.decode("utf8").strip(), os.path.abspath(testdata_dir)) + + def test_parse_json_reads_json_contents_into_memory(self): + + json_file = os.path.join(os.path.dirname(__file__), "test_data", "test.json") + json_contents = self.osutils.parse_json(json_file) + self.assertEqual(json_contents['a'], 1) + self.assertEqual(json_contents['b']['c'], 2) diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index c8fc0f8bc..5ce979157 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -125,4 +125,4 @@ def test_fails_if_package_json_is_broken(self): runtime=self.runtime, ) - self.assertIn("Unexpected end of JSON input", str(ctx.exception)) + self.assertIn("NodejsNpmBuilder:ParseManifest", str(ctx.exception)) diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py new file mode 100644 index 000000000..e520fd172 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py @@ -0,0 +1,44 @@ +import os +import shutil +import tempfile + +from unittest import TestCase + +from aws_lambda_builders.builder import LambdaBuilder +from aws_lambda_builders.exceptions import WorkflowFailedError + + +class TestNodejsNpmWorkflowWithEsbuild(TestCase): + """ + Verifies that `nodejs_npm` workflow works by building a Lambda using NPM + """ + + TEST_DATA_FOLDER = os.path.join(os.path.dirname(__file__), "testdata") + + def setUp(self): + self.artifacts_dir = tempfile.mkdtemp() + self.scratch_dir = tempfile.mkdtemp() + + self.no_deps = os.path.join(self.TEST_DATA_FOLDER, "no-deps-esbuild") + + self.builder = LambdaBuilder(language="nodejs", dependency_manager="npm", application_framework=None) + self.runtime = "nodejs14.x" + + def tearDown(self): + shutil.rmtree(self.artifacts_dir) + shutil.rmtree(self.scratch_dir) + + def testBuildsProjectWithoutDependencies(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "no-deps-esbuild") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + ) + + expected_files = {"included.js", "included.js.map"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/included.js b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/included.js new file mode 100644 index 000000000..17fcc2576 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/included.js @@ -0,0 +1,3 @@ +//included +const x = 1; +module.exports = x; diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json new file mode 100644 index 000000000..a3a237fb5 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json @@ -0,0 +1,12 @@ +{ + "name": "nodeps-esbuild", + "version": "1.0.0", + "description": "", + "keywords": [], + "author": "", + "license": "APACHE2.0", + "aws-sam": { + "bundler": "esbuild", + "main": "included.js" + } +} diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index 2da7054e0..c52e86299 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -7,7 +7,8 @@ NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction, - NodejsNpmLockFileCleanUpAction + NodejsNpmLockFileCleanUpAction, + NodejsNpmCIAction ) from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError @@ -80,6 +81,30 @@ def test_raises_action_failed_when_npm_fails(self, SubprocessNpmMock): self.assertEqual(raised.exception.args[0], "NPM Failed: boom!") +class TestNodejsNpmCIAction(TestCase): + @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm") + def test_tars_and_unpacks_npm_project(self, SubprocessNpmMock): + subprocess_npm = SubprocessNpmMock.return_value + + action = NodejsNpmCIAction("sources", subprocess_npm=subprocess_npm) + + action.execute() + + subprocess_npm.run.assert_called_with(["ci"], cwd="sources") + + @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm") + def test_raises_action_failed_when_npm_fails(self, SubprocessNpmMock): + subprocess_npm = SubprocessNpmMock.return_value + + builder_instance = SubprocessNpmMock.return_value + builder_instance.run.side_effect = NpmExecutionError(message="boom!") + + action = NodejsNpmCIAction("sources", subprocess_npm=subprocess_npm) + + with self.assertRaises(ActionFailedError) as raised: + action.execute() + + self.assertEqual(raised.exception.args[0], "NPM Failed: boom!") class TestNodejsNpmrcCopyAction(TestCase): @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") diff --git a/tests/unit/workflows/nodejs_npm/test_esbuild.py b/tests/unit/workflows/nodejs_npm/test_esbuild.py new file mode 100644 index 000000000..aadea80c7 --- /dev/null +++ b/tests/unit/workflows/nodejs_npm/test_esbuild.py @@ -0,0 +1,82 @@ +from unittest import TestCase +from mock import patch + +from aws_lambda_builders.workflows.nodejs_npm.esbuild import SubprocessEsbuild, EsbuildExecutionError + + +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 TestSubprocessNpm(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] + self.under_test = SubprocessEsbuild(self.osutils, esbuild_exe="/a/b/c/esbuild.exe") + + def test_run_executes_npm_on_nixes(self): + self.osutils.is_windows.side_effect = [False] + + self.under_test = SubprocessEsbuild(self.osutils) + + self.under_test.run(["arg-a", "arg-b"]) + + self.osutils.popen.assert_called_with(["esbuild", "arg-a", "arg-b"], cwd=None, stderr="PIPE", stdout="PIPE") + + def test_run_executes_esbuild_cmd_on_windows(self): + self.osutils.is_windows.side_effect = [True] + + self.under_test = SubprocessEsbuild(self.osutils) + + self.under_test.run(["arg-a", "arg-b"]) + + self.osutils.popen.assert_called_with(["esbuild.cmd", "arg-a", "arg-b"], cwd=None, stderr="PIPE", stdout="PIPE") + + def test_uses_custom_npm_path_if_supplied(self): + self.under_test.run(["arg-a", "arg-b"]) + + self.osutils.popen.assert_called_with(["/a/b/c/esbuild.exe", "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/c/esbuild.exe", "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_NpmExecutionError_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(EsbuildExecutionError) as raised: + self.under_test.run(["pack"]) + + self.assertEqual(raised.exception.args[0], "Esbuild Failed: some error text") + + 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/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index c4862b82e..7fbb3f8af 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -1,4 +1,5 @@ from unittest import TestCase +from mock import patch from aws_lambda_builders.actions import CopySourceAction from aws_lambda_builders.workflows.nodejs_npm.workflow import NodejsNpmWorkflow @@ -13,6 +14,10 @@ class TestNodejsNpmWorkflow(TestCase): + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def setUp(self, OSUtilMock): + self.osutils = OSUtilMock.return_value + """ the workflow requires an external utility (npm) to run, so it is extensively tested in integration tests. this is just a quick wiring test to provide fast feedback if things are badly broken @@ -20,7 +25,7 @@ class TestNodejsNpmWorkflow(TestCase): def test_workflow_sets_up_npm_actions(self): - workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest") + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) self.assertEqual(len(workflow.actions), 6) From 0a56b200a43ca95a0d8bef8522add69b6c590ee2 Mon Sep 17 00:00:00 2001 From: Gojko Adzic Date: Mon, 17 May 2021 14:42:10 +0200 Subject: [PATCH 03/17] initial esbuild support --- .../workflows/nodejs_npm/actions.py | 20 ++- .../workflows/nodejs_npm/esbuild.py | 37 ++++-- .../workflows/nodejs_npm/workflow.py | 19 ++- .../test_nodejs_npm_with_esbuild.py | 46 ++++++- .../testdata/esbuild-binary/package-lock.json | 31 +++++ .../testdata/esbuild-binary/package.json | 14 ++ .../testdata/no-deps-esbuild/.gitignore | 1 + .../testdata/no-deps-esbuild/package.json | 4 +- .../with-deps-esbuild-typescript/included.ts | 9 ++ .../package-lock.json | 46 +++++++ .../with-deps-esbuild-typescript/package.json | 18 +++ .../testdata/with-deps-esbuild/excluded.js | 2 + .../testdata/with-deps-esbuild/included.js | 6 + .../with-deps-esbuild/package-lock.json | 44 +++++++ .../testdata/with-deps-esbuild/package.json | 18 +++ .../unit/workflows/nodejs_npm/test_actions.py | 109 ++++++++++++++- .../unit/workflows/nodejs_npm/test_esbuild.py | 38 +++--- .../workflows/nodejs_npm/test_workflow.py | 124 +++++++++++++++++- 18 files changed, 528 insertions(+), 58 deletions(-) create mode 100644 tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package-lock.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/.gitignore create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/included.ts create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package-lock.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/excluded.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/included.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package-lock.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index 1f939d038..38a8ffb9a 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -335,13 +335,19 @@ def execute(self): if not self.osutils.file_exists(entrypath): raise ActionFailedError("main entry point %s does not exist" % entrypath) + args = [entrypoint, "--bundle", "--platform=node", "--format=cjs"] + skip_minify = "minify" in self.bundler_config and not self.bundler_config["minify"] + skip_sourcemap = "sourcemap" in self.bundler_config and not self.bundler_config["sourcemap"] + if not skip_minify: + args.append("--minify") + if not skip_sourcemap: + args.append("--sourcemap") + if "target" not in self.bundler_config: + args.append("--target=es2020") + else: + args.append("--target=" + self.bundler_config["target"]) + args.append("--outdir=" + self.artifacts_dir) try: - self.subprocess_esbuild.run([ - entrypath, - "--bundle", "--platform=node", - "--minify", "--sourcemap", - ("--outfile=" + entrypoint) - ], cwd=self.artifacts_dir) - + self.subprocess_esbuild.run(args, cwd=self.source_dir) except EsbuildExecutionError as ex: raise ActionFailedError(str(ex)) diff --git a/aws_lambda_builders/workflows/nodejs_npm/esbuild.py b/aws_lambda_builders/workflows/nodejs_npm/esbuild.py index ddd8f93c3..d3acc9aeb 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/esbuild.py +++ b/aws_lambda_builders/workflows/nodejs_npm/esbuild.py @@ -3,7 +3,6 @@ """ import logging - LOG = logging.getLogger(__name__) @@ -27,24 +26,38 @@ class SubprocessEsbuild(object): easy to consume execution results. """ - def __init__(self, osutils, esbuild_exe=None): + 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 esbuild_exe: str - :param esbuild_exe: Path to the Esbuild binary. If not set, - the default executable path esbuild will be used + :type bin_path: str + :param bin_path: Path to the NPM package binary utilities. This will + be used to find embedded esbuild at runtime if present in the package """ self.osutils = osutils + self.executable_search_paths = executable_search_paths + self.which = which + + def esbuild_binary(self): + """ + Finds the esbuild 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 NPM dependencies will be installed by the workflow + using one of the previous actions. + """ - if esbuild_exe is None: - if osutils.is_windows(): - esbuild_exe = "esbuild.cmd" - else: - esbuild_exe = "esbuild" + LOG.debug("checking for esbuild in: %s", self.executable_search_paths) + binaries = self.which('esbuild', executable_search_paths=self.executable_search_paths) + LOG.debug("potential esbuild binaries: %s", binaries) - self.esbuild_exe = esbuild_exe + if len(binaries) > 0: + return binaries[0] + else: + raise EsbuildExecutionError(message="cannot find esbuild") def run(self, args, cwd=None): @@ -73,7 +86,7 @@ def run(self, args, cwd=None): if not args: raise ValueError("requires at least one arg") - invoke_esbuild = [self.esbuild_exe] + args + invoke_esbuild = [self.esbuild_binary()] + args LOG.debug("executing Esbuild: %s", invoke_esbuild) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index c2d7f030b..68c3ea89a 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -8,10 +8,17 @@ from aws_lambda_builders.path_resolver import PathResolver from aws_lambda_builders.workflow import BaseWorkflow, Capability from aws_lambda_builders.actions import CopySourceAction +from aws_lambda_builders.utils import which from aws_lambda_builders.exceptions import WorkflowFailedError -from .actions import NodejsNpmPackAction, NodejsNpmLockFileCleanUpAction, \ - NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction, \ - NodejsNpmCIAction, EsbuildBundleAction +from .actions import ( + NodejsNpmPackAction, + NodejsNpmLockFileCleanUpAction, + NodejsNpmInstallAction, + NodejsNpmrcCopyAction, + NodejsNpmrcCleanUpAction, + NodejsNpmCIAction, + EsbuildBundleAction +) from .utils import OSUtils from .npm import SubprocessNpm from .esbuild import SubprocessEsbuild @@ -52,7 +59,11 @@ def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manife def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutils, subprocess_npm): lockfile_path = osutils.joinpath(source_dir, "package-lock.json") shrinkwrap_path = osutils.joinpath(source_dir, "npm-shrinkwrap.json") - subprocess_esbuild = SubprocessEsbuild(osutils) + npm_bin_path = subprocess_npm.run(["bin"], cwd=source_dir) + executable_search_paths = [npm_bin_path] + if self.executable_search_paths is not None: + executable_search_paths = executable_search_paths + self.executable_search_paths + subprocess_esbuild = SubprocessEsbuild(osutils, executable_search_paths, which=which) if (osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path)): install_action = NodejsNpmCIAction(source_dir, subprocess_npm=subprocess_npm) diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py index e520fd172..6f7dc22a0 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py @@ -1,11 +1,10 @@ import os import shutil import tempfile - from unittest import TestCase - from aws_lambda_builders.builder import LambdaBuilder -from aws_lambda_builders.exceptions import WorkflowFailedError +from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm +from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils class TestNodejsNpmWorkflowWithEsbuild(TestCase): @@ -28,8 +27,46 @@ def tearDown(self): shutil.rmtree(self.artifacts_dir) shutil.rmtree(self.scratch_dir) - def testBuildsProjectWithoutDependencies(self): + def testBuildsJavascriptProjectWithDependencies(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + ) + + expected_files = {"included.js", "included.js.map"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + + def testBuildsTypescriptProjects(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild-typescript") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + ) + + expected_files = {"included.js", "included.js.map"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + def testBuildsWithExternalEsbuild(self): + osutils = OSUtils() + npm = SubprocessNpm(osutils) source_dir = os.path.join(self.TEST_DATA_FOLDER, "no-deps-esbuild") + esbuild_dir = os.path.join(self.TEST_DATA_FOLDER, "esbuild-binary") + + npm.run(["ci"], cwd=esbuild_dir) + + binpath = npm.run(["bin"], cwd=esbuild_dir) self.builder.build( source_dir, @@ -37,6 +74,7 @@ def testBuildsProjectWithoutDependencies(self): self.scratch_dir, os.path.join(source_dir, "package.json"), runtime=self.runtime, + executable_search_paths=[binpath] ) expected_files = {"included.js", "included.js.map"} diff --git a/tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package-lock.json b/tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package-lock.json new file mode 100644 index 000000000..9ce0cc19a --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package-lock.json @@ -0,0 +1,31 @@ +{ + "name": "esbuild-binary", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "version": "1.0.0", + "license": "ISC", + "dependencies": { + "esbuild": "^0.11.23" + } + }, + "node_modules/esbuild": { + "version": "0.11.23", + "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.11.23.tgz", + "integrity": "sha512-iaiZZ9vUF5wJV8ob1tl+5aJTrwDczlvGP0JoMmnpC2B0ppiMCu8n8gmy5ZTGl5bcG081XBVn+U+jP+mPFm5T5Q==", + "hasInstallScript": true, + "bin": { + "esbuild": "bin/esbuild" + } + } + }, + "dependencies": { + "esbuild": { + "version": "0.11.23", + "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.11.23.tgz", + "integrity": "sha512-iaiZZ9vUF5wJV8ob1tl+5aJTrwDczlvGP0JoMmnpC2B0ppiMCu8n8gmy5ZTGl5bcG081XBVn+U+jP+mPFm5T5Q==" + } + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package.json b/tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package.json new file mode 100644 index 000000000..eb70cca6e --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package.json @@ -0,0 +1,14 @@ +{ + "name": "esbuild-binary", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "esbuild": "^0.11.23" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/.gitignore b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/.gitignore new file mode 100644 index 000000000..d8b83df9c --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/.gitignore @@ -0,0 +1 @@ +package-lock.json diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json index a3a237fb5..b7e6a930f 100644 --- a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json +++ b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json @@ -6,7 +6,7 @@ "author": "", "license": "APACHE2.0", "aws-sam": { - "bundler": "esbuild", - "main": "included.js" + "bundler": "esbuild", + "main": "included.js" } } diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/included.ts b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/included.ts new file mode 100644 index 000000000..82397888a --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/included.ts @@ -0,0 +1,9 @@ +import { APIGatewayProxyEvent, APIGatewayProxyResult } from "aws-lambda"; + +export const lambdaHandler = async (event: APIGatewayProxyEvent): Promise => { + const queries = JSON.stringify(event.queryStringParameters); + return { + statusCode: 200, + body: "OK" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package-lock.json b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package-lock.json new file mode 100644 index 000000000..7230f5534 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package-lock.json @@ -0,0 +1,46 @@ +{ + "name": "with-deps-esbuild-typescript", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "version": "1.0.0", + "license": "APACHE2.0", + "dependencies": { + "@types/aws-lambda": "^8.10.76" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } + }, + "node_modules/@types/aws-lambda": { + "version": "8.10.76", + "resolved": "https://registry.npmjs.org/@types/aws-lambda/-/aws-lambda-8.10.76.tgz", + "integrity": "sha512-lCTyeRm3NWqSwDnoji0z82Pl0tsOpr1p+33AiNeidgarloWXh3wdiVRUuxEa+sY9S5YLOYGz5X3N3Zvpibvm5w==" + }, + "node_modules/esbuild": { + "version": "0.11.23", + "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.11.23.tgz", + "integrity": "sha512-iaiZZ9vUF5wJV8ob1tl+5aJTrwDczlvGP0JoMmnpC2B0ppiMCu8n8gmy5ZTGl5bcG081XBVn+U+jP+mPFm5T5Q==", + "dev": true, + "hasInstallScript": true, + "bin": { + "esbuild": "bin/esbuild" + } + } + }, + "dependencies": { + "@types/aws-lambda": { + "version": "8.10.76", + "resolved": "https://registry.npmjs.org/@types/aws-lambda/-/aws-lambda-8.10.76.tgz", + "integrity": "sha512-lCTyeRm3NWqSwDnoji0z82Pl0tsOpr1p+33AiNeidgarloWXh3wdiVRUuxEa+sY9S5YLOYGz5X3N3Zvpibvm5w==" + }, + "esbuild": { + "version": "0.11.23", + "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.11.23.tgz", + "integrity": "sha512-iaiZZ9vUF5wJV8ob1tl+5aJTrwDczlvGP0JoMmnpC2B0ppiMCu8n8gmy5ZTGl5bcG081XBVn+U+jP+mPFm5T5Q==", + "dev": true + } + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json new file mode 100644 index 000000000..1eb39fa31 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json @@ -0,0 +1,18 @@ +{ + "name": "with-deps-esbuild-typescript", + "version": "1.0.0", + "description": "", + "keywords": [], + "author": "", + "license": "APACHE2.0", + "aws-sam": { + "bundler": "esbuild", + "main": "included.ts" + }, + "dependencies": { + "@types/aws-lambda": "^8.10.76" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/included.js b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/included.js new file mode 100644 index 000000000..6d43fd9f0 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/included.js @@ -0,0 +1,6 @@ +//included +const request = require('minimal-request-promise'); +exports.handler = async (event, context) => { + const result = await(request.get(event.url)); + return request; +}; diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package-lock.json b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package-lock.json new file mode 100644 index 000000000..2fdd27436 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package-lock.json @@ -0,0 +1,44 @@ +{ + "name": "with-deps-esbuild", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "version": "1.0.0", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "^1.5.0" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } + }, + "node_modules/esbuild": { + "version": "0.11.23", + "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.11.23.tgz", + "integrity": "sha512-iaiZZ9vUF5wJV8ob1tl+5aJTrwDczlvGP0JoMmnpC2B0ppiMCu8n8gmy5ZTGl5bcG081XBVn+U+jP+mPFm5T5Q==", + "hasInstallScript": true, + "bin": { + "esbuild": "bin/esbuild" + } + }, + "node_modules/minimal-request-promise": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/minimal-request-promise/-/minimal-request-promise-1.5.0.tgz", + "integrity": "sha1-YPXX9VtAJtGXB04uFVYm1MxcLrw=" + } + }, + "dependencies": { + "esbuild": { + "version": "0.11.23", + "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.11.23.tgz", + "integrity": "sha512-iaiZZ9vUF5wJV8ob1tl+5aJTrwDczlvGP0JoMmnpC2B0ppiMCu8n8gmy5ZTGl5bcG081XBVn+U+jP+mPFm5T5Q==" + }, + "minimal-request-promise": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/minimal-request-promise/-/minimal-request-promise-1.5.0.tgz", + "integrity": "sha1-YPXX9VtAJtGXB04uFVYm1MxcLrw=" + } + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json new file mode 100644 index 000000000..6bd4e06e2 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json @@ -0,0 +1,18 @@ +{ + "name": "with-deps-esbuild", + "version": "1.0.0", + "description": "", + "keywords": [], + "author": "", + "license": "APACHE2.0", + "aws-sam": { + "bundler": "esbuild", + "main": "included.js" + }, + "dependencies": { + "minimal-request-promise": "^1.5.0" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } +} diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index c52e86299..f643d9365 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -8,7 +8,8 @@ NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction, NodejsNpmLockFileCleanUpAction, - NodejsNpmCIAction + NodejsNpmCIAction, + EsbuildBundleAction ) from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError @@ -81,6 +82,7 @@ def test_raises_action_failed_when_npm_fails(self, SubprocessNpmMock): self.assertEqual(raised.exception.args[0], "NPM Failed: boom!") + class TestNodejsNpmCIAction(TestCase): @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm") def test_tars_and_unpacks_npm_project(self, SubprocessNpmMock): @@ -106,6 +108,7 @@ def test_raises_action_failed_when_npm_fails(self, SubprocessNpmMock): self.assertEqual(raised.exception.args[0], "NPM Failed: boom!") + class TestNodejsNpmrcCopyAction(TestCase): @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") def test_copies_npmrc_into_a_project(self, OSUtilMock): @@ -167,6 +170,18 @@ def test_skips_npmrc_removal_if_npmrc_doesnt_exist(self, OSUtilMock): osutils.remove_file.assert_not_called() + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_raises_action_failed_when_removing_fails(self, OSUtilMock): + osutils = OSUtilMock.return_value + osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) + + osutils.remove_file.side_effect = OSError() + + action = NodejsNpmrcCleanUpAction("artifacts", osutils=osutils) + + with self.assertRaises(ActionFailedError): + action.execute() + class TestNodejsNpmLockFileCleanUpAction(TestCase): @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") @@ -190,3 +205,95 @@ def test_skips_lockfile_removal_if_it_doesnt_exist(self, OSUtilMock): action.execute() osutils.remove_file.assert_not_called() + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def test_raises_action_failed_when_removing_fails(self, OSUtilMock): + osutils = OSUtilMock.return_value + osutils.joinpath.side_effect = lambda a, b, c: "{}/{}/{}".format(a, b, c) + + osutils.remove_file.side_effect = OSError() + + action = NodejsNpmLockFileCleanUpAction("artifacts", osutils=osutils) + + with self.assertRaises(ActionFailedError): + action.execute() + + +class TestEsbuildBundleAction(TestCase): + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + @patch("aws_lambda_builders.workflows.nodejs_npm.esbuild.SubprocessEsbuild") + def setUp(self, OSUtilMock, SubprocessEsbuildMock): + self.osutils = OSUtilMock.return_value + self.subprocess_esbuild = SubprocessEsbuildMock.return_value + self.osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) + self.osutils.file_exists.side_effect = [True] + + def test_raises_error_if_main_entrypoint_not_specified(self): + action = EsbuildBundleAction("source", "artifacts", {}, self.osutils, self.subprocess_esbuild) + with self.assertRaises(ActionFailedError): + action.execute() + + def test_packages_javascript_with_minification_and_sourcemap(self): + action = EsbuildBundleAction("source", "artifacts", {"main": "x.js"}, self.osutils, self.subprocess_esbuild) + action.execute() + + self.subprocess_esbuild.run.assert_called_with([ + "x.js", + "--bundle", + "--platform=node", + "--format=cjs", + "--minify", + "--sourcemap", + "--target=es2020", + "--outdir=artifacts" + ], cwd="source") + + def test_checks_if_entrypoint_exists(self): + + action = EsbuildBundleAction("source", "artifacts", {"main": "x.js"}, self.osutils, self.subprocess_esbuild) + self.osutils.file_exists.side_effect = [False] + + with self.assertRaises(ActionFailedError): + action.execute() + + self.osutils.file_exists.assert_called_with("source/x.js") + + def test_excludes_sourcemap_if_requested(self): + action = EsbuildBundleAction("source", "artifacts", {"main": "x.js", "sourcemap": False}, self.osutils, self.subprocess_esbuild) + action.execute() + self.subprocess_esbuild.run.assert_called_with([ + "x.js", + "--bundle", + "--platform=node", + "--format=cjs", + "--minify", + "--target=es2020", + "--outdir=artifacts" + ], cwd="source") + + def test_does_not_minify_if_requested(self): + action = EsbuildBundleAction("source", "artifacts", {"main": "x.js", "minify": False}, self.osutils, self.subprocess_esbuild) + action.execute() + self.subprocess_esbuild.run.assert_called_with([ + "x.js", + "--bundle", + "--platform=node", + "--format=cjs", + "--sourcemap", + "--target=es2020", + "--outdir=artifacts" + ], cwd="source") + + def test_uses_specified_target(self): + action = EsbuildBundleAction("source", "artifacts", {"main": "x.js", "target": "node14"}, self.osutils, self.subprocess_esbuild) + action.execute() + self.subprocess_esbuild.run.assert_called_with([ + "x.js", + "--bundle", + "--platform=node", + "--format=cjs", + "--minify", + "--sourcemap", + "--target=node14", + "--outdir=artifacts" + ], cwd="source") diff --git a/tests/unit/workflows/nodejs_npm/test_esbuild.py b/tests/unit/workflows/nodejs_npm/test_esbuild.py index aadea80c7..4a47d3939 100644 --- a/tests/unit/workflows/nodejs_npm/test_esbuild.py +++ b/tests/unit/workflows/nodejs_npm/test_esbuild.py @@ -14,43 +14,29 @@ def communicate(self): return self.out, self.err -class TestSubprocessNpm(TestCase): +class TestSubprocessEsbuild(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] - self.under_test = SubprocessEsbuild(self.osutils, esbuild_exe="/a/b/c/esbuild.exe") - def test_run_executes_npm_on_nixes(self): - self.osutils.is_windows.side_effect = [False] + which = lambda cmd, executable_search_paths: ["{}/{}".format(executable_search_paths[0], cmd)] - self.under_test = SubprocessEsbuild(self.osutils) + self.under_test = SubprocessEsbuild(self.osutils, ["/a/b", "/c/d"], which) - self.under_test.run(["arg-a", "arg-b"]) - - self.osutils.popen.assert_called_with(["esbuild", "arg-a", "arg-b"], cwd=None, stderr="PIPE", stdout="PIPE") - - def test_run_executes_esbuild_cmd_on_windows(self): - self.osutils.is_windows.side_effect = [True] - - self.under_test = SubprocessEsbuild(self.osutils) - - self.under_test.run(["arg-a", "arg-b"]) - - self.osutils.popen.assert_called_with(["esbuild.cmd", "arg-a", "arg-b"], cwd=None, stderr="PIPE", stdout="PIPE") + def test_run_executes_binary_found_in_exec_paths(self): - def test_uses_custom_npm_path_if_supplied(self): self.under_test.run(["arg-a", "arg-b"]) - self.osutils.popen.assert_called_with(["/a/b/c/esbuild.exe", "arg-a", "arg-b"], cwd=None, stderr="PIPE", stdout="PIPE") + self.osutils.popen.assert_called_with(["/a/b/esbuild", "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/c/esbuild.exe", "arg-a", "arg-b"], cwd="/a/cwd", stderr="PIPE", stdout="PIPE" + ["/a/b/esbuild", "arg-a", "arg-b"], cwd="/a/cwd", stderr="PIPE", stdout="PIPE" ) def test_returns_popen_out_decoded_if_retcode_is_0(self): @@ -60,7 +46,7 @@ def test_returns_popen_out_decoded_if_retcode_is_0(self): self.assertEqual(result, "some encoded text") - def test_raises_NpmExecutionError_with_err_text_if_retcode_is_not_0(self): + def test_raises_EsbuildExecutionError_with_err_text_if_retcode_is_not_0(self): self.popen.returncode = 1 self.popen.err = b"some error text\n\n" @@ -69,6 +55,16 @@ def test_raises_NpmExecutionError_with_err_text_if_retcode_is_not_0(self): self.assertEqual(raised.exception.args[0], "Esbuild Failed: some error text") + def test_raises_EsbuildExecutionError_if_which_returns_no_results(self): + + which = lambda cmd, executable_search_paths: [] + self.under_test = SubprocessEsbuild(self.osutils, ["/a/b", "/c/d"], which) + with self.assertRaises(EsbuildExecutionError) as raised: + self.under_test.run(["pack"]) + + self.assertEqual(raised.exception.args[0], "Esbuild Failed: cannot find esbuild") + + def test_raises_ValueError_if_args_not_a_list(self): with self.assertRaises(ValueError) as raised: self.under_test.run(("pack")) diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 7fbb3f8af..7db62be1a 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -1,29 +1,59 @@ from unittest import TestCase -from mock import patch +from mock import patch, call +from aws_lambda_builders.exceptions import WorkflowFailedError from aws_lambda_builders.actions import CopySourceAction from aws_lambda_builders.workflows.nodejs_npm.workflow import NodejsNpmWorkflow +from aws_lambda_builders.workflows.nodejs_npm.esbuild import SubprocessEsbuild from aws_lambda_builders.workflows.nodejs_npm.actions import ( NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction, - NodejsNpmLockFileCleanUpAction + NodejsNpmLockFileCleanUpAction, + NodejsNpmCIAction, + EsbuildBundleAction ) -class TestNodejsNpmWorkflow(TestCase): +class FakePopen: + def __init__(self, out=b"out", err=b"err", retcode=0): + self.out = out + self.err = err + self.returncode = retcode - @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") - def setUp(self, OSUtilMock): - self.osutils = OSUtilMock.return_value + def communicate(self): + return self.out, self.err + + +class TestNodejsNpmWorkflow(TestCase): """ the workflow requires an external utility (npm) to run, so it is extensively tested in integration tests. this is just a quick wiring test to provide fast feedback if things are badly broken """ - def test_workflow_sets_up_npm_actions(self): + @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] + self.osutils.is_windows.side_effect = [False] + self.osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) + + def test_workflow_fails_if_manifest_parsing_fails(self): + + self.osutils.parse_json.side_effect = OSError("boom!") + + with self.assertRaises(WorkflowFailedError) as raised: + NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + + self.assertEqual(raised.exception.args[0], "NodejsNpmBuilder:ParseManifest - boom!") + + self.osutils.parse_json.assert_called_with("manifest") + + def test_workflow_sets_up_npm_actions_without_bundler_if_manifest_doesnt_request_it(self): workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) @@ -40,3 +70,83 @@ def test_workflow_sets_up_npm_actions(self): self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) self.assertIsInstance(workflow.actions[5], NodejsNpmLockFileCleanUpAction) + + def test_workflow_sets_up_npm_actions_with_bundler_if_manifest_requests_it(self): + + self.osutils.parse_json.side_effect = [{"aws-sam": {"bundler": "esbuild"}}] + self.osutils.file_exists.side_effect = [False, False] + + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + + self.assertEqual(len(workflow.actions), 2) + + self.assertIsInstance(workflow.actions[0], NodejsNpmInstallAction) + + self.assertIsInstance(workflow.actions[1], EsbuildBundleAction) + + self.osutils.parse_json.assert_called_with("manifest") + + self.osutils.file_exists.assert_has_calls([call("source/package-lock.json"), call("source/npm-shrinkwrap.json")]) + + + def test_sets_up_esbuild_search_path_from_npm_bin(self): + + self.popen.out = b"project/bin" + self.osutils.parse_json.side_effect = [{"aws-sam": {"bundler": "esbuild"}}] + + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + + self.osutils.popen.assert_called_with(['npm', 'bin'], stdout='PIPE', stderr='PIPE', cwd='source') + + esbuild = workflow.actions[1].subprocess_esbuild + + self.assertIsInstance(esbuild, SubprocessEsbuild) + + self.assertEqual(esbuild.executable_search_paths, ["project/bin"]) + + def test_sets_up_esbuild_search_path_with_workflow_executable_search_paths_after_npm_bin(self): + + self.popen.out = b"project/bin" + self.osutils.parse_json.side_effect = [{"aws-sam": {"bundler": "esbuild"}}] + + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils, executable_search_paths=["other/bin"]) + + self.osutils.popen.assert_called_with(['npm', 'bin'], stdout='PIPE', stderr='PIPE', cwd='source') + + esbuild = workflow.actions[1].subprocess_esbuild + + self.assertIsInstance(esbuild, SubprocessEsbuild) + + self.assertEqual(esbuild.executable_search_paths, ["project/bin", "other/bin"]) + + def test_workflow_uses_npm_ci_if_lockfile_exists(self): + + self.osutils.parse_json.side_effect = [{"aws-sam": {"bundler": "esbuild"}}] + self.osutils.file_exists.side_effect = [True] + + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + + self.assertEqual(len(workflow.actions), 2) + + self.assertIsInstance(workflow.actions[0], NodejsNpmCIAction) + + self.assertIsInstance(workflow.actions[1], EsbuildBundleAction) + + self.osutils.file_exists.assert_has_calls([call("source/package-lock.json")]) + + def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): + + self.osutils.parse_json.side_effect = [{"aws-sam": {"bundler": "esbuild"}}] + self.osutils.file_exists.side_effect = [False, True] + + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + + self.assertEqual(len(workflow.actions), 2) + + self.assertIsInstance(workflow.actions[0], NodejsNpmCIAction) + + self.assertIsInstance(workflow.actions[1], EsbuildBundleAction) + + self.osutils.file_exists.assert_has_calls([call("source/package-lock.json"), call("source/npm-shrinkwrap.json")]) + + From 09567d799b9aca34bbccc7925b55eb5db7ecacc1 Mon Sep 17 00:00:00 2001 From: Gojko Adzic Date: Wed, 19 May 2021 09:12:04 +0200 Subject: [PATCH 04/17] aws-sam -> aws_sam, revert to node 10 for integration tests, document package.json options --- .../workflows/nodejs_npm/DESIGN.md | 126 ++++++++++++++++++ .../workflows/nodejs_npm/workflow.py | 6 +- .../workflows/nodejs_npm/test_nodejs_npm.py | 2 +- .../testdata/no-deps-esbuild/package.json | 2 +- .../with-deps-esbuild-typescript/package.json | 2 +- .../testdata/with-deps-esbuild/package.json | 2 +- .../workflows/nodejs_npm/test_workflow.py | 13 +- 7 files changed, 140 insertions(+), 13 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md index e981e1d7b..e80926191 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md +++ b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md @@ -249,3 +249,129 @@ included dependencies, and optionally a source map. Ensure that the target file name is the same as the entry point of the Lambda function, so that there is no impact on the CloudFormation template. + + +### Activating the bundler workflow + +Because there are advantages and disadvantages to both approaches (with and +without a bundler), the user should be able to choose between them. The default +is not to use a bundler (both because it's universally applicable and for +backwards compatibility). Node.js pakage manifests (`package.json`) allow for +custom properties, so a user can activate the bundler process by providing an +`aws_sam` configuration property in the package manifest. If this property is +present in the package manifest, and the sub-property `bundler` equals +`esbuild`, the Node.js NPM Lambda builder activates the bundler process. + +Because the Lambda builder workflow is not aware of the main entry point for +bundling, the user must also specify the main entry point for bundling (the +file containing the Lambda handler function). This is a bit of an unfortunate +duplication with SAM Cloudformation template, but with the current workflow +design there is no way around it. + +The following example is a minimal `package.json` to activate the `esbuild` bundler +on a javascript file, starting from `lambda.js`. It will produce a bundled `lambda.js` +in the artifacts folder. + +```json +{ + "name": "nodeps-esbuild", + "version": "1.0.0", + "license": "APACHE2.0", + "aws_sam": { + "bundler": "esbuild", + "main": "lambda.js" + } +} +``` + +#### Locating the esbuild binary + +`esbuild` supports platform-independent binary distribution using NPM, by +including the `esbuild` package as a dependency. The Lambda builder should +first try to locate the binary in the Lambda code repository (allowing the +user to include a specific version). Failing that, the Lambda builder should +try to locate the `esbuild` binary in the `executable_search_paths` configured +for the workflow, then the operating system `PATH` environment variable. + +The Lambda builder **should not** bring its own `esbuild` binary, but it should +clearly point to the error when one is not found, to allow users to configure the +build correctly. + +In the previous example, the esbuild binary is not included in the package dependencies, +so the Lambda builder will use the system executable paths to search for it. In the +example below, `esbuild` is included in the package, so the Lambda builder should use it +directly. + +```json +{ + "name": "with-deps-esbuild", + "version": "1.0.0", + "license": "APACHE2.0", + "aws_sam": { + "bundler": "esbuild", + "main": "lambda.js" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } +} +``` + +For a full example, see the [`with-deps-esbuild`](../../../tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/) test project. + +#### Building typescript + +`esbuild` supports bundling typescript out of the box and transpiling it to plain +javascript. The user just needs to point to a typescript file as the main entry point, +as in the example below. There is no transpiling process needed upfront. + + +```js +{ + "name": "with-deps-esbuild-typescript", + "version": "1.0.0", + "license": "APACHE2.0", + "aws_sam": { + "bundler": "esbuild", + "main": "included.ts" + }, + "dependencies": { + "@types/aws-lambda": "^8.10.76" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } +} +``` + +For a full example, see the [`with-deps-esbuild-typescript`](../../../tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/) test project. + +**important note:** esbuild does not perform type checking, so users wanting to ensure type-checks need to run the `tsc` process as part of their +testing flow before invoking `sam build`. For additional typescript caveats with esbuild, check out . + +#### Configuring the bundler + +The Lambda builder invokes `esbuild` with sensible defaults that will work for the majority of cases. Importantly, the following three parameters are set by default + +* `--minify`, as it [produces a smaller runtime package](https://esbuild.github.io/api/#minify) +* `--sourcemap`, as it generates a [source map that allows for correct stack trace reporting](https://esbuild.github.io/api/#sourcemap) in case of errors (see the [Error reporting](#error-reporting) section above) +* `--target es2020`, as it allows for javascript features present in Node 14 + +Users might want to tweak some of these runtime arguments for a specific project, for example not including the source map to further reduce the package size, or restricting javascript features to an older version. The Lambda builder allows this with optional sub-properties of the `aws_sam` configuration property. + +* `target`: string, corresponding to a supported [esbuild target](https://esbuild.github.io/api/#target) property +* `minify`: boolean, defaulting to `true` +* `sourcemap`: boolean, defaulting to `true` + +Here is an example that deactivates minification and source maps, and supports JavaScript features compatible with Node.js version 10. + +```json +{ + "aws_sam": { + "bundler": "esbuild", + "main": "included.ts", + "target": "node10", + "minify": false, + "sourcemap": false + } +} diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 68c3ea89a..45fd4bf48 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -39,6 +39,8 @@ class NodejsNpmWorkflow(BaseWorkflow): EXCLUDED_FILES = (".aws-sam", ".git") + CONFIG_PROPERTY = "aws_sam" + def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm): tar_dest_dir = osutils.joinpath(scratch_dir, "unpacked") tar_package_dir = osutils.joinpath(tar_dest_dir, "package") @@ -80,8 +82,8 @@ def get_manifest_config(self, osutils, manifest_path): LOG.debug("NODEJS reading manifest from %s", manifest_path) try: manifest = osutils.parse_json(manifest_path) - if 'aws-sam' in manifest and isinstance(manifest['aws-sam'], dict): - return manifest['aws-sam'] + if self.CONFIG_PROPERTY in manifest and isinstance(manifest[self.CONFIG_PROPERTY], dict): + return manifest[self.CONFIG_PROPERTY] else: return {'bundler': ''} except (OSError, json.decoder.JSONDecodeError) as ex: diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index 5ce979157..d2a0cefc9 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -22,7 +22,7 @@ def setUp(self): self.no_deps = os.path.join(self.TEST_DATA_FOLDER, "no-deps") self.builder = LambdaBuilder(language="nodejs", dependency_manager="npm", application_framework=None) - self.runtime = "nodejs14.x" + self.runtime = "nodejs10.x" def tearDown(self): shutil.rmtree(self.artifacts_dir) diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json index b7e6a930f..79597eff8 100644 --- a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json +++ b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json @@ -5,7 +5,7 @@ "keywords": [], "author": "", "license": "APACHE2.0", - "aws-sam": { + "aws_sam": { "bundler": "esbuild", "main": "included.js" } diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json index 1eb39fa31..bc41f7546 100644 --- a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json @@ -5,7 +5,7 @@ "keywords": [], "author": "", "license": "APACHE2.0", - "aws-sam": { + "aws_sam": { "bundler": "esbuild", "main": "included.ts" }, diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json index 6bd4e06e2..2e65a570d 100644 --- a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json @@ -5,7 +5,7 @@ "keywords": [], "author": "", "license": "APACHE2.0", - "aws-sam": { + "aws_sam": { "bundler": "esbuild", "main": "included.js" }, diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 7db62be1a..354c0e32d 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -73,7 +73,8 @@ def test_workflow_sets_up_npm_actions_without_bundler_if_manifest_doesnt_request def test_workflow_sets_up_npm_actions_with_bundler_if_manifest_requests_it(self): - self.osutils.parse_json.side_effect = [{"aws-sam": {"bundler": "esbuild"}}] + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] + self.osutils.file_exists.side_effect = [False, False] workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) @@ -92,7 +93,7 @@ def test_workflow_sets_up_npm_actions_with_bundler_if_manifest_requests_it(self) def test_sets_up_esbuild_search_path_from_npm_bin(self): self.popen.out = b"project/bin" - self.osutils.parse_json.side_effect = [{"aws-sam": {"bundler": "esbuild"}}] + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) @@ -107,7 +108,7 @@ def test_sets_up_esbuild_search_path_from_npm_bin(self): def test_sets_up_esbuild_search_path_with_workflow_executable_search_paths_after_npm_bin(self): self.popen.out = b"project/bin" - self.osutils.parse_json.side_effect = [{"aws-sam": {"bundler": "esbuild"}}] + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils, executable_search_paths=["other/bin"]) @@ -121,7 +122,7 @@ def test_sets_up_esbuild_search_path_with_workflow_executable_search_paths_after def test_workflow_uses_npm_ci_if_lockfile_exists(self): - self.osutils.parse_json.side_effect = [{"aws-sam": {"bundler": "esbuild"}}] + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] self.osutils.file_exists.side_effect = [True] workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) @@ -136,7 +137,7 @@ def test_workflow_uses_npm_ci_if_lockfile_exists(self): def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): - self.osutils.parse_json.side_effect = [{"aws-sam": {"bundler": "esbuild"}}] + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] self.osutils.file_exists.side_effect = [False, True] workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) @@ -148,5 +149,3 @@ def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): self.assertIsInstance(workflow.actions[1], EsbuildBundleAction) self.osutils.file_exists.assert_has_calls([call("source/package-lock.json"), call("source/npm-shrinkwrap.json")]) - - From 8174eac7b97466c604bd06621e873213512244e6 Mon Sep 17 00:00:00 2001 From: Gojko Adzic Date: Mon, 12 Jul 2021 15:50:24 +0200 Subject: [PATCH 05/17] rename main -> entry_point, support multiple entrypoints --- .../workflows/nodejs_npm/DESIGN.md | 24 ++++-- .../workflows/nodejs_npm/actions.py | 38 ++++++--- .../workflows/nodejs_npm/workflow.py | 5 +- .../test_nodejs_npm_with_esbuild.py | 14 ++++ .../testdata/no-deps-esbuild/package.json | 2 +- .../.gitignore | 1 + .../excluded.js | 2 + .../included.js | 3 + .../included2.js | 3 + .../package.json | 18 ++++ .../with-deps-esbuild-typescript/package.json | 2 +- .../testdata/with-deps-esbuild/package.json | 2 +- .../unit/workflows/nodejs_npm/test_actions.py | 84 ++++++++++++++++--- 13 files changed, 161 insertions(+), 37 deletions(-) create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/.gitignore create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/excluded.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included2.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/package.json diff --git a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md index e80926191..de3800617 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md +++ b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md @@ -262,11 +262,17 @@ custom properties, so a user can activate the bundler process by providing an present in the package manifest, and the sub-property `bundler` equals `esbuild`, the Node.js NPM Lambda builder activates the bundler process. -Because the Lambda builder workflow is not aware of the main entry point for -bundling, the user must also specify the main entry point for bundling (the -file containing the Lambda handler function). This is a bit of an unfortunate -duplication with SAM Cloudformation template, but with the current workflow -design there is no way around it. +Because the Lambda builder workflow is not aware of the main lambda function +definition, (the file containing the Lambda handler function) the user must +also specify the main entry point for bundling . This is a bit of an +unfortunate duplication with SAM Cloudformation template, but with the current +workflow design there is no way around it. + +In addition, as a single JavaScript source package can contain multiple functions, +and can be included multiple times in a single CloudFormation template, it's possible +that there may be multiple entry points for bundling. SAM build executes the build +only once for the function in this case, so all entry points have to be bundled +at once. The following example is a minimal `package.json` to activate the `esbuild` bundler on a javascript file, starting from `lambda.js`. It will produce a bundled `lambda.js` @@ -279,7 +285,7 @@ in the artifacts folder. "license": "APACHE2.0", "aws_sam": { "bundler": "esbuild", - "main": "lambda.js" + "entry_points": ["lambda.js"] } } ``` @@ -309,7 +315,7 @@ directly. "license": "APACHE2.0", "aws_sam": { "bundler": "esbuild", - "main": "lambda.js" + "entry_points": ["lambda.js"] }, "devDependencies": { "esbuild": "^0.11.23" @@ -333,7 +339,7 @@ as in the example below. There is no transpiling process needed upfront. "license": "APACHE2.0", "aws_sam": { "bundler": "esbuild", - "main": "included.ts" + "entry_points": ["included.ts"] }, "dependencies": { "@types/aws-lambda": "^8.10.76" @@ -369,7 +375,7 @@ Here is an example that deactivates minification and source maps, and supports J { "aws_sam": { "bundler": "esbuild", - "main": "included.ts", + "entry_points": ["included.ts"], "target": "node10", "minify": false, "sourcemap": false diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index 38a8ffb9a..d8d1fdc06 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -81,7 +81,7 @@ class NodejsNpmInstallAction(BaseAction): DESCRIPTION = "Installing dependencies from NPM" PURPOSE = Purpose.RESOLVE_DEPENDENCIES - def __init__(self, artifacts_dir, subprocess_npm): + def __init__(self, artifacts_dir, subprocess_npm, mode="--production"): """ :type artifacts_dir: str :param artifacts_dir: an existing (writable) directory with project source files. @@ -89,11 +89,15 @@ def __init__(self, artifacts_dir, subprocess_npm): :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm :param subprocess_npm: An instance of the NPM process wrapper + + :type mode: str + :param mode: NPM installation mode (eg --production=false to force dev dependencies) """ super(NodejsNpmInstallAction, self).__init__() self.artifacts_dir = artifacts_dir self.subprocess_npm = subprocess_npm + self.mode = mode def execute(self): """ @@ -106,7 +110,7 @@ def execute(self): LOG.debug("NODEJS installing in: %s", self.artifacts_dir) self.subprocess_npm.run( - ["install", "-q", "--no-audit", "--no-save", "--production", "--unsafe-perm"], cwd=self.artifacts_dir + ["install", "-q", "--no-audit", "--no-save", self.mode, "--unsafe-perm"], cwd=self.artifacts_dir ) except NpmExecutionError as ex: @@ -326,16 +330,26 @@ def execute(self): :raises lambda_builders.actions.ActionFailedError: when esbuild packaging fails """ - if 'main' not in self.bundler_config: - raise ActionFailedError("main entry point not set %s" % self.bundler_config) + if 'entry_points' not in self.bundler_config: + raise ActionFailedError("entry_points not set ({})".format(self.bundler_config)) + + entry_points = self.bundler_config['entry_points'] + + if not isinstance(entry_points, list): + raise ActionFailedError("entry_points must be a list ({})".format(self.bundler_config)) + + if len(entry_points) < 1: + raise ActionFailedError("entry_points must not be empty ({})".format(self.bundler_config)) + + entry_paths = [self.osutils.joinpath(self.source_dir, entry_point) for entry_point in entry_points] + + LOG.debug("NODEJS building %s using esbuild to %s", entry_paths, self.artifacts_dir) - entrypoint = self.bundler_config['main'] - entrypath = self.osutils.joinpath(self.source_dir, entrypoint) - LOG.debug("NODEJS bunlding %s using esbuild to %s", entrypath, self.artifacts_dir) - if not self.osutils.file_exists(entrypath): - raise ActionFailedError("main entry point %s does not exist" % entrypath) + for entry_point in entry_paths: + if not self.osutils.file_exists(entry_point): + raise ActionFailedError("entry point {} does not exist".format(entry_point)) - args = [entrypoint, "--bundle", "--platform=node", "--format=cjs"] + args = entry_points + ["--bundle", "--platform=node", "--format=cjs"] skip_minify = "minify" in self.bundler_config and not self.bundler_config["minify"] skip_sourcemap = "sourcemap" in self.bundler_config and not self.bundler_config["sourcemap"] if not skip_minify: @@ -345,8 +359,8 @@ def execute(self): if "target" not in self.bundler_config: args.append("--target=es2020") else: - args.append("--target=" + self.bundler_config["target"]) - args.append("--outdir=" + self.artifacts_dir) + args.append("--target={}".format(self.bundler_config["target"])) + args.append("--outdir={}".format(self.artifacts_dir)) try: self.subprocess_esbuild.run(args, cwd=self.source_dir) except EsbuildExecutionError as ex: diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 45fd4bf48..c6f5d749e 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -70,7 +70,10 @@ def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutil if (osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path)): install_action = NodejsNpmCIAction(source_dir, subprocess_npm=subprocess_npm) else: - install_action = NodejsNpmInstallAction(source_dir, subprocess_npm=subprocess_npm) + install_action = NodejsNpmInstallAction( + source_dir, + subprocess_npm=subprocess_npm, + mode="--production=false") esbuild_action = EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) return [ diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py index 6f7dc22a0..9ae5628a3 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py @@ -42,6 +42,20 @@ def testBuildsJavascriptProjectWithDependencies(self): output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) + def testBuildsJavascriptProjectWithMultipleEntrypoints(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild-multiple-entrypoints") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + ) + + expected_files = {"included.js", "included.js.map", "included2.js", "included2.js.map"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) def testBuildsTypescriptProjects(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild-typescript") diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json index 79597eff8..997d05815 100644 --- a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json +++ b/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json @@ -7,6 +7,6 @@ "license": "APACHE2.0", "aws_sam": { "bundler": "esbuild", - "main": "included.js" + "entry_points": ["included.js"] } } diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/.gitignore b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/.gitignore new file mode 100644 index 000000000..d8b83df9c --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/.gitignore @@ -0,0 +1 @@ +package-lock.json diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included.js b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included.js new file mode 100644 index 000000000..17fcc2576 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included.js @@ -0,0 +1,3 @@ +//included +const x = 1; +module.exports = x; diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included2.js b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included2.js new file mode 100644 index 000000000..2f7ab0b1e --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included2.js @@ -0,0 +1,3 @@ +//included2 +const y = 1; +module.exports = y; diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/package.json b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/package.json new file mode 100644 index 000000000..e85d55506 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/package.json @@ -0,0 +1,18 @@ +{ + "name": "with-deps-esbuild-multiple-entrypoints", + "version": "1.0.0", + "description": "", + "keywords": [], + "author": "", + "license": "APACHE2.0", + "aws_sam": { + "bundler": "esbuild", + "entry_points": ["included.js", "included2.js"] + }, + "dependencies": { + "minimal-request-promise": "^1.5.0" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json index bc41f7546..43827dd4c 100644 --- a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json @@ -7,7 +7,7 @@ "license": "APACHE2.0", "aws_sam": { "bundler": "esbuild", - "main": "included.ts" + "entry_points": ["included.ts"] }, "dependencies": { "@types/aws-lambda": "^8.10.76" diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json index 2e65a570d..c4f10260c 100644 --- a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json @@ -7,7 +7,7 @@ "license": "APACHE2.0", "aws_sam": { "bundler": "esbuild", - "main": "included.js" + "entry_points": ["included.js"] }, "dependencies": { "minimal-request-promise": "^1.5.0" diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index f643d9365..52e3e21d9 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -57,7 +57,7 @@ def test_raises_action_failed_when_npm_fails(self, OSUtilMock, SubprocessNpmMock class TestNodejsNpmInstallAction(TestCase): @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm") - def test_tars_and_unpacks_npm_project(self, SubprocessNpmMock): + def test_installs_npm_production_dependencies_for_npm_project(self, SubprocessNpmMock): subprocess_npm = SubprocessNpmMock.return_value action = NodejsNpmInstallAction("artifacts", subprocess_npm=subprocess_npm) @@ -68,6 +68,18 @@ def test_tars_and_unpacks_npm_project(self, SubprocessNpmMock): subprocess_npm.run.assert_called_with(expected_args, cwd="artifacts") + @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm") + def test_can_set_mode(self, SubprocessNpmMock): + subprocess_npm = SubprocessNpmMock.return_value + + action = NodejsNpmInstallAction("artifacts", subprocess_npm=subprocess_npm, mode="--no-optional") + + action.execute() + + expected_args = ["install", "-q", "--no-audit", "--no-save", "--no-optional", "--unsafe-perm"] + + subprocess_npm.run.assert_called_with(expected_args, cwd="artifacts") + @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm") def test_raises_action_failed_when_npm_fails(self, SubprocessNpmMock): subprocess_npm = SubprocessNpmMock.return_value @@ -226,15 +238,32 @@ def setUp(self, OSUtilMock, SubprocessEsbuildMock): self.osutils = OSUtilMock.return_value self.subprocess_esbuild = SubprocessEsbuildMock.return_value self.osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) - self.osutils.file_exists.side_effect = [True] + self.osutils.file_exists.side_effect = [True, True] - def test_raises_error_if_main_entrypoint_not_specified(self): - action = EsbuildBundleAction("source", "artifacts", {}, self.osutils, self.subprocess_esbuild) - with self.assertRaises(ActionFailedError): + def test_raises_error_if_entrypoints_not_specified(self): + action = EsbuildBundleAction("source", "artifacts", {"config": "param"}, self.osutils, self.subprocess_esbuild) + with self.assertRaises(ActionFailedError) as raised: action.execute() + self.assertEqual(raised.exception.args[0], "entry_points not set ({'config': 'param'})") + + def test_raises_error_if_entrypoints_not_a_list(self): + action = EsbuildBundleAction("source", "artifacts", {"config": "param", "entry_points": "abc"}, self.osutils, self.subprocess_esbuild) + with self.assertRaises(ActionFailedError) as raised: + action.execute() + + self.assertEqual(raised.exception.args[0], "entry_points must be a list ({'config': 'param', 'entry_points': 'abc'})") + + def test_raises_error_if_entrypoints_empty_list(self): + action = EsbuildBundleAction("source", "artifacts", {"config": "param", "entry_points": []}, self.osutils, self.subprocess_esbuild) + with self.assertRaises(ActionFailedError) as raised: + action.execute() + + self.assertEqual(raised.exception.args[0], "entry_points must not be empty ({'config': 'param', 'entry_points': []})") + + def test_packages_javascript_with_minification_and_sourcemap(self): - action = EsbuildBundleAction("source", "artifacts", {"main": "x.js"}, self.osutils, self.subprocess_esbuild) + action = EsbuildBundleAction("source", "artifacts", {"entry_points": ["x.js"]}, self.osutils, self.subprocess_esbuild) action.execute() self.subprocess_esbuild.run.assert_called_with([ @@ -248,18 +277,34 @@ def test_packages_javascript_with_minification_and_sourcemap(self): "--outdir=artifacts" ], cwd="source") - def test_checks_if_entrypoint_exists(self): + def test_checks_if_single_entrypoint_exists(self): - action = EsbuildBundleAction("source", "artifacts", {"main": "x.js"}, self.osutils, self.subprocess_esbuild) + action = EsbuildBundleAction("source", "artifacts", {"entry_points": ["x.js"]}, self.osutils, self.subprocess_esbuild) self.osutils.file_exists.side_effect = [False] - with self.assertRaises(ActionFailedError): + with self.assertRaises(ActionFailedError) as raised: action.execute() self.osutils.file_exists.assert_called_with("source/x.js") + self.assertEqual(raised.exception.args[0], "entry point source/x.js does not exist") + + 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) + + with self.assertRaises(ActionFailedError) as raised: + action.execute() + + self.osutils.file_exists.assert_any_call("source/x.js") + + self.osutils.file_exists.assert_called_with("source/y.js") + + self.assertEqual(raised.exception.args[0], "entry point source/y.js does not exist") + def test_excludes_sourcemap_if_requested(self): - action = EsbuildBundleAction("source", "artifacts", {"main": "x.js", "sourcemap": False}, self.osutils, self.subprocess_esbuild) + action = EsbuildBundleAction("source", "artifacts", {"entry_points": ["x.js"], "sourcemap": False}, self.osutils, self.subprocess_esbuild) action.execute() self.subprocess_esbuild.run.assert_called_with([ "x.js", @@ -272,7 +317,7 @@ def test_excludes_sourcemap_if_requested(self): ], cwd="source") def test_does_not_minify_if_requested(self): - action = EsbuildBundleAction("source", "artifacts", {"main": "x.js", "minify": False}, self.osutils, self.subprocess_esbuild) + action = EsbuildBundleAction("source", "artifacts", {"entry_points": ["x.js"], "minify": False}, self.osutils, self.subprocess_esbuild) action.execute() self.subprocess_esbuild.run.assert_called_with([ "x.js", @@ -285,10 +330,25 @@ def test_does_not_minify_if_requested(self): ], cwd="source") def test_uses_specified_target(self): - action = EsbuildBundleAction("source", "artifacts", {"main": "x.js", "target": "node14"}, self.osutils, self.subprocess_esbuild) + action = EsbuildBundleAction("source", "artifacts", {"entry_points": ["x.js"], "target": "node14"}, self.osutils, self.subprocess_esbuild) + action.execute() + self.subprocess_esbuild.run.assert_called_with([ + "x.js", + "--bundle", + "--platform=node", + "--format=cjs", + "--minify", + "--sourcemap", + "--target=node14", + "--outdir=artifacts" + ], cwd="source") + + def test_includes_multiple_entry_points_if_requested(self): + action = EsbuildBundleAction("source", "artifacts", {"entry_points": ["x.js", "y.js"], "target": "node14"}, self.osutils, self.subprocess_esbuild) action.execute() self.subprocess_esbuild.run.assert_called_with([ "x.js", + "y.js", "--bundle", "--platform=node", "--format=cjs", From a3d2bfee5feb17f7a0b94464bc43977165002c2e Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Tue, 9 Nov 2021 15:11:49 -0800 Subject: [PATCH 06/17] Update Appveyor nodejs version --- .appveyor.yml | 2 +- .../workflows/nodejs_npm/test_workflow.py | 50 ++++++++++++++++--- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index 94bf1e486..ca5967cfa 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -6,7 +6,7 @@ image: environment: GOVERSION: 1.11 GRADLE_OPTS: -Dorg.gradle.daemon=false - nodejs_version: "10.10.0" + nodejs_version: "14.18.1" matrix: - PYTHON: "C:\\Python36-x64" diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 996d5bdbe..9d80d6c0e 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -1,5 +1,3 @@ -import mock - from unittest import TestCase from mock import patch, call @@ -45,16 +43,40 @@ def setUp(self, OSUtilMock): self.osutils.is_windows.side_effect = [False] self.osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) - def test_workflow_fails_if_manifest_parsing_fails(self): + def test_workflow_sets_up_npm_actions_with_download_dependencies_without_dependencies_dir(self): + self.osutils.file_exists.return_value = True - self.osutils.parse_json.side_effect = OSError("boom!") + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) - with self.assertRaises(WorkflowFailedError) as raised: - NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + self.assertEqual(len(workflow.actions), 6) + self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) + self.assertIsInstance(workflow.actions[2], CopySourceAction) + self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) + self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) + self.assertIsInstance(workflow.actions[5], NodejsNpmLockFileCleanUpAction) - self.assertEqual(raised.exception.args[0], "NodejsNpmBuilder:ParseManifest - boom!") + def test_workflow_sets_up_npm_actions_without_download_dependencies_with_dependencies_dir(self): + self.osutils.file_exists.return_value = True - self.osutils.parse_json.assert_called_with("manifest") + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + dependencies_dir="dep", + download_dependencies=False, + osutils=self.osutils, + ) + + self.assertEqual(len(workflow.actions), 6) + + self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) + self.assertIsInstance(workflow.actions[2], CopySourceAction) + self.assertIsInstance(workflow.actions[3], CopySourceAction) + self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) + self.assertIsInstance(workflow.actions[5], NodejsNpmLockFileCleanUpAction) def test_workflow_sets_up_npm_actions_without_bundler_if_manifest_doesnt_request_it(self): @@ -67,6 +89,7 @@ def test_workflow_sets_up_npm_actions_without_bundler_if_manifest_doesnt_request self.assertIsInstance(workflow.actions[2], CopySourceAction) self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) + self.assertIsInstance(workflow.actions[5], NodejsNpmLockFileCleanUpAction) def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencies_dir(self): @@ -158,6 +181,17 @@ def test_workflow_sets_up_npm_actions_with_bundler_if_manifest_requests_it(self) [call("source/package-lock.json"), call("source/npm-shrinkwrap.json")] ) + def test_workflow_fails_if_manifest_parsing_fails(self): + + self.osutils.parse_json.side_effect = OSError("boom!") + + with self.assertRaises(WorkflowFailedError) as raised: + NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + + self.assertEqual(raised.exception.args[0], "NodejsNpmBuilder:ParseManifest - boom!") + + self.osutils.parse_json.assert_called_with("manifest") + def test_sets_up_esbuild_search_path_from_npm_bin(self): self.popen.out = b"project/bin" From 724de39c93facf9ceb30e6cc1fe55781bb2ce256 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Tue, 9 Nov 2021 15:25:25 -0800 Subject: [PATCH 07/17] Testing different node version --- .appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.appveyor.yml b/.appveyor.yml index ca5967cfa..a0b631ef1 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -6,7 +6,7 @@ image: environment: GOVERSION: 1.11 GRADLE_OPTS: -Dorg.gradle.daemon=false - nodejs_version: "14.18.1" + nodejs_version: "14.1.0" matrix: - PYTHON: "C:\\Python36-x64" From 5993d55c924926eeeb68aa69196ffcc21fe27e63 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Tue, 9 Nov 2021 15:54:28 -0800 Subject: [PATCH 08/17] Update npm to version that supports node14 --- .appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index a0b631ef1..67aa9f515 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -6,7 +6,7 @@ image: environment: GOVERSION: 1.11 GRADLE_OPTS: -Dorg.gradle.daemon=false - nodejs_version: "14.1.0" + nodejs_version: "14.17.6" matrix: - PYTHON: "C:\\Python36-x64" @@ -93,7 +93,7 @@ for: - sh: "source ${HOME}/venv${PYTHON_VERSION}/bin/activate" - sh: "rvm use 2.5" - sh: "nvm install ${nodejs_version}" - - sh: "npm install npm@5.6.0 -g" + - sh: "npm install npm@7.24.2 -g" - sh: "npm -v" - sh: "echo $PATH" - sh: "java --version" From 2e5bf3d8ab4bcee164527a321125d7a9518c15ce Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Tue, 9 Nov 2021 16:27:26 -0800 Subject: [PATCH 09/17] Cleanup garbage package lock in dependencies dir generated by npm 7 --- aws_lambda_builders/workflows/nodejs_npm/workflow.py | 3 +++ tests/unit/workflows/nodejs_npm/test_workflow.py | 9 ++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 9944161d6..7f7224537 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -84,6 +84,9 @@ def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manife actions.append(NodejsNpmrcCleanUpAction(artifacts_dir, osutils=osutils)) actions.append(NodejsNpmLockFileCleanUpAction(artifacts_dir, osutils=osutils)) + if self.dependencies_dir: + actions.append(NodejsNpmLockFileCleanUpAction(self.dependencies_dir, osutils=osutils)) + return actions def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutils, subprocess_npm): diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 9d80d6c0e..4b2a94376 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -69,7 +69,7 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_with_depende osutils=self.osutils, ) - self.assertEqual(len(workflow.actions), 6) + self.assertEqual(len(workflow.actions), 7) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) @@ -77,6 +77,7 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_with_depende self.assertIsInstance(workflow.actions[3], CopySourceAction) self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) self.assertIsInstance(workflow.actions[5], NodejsNpmLockFileCleanUpAction) + self.assertIsInstance(workflow.actions[6], NodejsNpmLockFileCleanUpAction) def test_workflow_sets_up_npm_actions_without_bundler_if_manifest_doesnt_request_it(self): @@ -105,7 +106,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencie osutils=self.osutils, ) - self.assertEqual(len(workflow.actions), 8) + self.assertEqual(len(workflow.actions), 9) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) @@ -115,6 +116,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencie self.assertIsInstance(workflow.actions[5], CopyDependenciesAction) self.assertIsInstance(workflow.actions[6], NodejsNpmrcCleanUpAction) self.assertIsInstance(workflow.actions[7], NodejsNpmLockFileCleanUpAction) + self.assertIsInstance(workflow.actions[8], NodejsNpmLockFileCleanUpAction) def test_workflow_sets_up_npm_actions_without_download_dependencies_and_without_dependencies_dir(self): workflow = NodejsNpmWorkflow( @@ -150,7 +152,7 @@ def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): osutils=self.osutils, ) - self.assertEqual(len(workflow.actions), 8) + self.assertEqual(len(workflow.actions), 9) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) @@ -160,6 +162,7 @@ def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): self.assertIsInstance(workflow.actions[5], MoveDependenciesAction) self.assertIsInstance(workflow.actions[6], NodejsNpmrcCleanUpAction) self.assertIsInstance(workflow.actions[7], NodejsNpmLockFileCleanUpAction) + self.assertIsInstance(workflow.actions[8], NodejsNpmLockFileCleanUpAction) def test_workflow_sets_up_npm_actions_with_bundler_if_manifest_requests_it(self): From 3329cc0081c5aca7ddd9b6c194076a49adf09afe Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Wed, 5 Jan 2022 17:05:15 -0800 Subject: [PATCH 10/17] PEP8 compliance and missing doc strings --- .../workflows/nodejs_npm/actions.py | 16 ++- .../workflows/nodejs_npm/esbuild.py | 2 +- .../workflows/nodejs_npm/workflow.py | 111 +++++++++++++----- 3 files changed, 92 insertions(+), 37 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index c90015476..38ab4d523 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -338,7 +338,7 @@ def execute(self): if not isinstance(entry_points, list): raise ActionFailedError("entry_points must be a list ({})".format(self.bundler_config)) - if len(entry_points) < 1: + if not entry_points: raise ActionFailedError("entry_points must not be empty ({})".format(self.bundler_config)) entry_paths = [self.osutils.joinpath(self.source_dir, entry_point) for entry_point in entry_points] @@ -350,16 +350,14 @@ def execute(self): raise ActionFailedError("entry point {} does not exist".format(entry_point)) args = entry_points + ["--bundle", "--platform=node", "--format=cjs"] - skip_minify = "minify" in self.bundler_config and not self.bundler_config["minify"] - skip_sourcemap = "sourcemap" in self.bundler_config and not self.bundler_config["sourcemap"] - if not skip_minify: + minify = self.bundler_config.get("minify", True) + sourcemap = self.bundler_config.get("sourcemap", True) + target = self.bundler_config.get("target", "es2020") + if minify: args.append("--minify") - if not skip_sourcemap: + if sourcemap: args.append("--sourcemap") - if "target" not in self.bundler_config: - args.append("--target=es2020") - else: - args.append("--target={}".format(self.bundler_config["target"])) + args.append("--target={}".format(target)) args.append("--outdir={}".format(self.artifacts_dir)) try: self.subprocess_esbuild.run(args, cwd=self.source_dir) diff --git a/aws_lambda_builders/workflows/nodejs_npm/esbuild.py b/aws_lambda_builders/workflows/nodejs_npm/esbuild.py index fc39ecd45..866027c01 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/esbuild.py +++ b/aws_lambda_builders/workflows/nodejs_npm/esbuild.py @@ -55,7 +55,7 @@ def esbuild_binary(self): binaries = self.which("esbuild", executable_search_paths=self.executable_search_paths) LOG.debug("potential esbuild binaries: %s", binaries) - if len(binaries) > 0: + if binaries: return binaries[0] else: raise EsbuildExecutionError(message="cannot find esbuild") diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 7f7224537..427dd6c67 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -41,7 +41,58 @@ class NodejsNpmWorkflow(BaseWorkflow): CONFIG_PROPERTY = "aws_sam" + def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): + + super(NodejsNpmWorkflow, self).__init__( + source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=runtime, **kwargs + ) + + if osutils is None: + osutils = OSUtils() + + if not osutils.file_exists(manifest_path): + LOG.warning("package.json file not found. Continuing the build without dependencies.") + self.actions = [CopySourceAction(source_dir, artifacts_dir, excludes=self.EXCLUDED_FILES)] + return + + subprocess_npm = SubprocessNpm(osutils) + + manifest_config = self.get_manifest_config(osutils, manifest_path) + + if manifest_config["bundler"] == "esbuild": + self.actions = self.actions_with_bundler( + source_dir, artifacts_dir, manifest_config, osutils, subprocess_npm + ) + else: + self.actions = self.actions_without_bundler( + source_dir, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm + ) + def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm): + """ + Generate a list of Nodejs build actions without a bundler + + :type source_dir: str + :param source_dir: an existing (readable) directory containing source files + + :type artifacts_dir: str + :param artifacts_dir: an existing (writable) directory where to store the output. + + :type scratch_dir: str + :param scratch_dir: an existing (writable) directory for temporary files + + :type manifest_path: str + :param manifest_path: path to package.json of an NPM project with the source to pack + + :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils + :param osutils: An instance of OS Utilities for file manipulation + + :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm + :param subprocess_npm: An instance of the NPM process wrapper + + :rtype: list + :return: List of build actions to execute + """ tar_dest_dir = osutils.joinpath(scratch_dir, "unpacked") tar_package_dir = osutils.joinpath(tar_dest_dir, "package") npm_pack = NodejsNpmPackAction( @@ -90,6 +141,27 @@ def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manife return actions def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutils, subprocess_npm): + """ + Generate a list of Nodejs build actions with a bundler + + :type source_dir: str + :param source_dir: an existing (readable) directory containing source files + + :type artifacts_dir: str + :param artifacts_dir: an existing (writable) directory where to store the output. + + :type bundler_config: dict + :param bundler_config: configurations for the bundler action + + :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils + :param osutils: An instance of OS Utilities for file manipulation + + :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm + :param subprocess_npm: An instance of the NPM process wrapper + + :rtype: list + :return: List of build actions to execute + """ lockfile_path = osutils.joinpath(source_dir, "package-lock.json") shrinkwrap_path = osutils.joinpath(source_dir, "npm-shrinkwrap.json") npm_bin_path = subprocess_npm.run(["bin"], cwd=source_dir) @@ -107,6 +179,18 @@ def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutil return [install_action, esbuild_action] def get_manifest_config(self, osutils, manifest_path): + """ + Get the aws_sam specific properties from the manifest, if they exist. + + :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils + :param osutils: An instance of OS Utilities for file manipulation + + :type manifest_path: str + :param manifest_path: Path to the manifest file + + :rtype: dict + :return: Dict with aws_sam specific bundler configs + """ LOG.debug("NODEJS reading manifest from %s", manifest_path) try: manifest = osutils.parse_json(manifest_path) @@ -117,33 +201,6 @@ def get_manifest_config(self, osutils, manifest_path): except (OSError, json.decoder.JSONDecodeError) as ex: raise WorkflowFailedError(workflow_name=self.NAME, action_name="ParseManifest", reason=str(ex)) - def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): - - super(NodejsNpmWorkflow, self).__init__( - source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=runtime, **kwargs - ) - - if osutils is None: - osutils = OSUtils() - - if not osutils.file_exists(manifest_path): - LOG.warning("package.json file not found. Continuing the build without dependencies.") - self.actions = [CopySourceAction(source_dir, artifacts_dir, excludes=self.EXCLUDED_FILES)] - return - - subprocess_npm = SubprocessNpm(osutils) - - manifest_config = self.get_manifest_config(osutils, manifest_path) - - if manifest_config["bundler"] == "esbuild": - self.actions = self.actions_with_bundler( - source_dir, artifacts_dir, manifest_config, osutils, subprocess_npm - ) - else: - self.actions = self.actions_without_bundler( - source_dir, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm - ) - def get_resolvers(self): """ specialized path resolver that just returns the list of executable for the runtime on the path. From b2cb1ae54acca73d9309c2e915a7fa92cee8ef16 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Thu, 6 Jan 2022 13:24:34 -0800 Subject: [PATCH 11/17] Add support for auto dep layer and incremental build with esbuild --- .../workflows/nodejs_npm/workflow.py | 46 +++++++++-- .../test_nodejs_npm_with_esbuild.py | 31 ++++++- .../workflows/nodejs_npm/test_workflow.py | 81 +++++++++++++++++++ 3 files changed, 149 insertions(+), 9 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 427dd6c67..026906d5b 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -4,10 +4,17 @@ import logging import json +from typing import List from aws_lambda_builders.path_resolver import PathResolver from aws_lambda_builders.workflow import BaseWorkflow, Capability -from aws_lambda_builders.actions import CopySourceAction, CleanUpAction, CopyDependenciesAction, MoveDependenciesAction +from aws_lambda_builders.actions import ( + CopySourceAction, + CleanUpAction, + CopyDependenciesAction, + MoveDependenciesAction, + BaseAction, +) from aws_lambda_builders.utils import which from aws_lambda_builders.exceptions import WorkflowFailedError from .actions import ( @@ -61,7 +68,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim if manifest_config["bundler"] == "esbuild": self.actions = self.actions_with_bundler( - source_dir, artifacts_dir, manifest_config, osutils, subprocess_npm + source_dir, artifacts_dir, scratch_dir, manifest_path, manifest_config, osutils, subprocess_npm ) else: self.actions = self.actions_without_bundler( @@ -140,7 +147,9 @@ def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manife return actions - def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutils, subprocess_npm): + def actions_with_bundler( + self, source_dir, artifacts_dir, scratch_dir, manifest_path, bundler_config, osutils, subprocess_npm + ): """ Generate a list of Nodejs build actions with a bundler @@ -150,6 +159,12 @@ def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutil :type artifacts_dir: str :param artifacts_dir: an existing (writable) directory where to store the output. + :type scratch_dir: str + :param scratch_dir: an existing (writable) directory for temporary files + + :type manifest_path: str + :param manifest_path: path to package.json of an NPM project with the source to pack + :type bundler_config: dict :param bundler_config: configurations for the bundler action @@ -175,8 +190,29 @@ def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutil else: install_action = NodejsNpmInstallAction(source_dir, subprocess_npm=subprocess_npm, is_production=False) - esbuild_action = EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) - return [install_action, esbuild_action] + actions = [] + + if self.download_dependencies: + actions.append(install_action) + if self.dependencies_dir: + if self.combine_dependencies: + # If combine_dependencies, bundle the code then copy bundled source and + # dependencies from artifacts dir to dep dir + actions.append(CleanUpAction(self.dependencies_dir)) + actions.append( + EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) + ) + actions.append(CopySourceAction(artifacts_dir, self.dependencies_dir, excludes=self.EXCLUDED_FILES)) + return actions + else: + # In order for auto dependency layer to work for sam accelerate local testing, + # we need to forgo bundling in this case + return self.actions_without_bundler( + source_dir, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm + ) + + actions.append(EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild)) + return actions def get_manifest_config(self, osutils, manifest_path): """ diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py index 46cffcd0a..5622f3857 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py @@ -17,6 +17,7 @@ class TestNodejsNpmWorkflowWithEsbuild(TestCase): def setUp(self): self.artifacts_dir = tempfile.mkdtemp() self.scratch_dir = tempfile.mkdtemp() + self.dependencies_dir = tempfile.mkdtemp() self.no_deps = os.path.join(self.TEST_DATA_FOLDER, "no-deps-esbuild") @@ -26,8 +27,9 @@ def setUp(self): def tearDown(self): shutil.rmtree(self.artifacts_dir) shutil.rmtree(self.scratch_dir) + shutil.rmtree(self.dependencies_dir) - def testBuildsJavascriptProjectWithDependencies(self): + def test_builds_javascript_project_with_dependencies(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild") self.builder.build( @@ -42,7 +44,7 @@ def testBuildsJavascriptProjectWithDependencies(self): output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) - def testBuildsJavascriptProjectWithMultipleEntrypoints(self): + def test_builds_javascript_project_with_multiple_entrypoints(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild-multiple-entrypoints") self.builder.build( @@ -57,7 +59,7 @@ def testBuildsJavascriptProjectWithMultipleEntrypoints(self): output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) - def testBuildsTypescriptProjects(self): + def test_builds_typescript_projects(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild-typescript") self.builder.build( @@ -72,7 +74,7 @@ def testBuildsTypescriptProjects(self): output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) - def testBuildsWithExternalEsbuild(self): + def test_builds_with_external_esbuild(self): osutils = OSUtils() npm = SubprocessNpm(osutils) source_dir = os.path.join(self.TEST_DATA_FOLDER, "no-deps-esbuild") @@ -94,3 +96,24 @@ def testBuildsWithExternalEsbuild(self): expected_files = {"included.js", "included.js.map"} output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) + + def test_with_download_dependencies_and_dependencies_dir(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + dependencies_dir=self.dependencies_dir, + download_dependencies=True, + ) + + expected_files = {"included.js.map", "included.js"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + expected_modules = {"included.js.map", "included.js"} + output_modules = set(os.listdir(os.path.join(self.dependencies_dir))) + self.assertEqual(expected_modules, output_modules) diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 4b2a94376..51279734d 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -264,6 +264,87 @@ def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): [call("source/package-lock.json"), call("source/npm-shrinkwrap.json")] ) + def test_workflow_sets_up_npm_actions_without_download_dependencies_with_dependencies_dir_with_bundler(self): + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] + self.osutils.file_exists.side_effect = [True, False, True] + + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + dependencies_dir="dep", + download_dependencies=False, + osutils=self.osutils, + ) + + self.assertEqual(len(workflow.actions), 1) + self.assertIsInstance(workflow.actions[0], EsbuildBundleAction) + + def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencies_dir_with_bundler(self): + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] + self.osutils.file_exists.side_effect = [True, False, True] + + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + dependencies_dir="dep", + download_dependencies=True, + osutils=self.osutils, + ) + + self.assertEqual(len(workflow.actions), 4) + self.assertIsInstance(workflow.actions[0], NodejsNpmCIAction) + self.assertIsInstance(workflow.actions[1], CleanUpAction) + self.assertIsInstance(workflow.actions[2], EsbuildBundleAction) + self.assertIsInstance(workflow.actions[3], CopySourceAction) + + def test_workflow_sets_up_npm_actions_without_download_dependencies_and_without_dependencies_dir_with_bundler(self): + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] + self.osutils.file_exists.side_effect = [True, False, True] + + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + dependencies_dir=None, + download_dependencies=False, + osutils=self.osutils, + ) + + self.assertEqual(len(workflow.actions), 1) + self.assertIsInstance(workflow.actions[0], EsbuildBundleAction) + + def test_workflow_sets_up_npm_actions_without_combine_dependencies_with_bundler(self): + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] + self.osutils.file_exists.side_effect = [True, False, True, True] + + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + dependencies_dir="dep", + download_dependencies=True, + combine_dependencies=False, + osutils=self.osutils, + ) + + self.assertEqual(len(workflow.actions), 9) + + self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) + self.assertIsInstance(workflow.actions[2], CopySourceAction) + self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) + self.assertIsInstance(workflow.actions[4], CleanUpAction) + self.assertIsInstance(workflow.actions[5], MoveDependenciesAction) + self.assertIsInstance(workflow.actions[6], NodejsNpmrcCleanUpAction) + self.assertIsInstance(workflow.actions[7], NodejsNpmLockFileCleanUpAction) + self.assertIsInstance(workflow.actions[8], NodejsNpmLockFileCleanUpAction) + def test_must_validate_architecture(self): self.osutils.is_windows.side_effect = [False, False] workflow = NodejsNpmWorkflow( From 01562e7a826b35d3e36b98d9e0bb84391b77ae48 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Tue, 11 Jan 2022 09:26:28 -0800 Subject: [PATCH 12/17] Fix documentation --- aws_lambda_builders/workflows/nodejs_npm/DESIGN.md | 2 +- aws_lambda_builders/workflows/nodejs_npm/actions.py | 4 ++-- aws_lambda_builders/workflows/nodejs_npm/esbuild.py | 8 ++++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md index de3800617..b947573a9 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md +++ b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md @@ -82,7 +82,7 @@ Lambda environment. ### Choosing the packaging type -For a large majority of projects, packaging is using a bundler has significant +For a large majority of projects, packaging using a bundler has significant advantages (speed and runtime package size, supporting local dependencies). However, there are also some drawbacks to using a bundler for a small set of diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index 38ab4d523..cb220d7f3 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -90,8 +90,8 @@ def __init__(self, artifacts_dir, subprocess_npm, is_production=True): :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm :param subprocess_npm: An instance of the NPM process wrapper - :type mode: str - :param mode: NPM installation mode (eg --production=false to force dev dependencies) + :type is_production: bool + :param is_production: NPM installation mode is production (eg --production=false to force dev dependencies) """ super(NodejsNpmInstallAction, self).__init__() diff --git a/aws_lambda_builders/workflows/nodejs_npm/esbuild.py b/aws_lambda_builders/workflows/nodejs_npm/esbuild.py index 866027c01..992c49fe5 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/esbuild.py +++ b/aws_lambda_builders/workflows/nodejs_npm/esbuild.py @@ -32,9 +32,13 @@ 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 bin_path: str - :param bin_path: Path to the NPM package binary utilities. This will + :type executable_search_paths: list + :param executable_search_paths: List of paths to the NPM package binary utilities. This will be used to find embedded esbuild 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 From 3ef8ef295e4a704b0d6ed7e0fefef144d554a481 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Thu, 20 Jan 2022 11:28:39 -0800 Subject: [PATCH 13/17] Revert accelerate feature changes --- .../workflows/nodejs_npm/workflow.py | 37 +-------- .../test_nodejs_npm_with_esbuild.py | 23 ------ .../workflows/nodejs_npm/test_workflow.py | 81 ------------------- 3 files changed, 4 insertions(+), 137 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 026906d5b..9ee7ada44 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -68,7 +68,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim if manifest_config["bundler"] == "esbuild": self.actions = self.actions_with_bundler( - source_dir, artifacts_dir, scratch_dir, manifest_path, manifest_config, osutils, subprocess_npm + source_dir, artifacts_dir, manifest_config, osutils, subprocess_npm ) else: self.actions = self.actions_without_bundler( @@ -147,9 +147,7 @@ def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manife return actions - def actions_with_bundler( - self, source_dir, artifacts_dir, scratch_dir, manifest_path, bundler_config, osutils, subprocess_npm - ): + def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutils, subprocess_npm): """ Generate a list of Nodejs build actions with a bundler @@ -159,12 +157,6 @@ def actions_with_bundler( :type artifacts_dir: str :param artifacts_dir: an existing (writable) directory where to store the output. - :type scratch_dir: str - :param scratch_dir: an existing (writable) directory for temporary files - - :type manifest_path: str - :param manifest_path: path to package.json of an NPM project with the source to pack - :type bundler_config: dict :param bundler_config: configurations for the bundler action @@ -190,29 +182,8 @@ def actions_with_bundler( else: install_action = NodejsNpmInstallAction(source_dir, subprocess_npm=subprocess_npm, is_production=False) - actions = [] - - if self.download_dependencies: - actions.append(install_action) - if self.dependencies_dir: - if self.combine_dependencies: - # If combine_dependencies, bundle the code then copy bundled source and - # dependencies from artifacts dir to dep dir - actions.append(CleanUpAction(self.dependencies_dir)) - actions.append( - EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) - ) - actions.append(CopySourceAction(artifacts_dir, self.dependencies_dir, excludes=self.EXCLUDED_FILES)) - return actions - else: - # In order for auto dependency layer to work for sam accelerate local testing, - # we need to forgo bundling in this case - return self.actions_without_bundler( - source_dir, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm - ) - - actions.append(EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild)) - return actions + esbuild_action = EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) + return [install_action, esbuild_action] def get_manifest_config(self, osutils, manifest_path): """ diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py index 5622f3857..e78931da2 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py @@ -17,7 +17,6 @@ class TestNodejsNpmWorkflowWithEsbuild(TestCase): def setUp(self): self.artifacts_dir = tempfile.mkdtemp() self.scratch_dir = tempfile.mkdtemp() - self.dependencies_dir = tempfile.mkdtemp() self.no_deps = os.path.join(self.TEST_DATA_FOLDER, "no-deps-esbuild") @@ -27,7 +26,6 @@ def setUp(self): def tearDown(self): shutil.rmtree(self.artifacts_dir) shutil.rmtree(self.scratch_dir) - shutil.rmtree(self.dependencies_dir) def test_builds_javascript_project_with_dependencies(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild") @@ -96,24 +94,3 @@ def test_builds_with_external_esbuild(self): expected_files = {"included.js", "included.js.map"} output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) - - def test_with_download_dependencies_and_dependencies_dir(self): - source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild") - - self.builder.build( - source_dir, - self.artifacts_dir, - self.scratch_dir, - os.path.join(source_dir, "package.json"), - runtime=self.runtime, - dependencies_dir=self.dependencies_dir, - download_dependencies=True, - ) - - expected_files = {"included.js.map", "included.js"} - output_files = set(os.listdir(self.artifacts_dir)) - self.assertEqual(expected_files, output_files) - - expected_modules = {"included.js.map", "included.js"} - output_modules = set(os.listdir(os.path.join(self.dependencies_dir))) - self.assertEqual(expected_modules, output_modules) diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 51279734d..4b2a94376 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -264,87 +264,6 @@ def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): [call("source/package-lock.json"), call("source/npm-shrinkwrap.json")] ) - def test_workflow_sets_up_npm_actions_without_download_dependencies_with_dependencies_dir_with_bundler(self): - self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] - self.osutils.file_exists.side_effect = [True, False, True] - - workflow = NodejsNpmWorkflow( - "source", - "artifacts", - "scratch_dir", - "manifest", - dependencies_dir="dep", - download_dependencies=False, - osutils=self.osutils, - ) - - self.assertEqual(len(workflow.actions), 1) - self.assertIsInstance(workflow.actions[0], EsbuildBundleAction) - - def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencies_dir_with_bundler(self): - self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] - self.osutils.file_exists.side_effect = [True, False, True] - - workflow = NodejsNpmWorkflow( - "source", - "artifacts", - "scratch_dir", - "manifest", - dependencies_dir="dep", - download_dependencies=True, - osutils=self.osutils, - ) - - self.assertEqual(len(workflow.actions), 4) - self.assertIsInstance(workflow.actions[0], NodejsNpmCIAction) - self.assertIsInstance(workflow.actions[1], CleanUpAction) - self.assertIsInstance(workflow.actions[2], EsbuildBundleAction) - self.assertIsInstance(workflow.actions[3], CopySourceAction) - - def test_workflow_sets_up_npm_actions_without_download_dependencies_and_without_dependencies_dir_with_bundler(self): - self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] - self.osutils.file_exists.side_effect = [True, False, True] - - workflow = NodejsNpmWorkflow( - "source", - "artifacts", - "scratch_dir", - "manifest", - dependencies_dir=None, - download_dependencies=False, - osutils=self.osutils, - ) - - self.assertEqual(len(workflow.actions), 1) - self.assertIsInstance(workflow.actions[0], EsbuildBundleAction) - - def test_workflow_sets_up_npm_actions_without_combine_dependencies_with_bundler(self): - self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] - self.osutils.file_exists.side_effect = [True, False, True, True] - - workflow = NodejsNpmWorkflow( - "source", - "artifacts", - "scratch_dir", - "manifest", - dependencies_dir="dep", - download_dependencies=True, - combine_dependencies=False, - osutils=self.osutils, - ) - - self.assertEqual(len(workflow.actions), 9) - - self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) - self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) - self.assertIsInstance(workflow.actions[2], CopySourceAction) - self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) - self.assertIsInstance(workflow.actions[4], CleanUpAction) - self.assertIsInstance(workflow.actions[5], MoveDependenciesAction) - self.assertIsInstance(workflow.actions[6], NodejsNpmrcCleanUpAction) - self.assertIsInstance(workflow.actions[7], NodejsNpmLockFileCleanUpAction) - self.assertIsInstance(workflow.actions[8], NodejsNpmLockFileCleanUpAction) - def test_must_validate_architecture(self): self.osutils.is_windows.side_effect = [False, False] workflow = NodejsNpmWorkflow( From 045decda658ec53c3d161641032528e443a7f2c7 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Fri, 21 Jan 2022 16:53:04 -0800 Subject: [PATCH 14/17] Add experimental feature flag, route all requests to old workflow --- tests/unit/workflows/nodejs_npm/test_utils.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/unit/workflows/nodejs_npm/test_utils.py diff --git a/tests/unit/workflows/nodejs_npm/test_utils.py b/tests/unit/workflows/nodejs_npm/test_utils.py new file mode 100644 index 000000000..e69de29bb From ffec02aa5e2b305070ebb8bb945405fb47b831dd Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Fri, 21 Jan 2022 16:54:13 -0800 Subject: [PATCH 15/17] Add experimental feature flag, route all requests to old workflow --- aws_lambda_builders/builder.py | 7 ++++ aws_lambda_builders/workflow.py | 4 ++ .../workflows/nodejs_npm/utils.py | 9 +++++ .../workflows/nodejs_npm/workflow.py | 4 +- .../test_nodejs_npm_with_esbuild.py | 21 +++++++++- tests/unit/test_builder.py | 39 ++++++++++++++----- tests/unit/workflows/nodejs_npm/test_utils.py | 17 ++++++++ .../workflows/nodejs_npm/test_workflow.py | 38 ++++++++++++++++-- 8 files changed, 122 insertions(+), 17 deletions(-) diff --git a/aws_lambda_builders/builder.py b/aws_lambda_builders/builder.py index f6524c659..bcacb67f7 100644 --- a/aws_lambda_builders/builder.py +++ b/aws_lambda_builders/builder.py @@ -69,7 +69,9 @@ def build( dependencies_dir=None, combine_dependencies=True, architecture=X86_64, + experimental_flags=None, ): + # pylint: disable-msg=too-many-locals """ Actually build the code by running workflows @@ -127,6 +129,10 @@ def build( :type architecture: str :param architecture: Type of architecture x86_64 and arm64 for Lambda Function + + :type experimental_flags: list + :param experimental_flags: + List of strings, which will indicate enabled experimental flags for the current build session """ if not os.path.exists(scratch_dir): @@ -146,6 +152,7 @@ def build( dependencies_dir=dependencies_dir, combine_dependencies=combine_dependencies, architecture=architecture, + experimental_flags=experimental_flags, ) return workflow.run() diff --git a/aws_lambda_builders/workflow.py b/aws_lambda_builders/workflow.py index 7ff19534c..e979a8392 100644 --- a/aws_lambda_builders/workflow.py +++ b/aws_lambda_builders/workflow.py @@ -164,6 +164,7 @@ def __init__( dependencies_dir=None, combine_dependencies=True, architecture=X86_64, + experimental_flags=None, ): """ Initialize the builder with given arguments. These arguments together form the "public API" that each @@ -200,6 +201,8 @@ def __init__( from dependency_folder into build folder architecture : str, optional Architecture type either arm64 or x86_64 for which the build will be based on in AWS lambda, by default X86_64 + experimental_flags: list, optional + List of strings, which will indicate enabled experimental flags for the current build session """ self.source_dir = source_dir @@ -215,6 +218,7 @@ def __init__( self.dependencies_dir = dependencies_dir self.combine_dependencies = combine_dependencies self.architecture = architecture + self.experimental_flags = experimental_flags if experimental_flags else [] # Actions are registered by the subclasses as they seem fit self.actions = [] diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py index aebfb3b7b..dc114ba06 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/utils.py +++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py @@ -9,6 +9,8 @@ import shutil import json +EXPERIMENTAL_FLAG_ESBUILD = "experimentalEsbuild" + class OSUtils(object): @@ -53,3 +55,10 @@ def is_windows(self): def parse_json(self, path): with open(path) as json_file: return json.load(json_file) + + +def is_experimental_esbuild_scope(experimental_flags): + """ + A function which will determine if experimental esbuild scope is active + """ + return bool(experimental_flags) and EXPERIMENTAL_FLAG_ESBUILD in experimental_flags diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 9ee7ada44..17133be12 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -26,7 +26,7 @@ NodejsNpmCIAction, EsbuildBundleAction, ) -from .utils import OSUtils +from .utils import OSUtils, EXPERIMENTAL_FLAG_ESBUILD, is_experimental_esbuild_scope from .npm import SubprocessNpm from .esbuild import SubprocessEsbuild @@ -66,7 +66,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim manifest_config = self.get_manifest_config(osutils, manifest_path) - if manifest_config["bundler"] == "esbuild": + if manifest_config["bundler"] == "esbuild" and is_experimental_esbuild_scope(self.experimental_flags): self.actions = self.actions_with_bundler( source_dir, artifacts_dir, manifest_config, osutils, subprocess_npm ) diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py index e78931da2..01f225198 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py @@ -4,7 +4,7 @@ from unittest import TestCase from aws_lambda_builders.builder import LambdaBuilder from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm -from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils +from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils, EXPERIMENTAL_FLAG_ESBUILD class TestNodejsNpmWorkflowWithEsbuild(TestCase): @@ -27,6 +27,21 @@ def tearDown(self): shutil.rmtree(self.artifacts_dir) shutil.rmtree(self.scratch_dir) + def test_invokes_old_builder_without_feature_flag(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + ) + + expected_files = {'included.js', 'node_modules', 'excluded.js', 'package.json'} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + def test_builds_javascript_project_with_dependencies(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild") @@ -36,6 +51,7 @@ def test_builds_javascript_project_with_dependencies(self): self.scratch_dir, os.path.join(source_dir, "package.json"), runtime=self.runtime, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) expected_files = {"included.js", "included.js.map"} @@ -51,6 +67,7 @@ def test_builds_javascript_project_with_multiple_entrypoints(self): self.scratch_dir, os.path.join(source_dir, "package.json"), runtime=self.runtime, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) expected_files = {"included.js", "included.js.map", "included2.js", "included2.js.map"} @@ -66,6 +83,7 @@ def test_builds_typescript_projects(self): self.scratch_dir, os.path.join(source_dir, "package.json"), runtime=self.runtime, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) expected_files = {"included.js", "included.js.map"} @@ -89,6 +107,7 @@ def test_builds_with_external_esbuild(self): os.path.join(source_dir, "package.json"), runtime=self.runtime, executable_search_paths=[binpath], + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) expected_files = {"included.js", "included.js.map"} diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 5983a1810..2dd9b63ed 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -1,3 +1,4 @@ +import itertools from unittest import TestCase from mock import patch, call, Mock from parameterized import parameterized, param @@ -108,7 +109,7 @@ def __init__( self.assertEqual(builder.selected_workflow_cls, MyWorkflow) -class TesetLambdaBuilder_build(TestCase): +class TestLambdaBuilder_build(TestCase): def tearDown(self): # we don't want test classes lurking around and interfere with other tests DEFAULT_REGISTRY.clear() @@ -118,11 +119,27 @@ def setUp(self): self.lang_framework = "pip" self.app_framework = "chalice" - @parameterized.expand([param(True), param(False)]) + @parameterized.expand( + itertools.product( + [True, False], # scratch_dir_exists + [True, False], # download_dependencies + [None, "dependency_dir"], # dependency_dir + [True, False], # combine_dependencies + [None, [], ["a", "b"]], # experimental flags + ) + ) @patch("aws_lambda_builders.builder.os") - @patch("aws_lambda_builders.builder.importlib") @patch("aws_lambda_builders.builder.get_workflow") - def test_with_mocks(self, scratch_dir_exists, get_workflow_mock, importlib_mock, os_mock): + def test_with_mocks( + self, + scratch_dir_exists, + download_dependencies, + dependency_dir, + combine_dependencies, + experimental_flags, + get_workflow_mock, + os_mock, + ): workflow_cls = Mock() workflow_instance = workflow_cls.return_value = Mock() @@ -143,9 +160,10 @@ def test_with_mocks(self, scratch_dir_exists, get_workflow_mock, importlib_mock, options="options", executable_search_paths="executable_search_paths", mode=None, - download_dependencies=False, - dependencies_dir="dependency_folder", - combine_dependencies=False, + download_dependencies=download_dependencies, + dependencies_dir=dependency_dir, + combine_dependencies=combine_dependencies, + experimental_flags=experimental_flags, ) workflow_cls.assert_called_with( @@ -159,9 +177,10 @@ def test_with_mocks(self, scratch_dir_exists, get_workflow_mock, importlib_mock, options="options", executable_search_paths="executable_search_paths", mode=None, - download_dependencies=False, - dependencies_dir="dependency_folder", - combine_dependencies=False, + download_dependencies=download_dependencies, + dependencies_dir=dependency_dir, + combine_dependencies=combine_dependencies, + experimental_flags=experimental_flags, ) workflow_instance.run.assert_called_once() os_mock.path.exists.assert_called_once_with("scratch_dir") diff --git a/tests/unit/workflows/nodejs_npm/test_utils.py b/tests/unit/workflows/nodejs_npm/test_utils.py index e69de29bb..80d92a9a5 100644 --- a/tests/unit/workflows/nodejs_npm/test_utils.py +++ b/tests/unit/workflows/nodejs_npm/test_utils.py @@ -0,0 +1,17 @@ +from unittest import TestCase +from parameterized import parameterized + +from aws_lambda_builders.workflows.nodejs_npm.utils import EXPERIMENTAL_FLAG_ESBUILD, is_experimental_esbuild_scope + + +class TestNodejsUtils(TestCase): + @parameterized.expand( + [ + (None, False), + ([], False), + ([EXPERIMENTAL_FLAG_ESBUILD], True), + ([EXPERIMENTAL_FLAG_ESBUILD, "SomeOtherFlag"], True), + ] + ) + def test_experimental_esbuild_scope_check(self, experimental_flags, expected): + self.assertEqual(is_experimental_esbuild_scope(experimental_flags), expected) diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 4b2a94376..9d2469f70 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -15,6 +15,7 @@ NodejsNpmCIAction, EsbuildBundleAction, ) +from aws_lambda_builders.workflows.nodejs_npm.utils import EXPERIMENTAL_FLAG_ESBUILD class FakePopen: @@ -170,7 +171,14 @@ def test_workflow_sets_up_npm_actions_with_bundler_if_manifest_requests_it(self) self.osutils.file_exists.side_effect = [True, False, False] - workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) self.assertEqual(len(workflow.actions), 2) @@ -200,7 +208,14 @@ def test_sets_up_esbuild_search_path_from_npm_bin(self): self.popen.out = b"project/bin" self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] - workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) self.osutils.popen.assert_called_with(["npm", "bin"], stdout="PIPE", stderr="PIPE", cwd="source") @@ -222,6 +237,7 @@ def test_sets_up_esbuild_search_path_with_workflow_executable_search_paths_after "manifest", osutils=self.osutils, executable_search_paths=["other/bin"], + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) self.osutils.popen.assert_called_with(["npm", "bin"], stdout="PIPE", stderr="PIPE", cwd="source") @@ -237,7 +253,14 @@ def test_workflow_uses_npm_ci_if_lockfile_exists(self): self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] self.osutils.file_exists.side_effect = [True, True] - workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) self.assertEqual(len(workflow.actions), 2) @@ -252,7 +275,14 @@ def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] self.osutils.file_exists.side_effect = [True, False, True] - workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) self.assertEqual(len(workflow.actions), 2) From fd6a453edad4b1118021f60c9b1e26c40d7a0d12 Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Fri, 21 Jan 2022 16:55:23 -0800 Subject: [PATCH 16/17] Remove unused variable --- aws_lambda_builders/workflows/nodejs_npm/workflow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 17133be12..b8f209c20 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -26,7 +26,7 @@ NodejsNpmCIAction, EsbuildBundleAction, ) -from .utils import OSUtils, EXPERIMENTAL_FLAG_ESBUILD, is_experimental_esbuild_scope +from .utils import OSUtils, is_experimental_esbuild_scope from .npm import SubprocessNpm from .esbuild import SubprocessEsbuild From 55370cd6bd40d75c0762595e2e749f336ac8bdbc Mon Sep 17 00:00:00 2001 From: Daniel Mil Date: Fri, 21 Jan 2022 17:27:45 -0800 Subject: [PATCH 17/17] Black reformat --- .../workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py index 01f225198..0de36f41d 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py @@ -38,7 +38,7 @@ def test_invokes_old_builder_without_feature_flag(self): runtime=self.runtime, ) - expected_files = {'included.js', 'node_modules', 'excluded.js', 'package.json'} + expected_files = {"included.js", "node_modules", "excluded.js", "package.json"} output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files)