From 8bd869d9c2b83f67b4f37d45883f9436e1cb1179 Mon Sep 17 00:00:00 2001 From: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Date: Sat, 22 Jan 2022 09:28:20 -0800 Subject: [PATCH 1/6] Enable esbuild in the Node.js builder (#307) * upgrade integration tests to use nodejs14.x instead of obsolete 8.10; remove garbage lockfile left by npm 7 * esbuild bundling for javascript * initial esbuild support * aws-sam -> aws_sam, revert to node 10 for integration tests, document package.json options * rename main -> entry_point, support multiple entrypoints * Update Appveyor nodejs version * Testing different node version * Update npm to version that supports node14 * Cleanup garbage package lock in dependencies dir generated by npm 7 * PEP8 compliance and missing doc strings * Add support for auto dep layer and incremental build with esbuild * Fix documentation * Revert accelerate feature changes * Add experimental feature flag, route all requests to old workflow * Add experimental feature flag, route all requests to old workflow * Remove unused variable * Black reformat Co-authored-by: Gojko Adzic --- .appveyor.yml | 4 +- aws_lambda_builders/builder.py | 7 + aws_lambda_builders/workflow.py | 4 + .../workflows/nodejs_npm/DESIGN.md | 265 +++++++++++++++++- .../workflows/nodejs_npm/actions.py | 173 +++++++++++- .../workflows/nodejs_npm/esbuild.py | 105 +++++++ .../workflows/nodejs_npm/utils.py | 14 + .../workflows/nodejs_npm/workflow.py | 155 ++++++++-- .../workflows/nodejs_npm/test_data/test.json | 1 + .../workflows/nodejs_npm/test_utils.py | 7 + .../workflows/nodejs_npm/test_nodejs_npm.py | 3 +- .../test_nodejs_npm_with_esbuild.py | 115 ++++++++ .../testdata/esbuild-binary/package-lock.json | 31 ++ .../testdata/esbuild-binary/package.json | 14 + .../testdata/no-deps-esbuild/.gitignore | 1 + .../testdata/no-deps-esbuild/excluded.js | 2 + .../testdata/no-deps-esbuild/included.js | 3 + .../testdata/no-deps-esbuild/package.json | 12 + .../.gitignore | 1 + .../excluded.js | 2 + .../included.js | 3 + .../included2.js | 3 + .../package.json | 18 ++ .../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 | 47 ++++ .../testdata/with-deps-esbuild/package.json | 18 ++ tests/unit/test_builder.py | 39 ++- .../unit/workflows/nodejs_npm/test_actions.py | 259 ++++++++++++++++- .../unit/workflows/nodejs_npm/test_esbuild.py | 79 ++++++ tests/unit/workflows/nodejs_npm/test_utils.py | 17 ++ .../workflows/nodejs_npm/test_workflow.py | 223 ++++++++++++--- 35 files changed, 1629 insertions(+), 77 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/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/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/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 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 create mode 100644 tests/unit/workflows/nodejs_npm/test_esbuild.py create mode 100644 tests/unit/workflows/nodejs_npm/test_utils.py diff --git a/.appveyor.yml b/.appveyor.yml index 94bf1e486..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: "10.10.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" 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/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md index 803052ed7..b947573a9 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 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,157 @@ 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. + + +### 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 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` +in the artifacts folder. + +```json +{ + "name": "nodeps-esbuild", + "version": "1.0.0", + "license": "APACHE2.0", + "aws_sam": { + "bundler": "esbuild", + "entry_points": ["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", + "entry_points": ["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", + "entry_points": ["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", + "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 43259bd8c..cb220d7f3 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -6,6 +6,7 @@ from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError from .npm import NpmExecutionError +from .esbuild import EsbuildExecutionError LOG = logging.getLogger(__name__) @@ -80,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, is_production=True): """ :type artifacts_dir: str :param artifacts_dir: an existing (writable) directory with project source files. @@ -88,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 is_production: bool + :param is_production: NPM installation mode is production (eg --production=false to force dev dependencies) """ super(NodejsNpmInstallAction, self).__init__() self.artifacts_dir = artifacts_dir self.subprocess_npm = subprocess_npm + self.is_production = is_production def execute(self): """ @@ -101,17 +106,62 @@ def execute(self): :raises lambda_builders.actions.ActionFailedError: when NPM execution fails """ + mode = "--production" if self.is_production else "--production=false" + try: 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", mode, "--unsafe-perm"], cwd=self.artifacts_dir ) except NpmExecutionError as ex: 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): """ @@ -185,7 +235,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 +246,120 @@ 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)) + + +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 "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 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] + + LOG.debug("NODEJS building %s using esbuild to %s", entry_paths, self.artifacts_dir) + + 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 = entry_points + ["--bundle", "--platform=node", "--format=cjs"] + minify = self.bundler_config.get("minify", True) + sourcemap = self.bundler_config.get("sourcemap", True) + target = self.bundler_config.get("target", "es2020") + if minify: + args.append("--minify") + if sourcemap: + args.append("--sourcemap") + args.append("--target={}".format(target)) + args.append("--outdir={}".format(self.artifacts_dir)) + try: + 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 new file mode 100644 index 000000000..992c49fe5 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm/esbuild.py @@ -0,0 +1,105 @@ +""" +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, executable_search_paths, which): + """ + :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils + :param osutils: An instance of OS Utilities for file manipulation + + :type executable_search_paths: list + :param executable_search_paths: List of paths to the 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 + 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. + """ + + 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) + + if binaries: + return binaries[0] + else: + raise EsbuildExecutionError(message="cannot find esbuild") + + 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_binary()] + 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..dc114ba06 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/utils.py +++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py @@ -7,6 +7,9 @@ import tarfile import subprocess import shutil +import json + +EXPERIMENTAL_FLAG_ESBUILD = "experimentalEsbuild" class OSUtils(object): @@ -48,3 +51,14 @@ 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) + + +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 b890ddcf1..b8f209c20 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -1,14 +1,34 @@ """ NodeJS NPM Workflow """ + 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, CopyDependenciesAction, MoveDependenciesAction, CleanUpAction -from .actions import NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction -from .utils import OSUtils +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 ( + NodejsNpmPackAction, + NodejsNpmLockFileCleanUpAction, + NodejsNpmInstallAction, + NodejsNpmrcCopyAction, + NodejsNpmrcCleanUpAction, + NodejsNpmCIAction, + EsbuildBundleAction, +) +from .utils import OSUtils, is_experimental_esbuild_scope from .npm import SubprocessNpm +from .esbuild import SubprocessEsbuild LOG = logging.getLogger(__name__) @@ -26,6 +46,8 @@ class NodejsNpmWorkflow(BaseWorkflow): EXCLUDED_FILES = (".aws-sam", ".git") + CONFIG_PROPERTY = "aws_sam" + def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): super(NodejsNpmWorkflow, self).__init__( @@ -35,23 +57,57 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim if osutils is None: osutils = OSUtils() - subprocess_npm = SubprocessNpm(osutils) - - tar_dest_dir = osutils.joinpath(scratch_dir, "unpacked") - tar_package_dir = osutils.joinpath(tar_dest_dir, "package") - 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" and is_experimental_esbuild_scope(self.experimental_flags): + 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( tar_dest_dir, scratch_dir, manifest_path, osutils=osutils, subprocess_npm=subprocess_npm ) - npm_copy_npmrc = NodejsNpmrcCopyAction(tar_package_dir, source_dir, osutils=osutils) - self.actions = [ + actions = [ npm_pack, npm_copy_npmrc, CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES), @@ -59,31 +115,98 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim if self.download_dependencies: # installed the dependencies into artifact folder - self.actions.append(NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm)) + actions.append(NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm)) # if dependencies folder exists, copy or move dependencies from artifact folder to dependencies folder # depends on the combine_dependencies flag if self.dependencies_dir: # clean up the dependencies folder first - self.actions.append(CleanUpAction(self.dependencies_dir)) + actions.append(CleanUpAction(self.dependencies_dir)) # if combine_dependencies is set, we should keep dependencies and source code in the artifact folder # while copying the dependencies. Otherwise we should separate the dependencies and source code if self.combine_dependencies: - self.actions.append(CopyDependenciesAction(source_dir, artifacts_dir, self.dependencies_dir)) + actions.append(CopyDependenciesAction(source_dir, artifacts_dir, self.dependencies_dir)) else: - self.actions.append(MoveDependenciesAction(source_dir, artifacts_dir, self.dependencies_dir)) + actions.append(MoveDependenciesAction(source_dir, artifacts_dir, self.dependencies_dir)) else: # if dependencies folder exists and not download dependencies, simply copy the dependencies from the # dependencies folder to artifact folder if self.dependencies_dir and self.combine_dependencies: - self.actions.append(CopySourceAction(self.dependencies_dir, artifacts_dir)) + actions.append(CopySourceAction(self.dependencies_dir, artifacts_dir)) else: LOG.info( "download_dependencies is False and dependencies_dir is None. Copying the source files into the " "artifacts directory. " ) - self.actions.append(NodejsNpmrcCleanUpAction(artifacts_dir, osutils=osutils)) + 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): + """ + 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) + 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) + 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] + + 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) + 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: + raise WorkflowFailedError(workflow_name=self.NAME, action_name="ParseManifest", reason=str(ex)) def get_resolvers(self): """ 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..a45d5585f 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 dcd7397f8..02a85cf8a 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -1,5 +1,4 @@ import logging -import mock import os import shutil import tempfile @@ -149,7 +148,7 @@ 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)) def test_builds_project_with_remote_dependencies_without_download_dependencies_with_dependencies_dir(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "npm-deps") 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..0de36f41d --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py @@ -0,0 +1,115 @@ +import os +import shutil +import tempfile +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, EXPERIMENTAL_FLAG_ESBUILD + + +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 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") + + self.builder.build( + source_dir, + self.artifacts_dir, + 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"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + 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( + source_dir, + self.artifacts_dir, + 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"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + def test_builds_typescript_projects(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, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + expected_files = {"included.js", "included.js.map"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + def test_builds_with_external_esbuild(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, + self.artifacts_dir, + self.scratch_dir, + 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"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) 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/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..997d05815 --- /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", + "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/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..43827dd4c --- /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", + "entry_points": ["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..93a7f9fd2 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package-lock.json @@ -0,0 +1,47 @@ +{ + "name": "with-deps-esbuild", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "name": "with-deps-esbuild", + "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==", + "dev": true, + "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==", + "dev": true + }, + "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..c4f10260c --- /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", + "entry_points": ["included.js"] + }, + "dependencies": { + "minimal-request-promise": "^1.5.0" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } +} 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_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index 39007a59c..f1b48ab66 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -7,6 +7,9 @@ NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction, + NodejsNpmLockFileCleanUpAction, + NodejsNpmCIAction, + EsbuildBundleAction, ) from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError @@ -54,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) @@ -65,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, is_production=False) + + action.execute() + + expected_args = ["install", "-q", "--no-audit", "--no-save", "--production=false", "--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 @@ -80,6 +95,32 @@ 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") def test_copies_npmrc_into_a_project(self, OSUtilMock): @@ -140,3 +181,219 @@ def test_skips_npmrc_removal_if_npmrc_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: "{}/{}".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") + 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() + + @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, True] + + 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", {"entry_points": ["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_single_entrypoint_exists(self): + + action = EsbuildBundleAction( + "source", "artifacts", {"entry_points": ["x.js"]}, self.osutils, self.subprocess_esbuild + ) + self.osutils.file_exists.side_effect = [False] + + 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", {"entry_points": ["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", {"entry_points": ["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", {"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", + "--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 new file mode 100644 index 000000000..4a3f6493b --- /dev/null +++ b/tests/unit/workflows/nodejs_npm/test_esbuild.py @@ -0,0 +1,79 @@ +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 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] + + which = lambda cmd, executable_search_paths: ["{}/{}".format(executable_search_paths[0], cmd)] + + self.under_test = SubprocessEsbuild(self.osutils, ["/a/b", "/c/d"], which) + + def test_run_executes_binary_found_in_exec_paths(self): + + self.under_test.run(["arg-a", "arg-b"]) + + self.osutils.popen.assert_called_with( + ["/a/b/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/esbuild", "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_EsbuildExecutionError_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_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")) + + 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_utils.py b/tests/unit/workflows/nodejs_npm/test_utils.py new file mode 100644 index 000000000..80d92a9a5 --- /dev/null +++ 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 c47938fd0..9d2469f70 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -1,17 +1,31 @@ -import mock - from unittest import TestCase +from mock import patch, call -from aws_lambda_builders.actions import CopySourceAction, CopyDependenciesAction, MoveDependenciesAction, CleanUpAction +from aws_lambda_builders.exceptions import WorkflowFailedError +from aws_lambda_builders.actions import CopySourceAction, CleanUpAction, CopyDependenciesAction, MoveDependenciesAction from aws_lambda_builders.architecture import ARM64 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, + NodejsNpmCIAction, + EsbuildBundleAction, ) -from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils +from aws_lambda_builders.workflows.nodejs_npm.utils import EXPERIMENTAL_FLAG_ESBUILD + + +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 TestNodejsNpmWorkflow(TestCase): @@ -21,25 +35,30 @@ class TestNodejsNpmWorkflow(TestCase): this is just a quick wiring test to provide fast feedback if things are badly broken """ - def setUp(self): - self.osutils_mock = mock.Mock(spec=OSUtils()) + @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_sets_up_npm_actions_with_download_dependencies_without_dependencies_dir(self): + self.osutils.file_exists.return_value = True - self.osutils_mock.file_exists.return_value = True - - workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils_mock) + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) - self.assertEqual(len(workflow.actions), 5) + 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) def test_workflow_sets_up_npm_actions_without_download_dependencies_with_dependencies_dir(self): - - self.osutils_mock.file_exists.return_value = True + self.osutils.file_exists.return_value = True workflow = NodejsNpmWorkflow( "source", @@ -48,20 +67,35 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_with_depende "manifest", dependencies_dir="dep", download_dependencies=False, - osutils=self.osutils_mock, + osutils=self.osutils, ) - self.assertEqual(len(workflow.actions), 5) + self.assertEqual(len(workflow.actions), 7) 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) + self.assertIsInstance(workflow.actions[6], NodejsNpmLockFileCleanUpAction) + + 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) + + 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) def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencies_dir(self): - self.osutils_mock.file_exists.return_value = True + self.osutils.file_exists.return_value = True workflow = NodejsNpmWorkflow( "source", @@ -70,10 +104,10 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencie "manifest", dependencies_dir="dep", download_dependencies=True, - osutils=self.osutils_mock, + osutils=self.osutils, ) - self.assertEqual(len(workflow.actions), 7) + self.assertEqual(len(workflow.actions), 9) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) @@ -82,11 +116,10 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencie self.assertIsInstance(workflow.actions[4], CleanUpAction) 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): - - self.osutils_mock.file_exists.return_value = True - workflow = NodejsNpmWorkflow( "source", "artifacts", @@ -94,19 +127,20 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_and_without_ "manifest", dependencies_dir=None, download_dependencies=False, - osutils=self.osutils_mock, + osutils=self.osutils, ) - self.assertEqual(len(workflow.actions), 4) + self.assertEqual(len(workflow.actions), 5) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) self.assertIsInstance(workflow.actions[2], CopySourceAction) self.assertIsInstance(workflow.actions[3], NodejsNpmrcCleanUpAction) + self.assertIsInstance(workflow.actions[4], NodejsNpmLockFileCleanUpAction) def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): - self.osutils_mock.file_exists.return_value = True + self.osutils.file_exists.return_value = True workflow = NodejsNpmWorkflow( "source", @@ -116,10 +150,10 @@ def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): dependencies_dir="dep", download_dependencies=True, combine_dependencies=False, - osutils=self.osutils_mock, + osutils=self.osutils, ) - self.assertEqual(len(workflow.actions), 7) + self.assertEqual(len(workflow.actions), 9) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) @@ -128,34 +162,155 @@ def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): 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_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 = [True, False, False] + + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + 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_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" + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] + + 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") + + 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"}}] - def test_workflow_only_copy_action(self): - self.osutils_mock.file_exists.return_value = False + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + executable_search_paths=["other/bin"], + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) - workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils_mock) + self.osutils.popen.assert_called_with(["npm", "bin"], stdout="PIPE", stderr="PIPE", cwd="source") - self.assertEqual(len(workflow.actions), 1) + esbuild = workflow.actions[1].subprocess_esbuild - self.assertIsInstance(workflow.actions[0], CopySourceAction) + 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, True] + + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + 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 = [True, False, True] + + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + 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")] + ) def test_must_validate_architecture(self): - self.osutils_mock.file_exists.return_value = True + self.osutils.is_windows.side_effect = [False, False] workflow = NodejsNpmWorkflow( "source", "artifacts", "scratch", "manifest", options={"artifact_executable_name": "foo"}, - osutils=self.osutils_mock, + osutils=self.osutils, ) workflow_with_arm = NodejsNpmWorkflow( "source", "artifacts", "scratch", "manifest", - options={"artifact_executable_name": "foo"}, architecture=ARM64, - osutils=self.osutils_mock, + osutils=self.osutils, ) self.assertEqual(workflow.architecture, "x86_64") From 79af5bf467719000878374a830f1b6996a516b36 Mon Sep 17 00:00:00 2001 From: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Date: Tue, 1 Feb 2022 15:12:27 -0800 Subject: [PATCH 2/6] feat: Turn esbuild into its own workflow (#320) * Create new esbuild workflow * Fix esbuild workflow capability --- aws_lambda_builders/workflows/__init__.py | 1 + .../workflows/nodejs_npm/DESIGN.md | 4 +- .../workflows/nodejs_npm/actions.py | 77 -------- .../workflows/nodejs_npm/utils.py | 9 - .../workflows/nodejs_npm/workflow.py | 85 +-------- .../workflows/nodejs_npm_esbuild/__init__.py | 5 + .../workflows/nodejs_npm_esbuild/actions.py | 84 +++++++++ .../esbuild.py | 0 .../workflows/nodejs_npm_esbuild/utils.py | 12 ++ .../workflows/nodejs_npm_esbuild/workflow.py | 130 +++++++++++++ .../workflows/nodejs_npm/test_nodejs_npm.py | 15 -- .../workflows/nodejs_npm_esbuild/__init__.py | 0 .../test_nodejs_npm_with_esbuild.py | 45 +++-- .../testdata/broken-package/excluded.js | 0 .../testdata/broken-package/included.js | 0 .../testdata/broken-package/package.json | 0 .../testdata/esbuild-binary/package-lock.json | 0 .../testdata/esbuild-binary/package.json | 0 .../testdata/no-deps-esbuild/.gitignore | 0 .../testdata/no-deps-esbuild/excluded.js | 0 .../testdata/no-deps-esbuild/included.js | 0 .../testdata/no-deps-esbuild/package.json | 0 .../.gitignore | 0 .../excluded.js | 0 .../included.js | 0 .../included2.js | 0 .../package.json | 0 .../with-deps-esbuild-typescript/included.ts | 0 .../package-lock.json | 0 .../with-deps-esbuild-typescript/package.json | 0 .../testdata/with-deps-esbuild/excluded.js | 0 .../testdata/with-deps-esbuild/included.js | 0 .../with-deps-esbuild/package-lock.json | 0 .../testdata/with-deps-esbuild/package.json | 0 .../unit/workflows/nodejs_npm/test_actions.py | 169 ----------------- .../workflows/nodejs_npm/test_workflow.py | 136 +------------- .../workflows/nodejs_npm_esbuild/__init__.py | 0 .../nodejs_npm_esbuild/test_actions.py | 173 +++++++++++++++++ .../test_esbuild.py | 2 +- .../test_utils.py | 5 +- .../nodejs_npm_esbuild/test_workflow.py | 176 ++++++++++++++++++ 41 files changed, 625 insertions(+), 503 deletions(-) create mode 100644 aws_lambda_builders/workflows/nodejs_npm_esbuild/__init__.py create mode 100644 aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py rename aws_lambda_builders/workflows/{nodejs_npm => nodejs_npm_esbuild}/esbuild.py (100%) create mode 100644 aws_lambda_builders/workflows/nodejs_npm_esbuild/utils.py create mode 100644 aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py create mode 100644 tests/integration/workflows/nodejs_npm_esbuild/__init__.py rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/test_nodejs_npm_with_esbuild.py (72%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/broken-package/excluded.js (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/broken-package/included.js (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/broken-package/package.json (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/esbuild-binary/package-lock.json (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/esbuild-binary/package.json (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/no-deps-esbuild/.gitignore (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/no-deps-esbuild/excluded.js (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/no-deps-esbuild/included.js (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/no-deps-esbuild/package.json (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild-multiple-entrypoints/.gitignore (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild-multiple-entrypoints/excluded.js (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild-multiple-entrypoints/included.js (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild-multiple-entrypoints/included2.js (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild-multiple-entrypoints/package.json (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild-typescript/included.ts (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild-typescript/package-lock.json (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild-typescript/package.json (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild/excluded.js (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild/included.js (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild/package-lock.json (100%) rename tests/integration/workflows/{nodejs_npm => nodejs_npm_esbuild}/testdata/with-deps-esbuild/package.json (100%) create mode 100644 tests/unit/workflows/nodejs_npm_esbuild/__init__.py create mode 100644 tests/unit/workflows/nodejs_npm_esbuild/test_actions.py rename tests/unit/workflows/{nodejs_npm => nodejs_npm_esbuild}/test_esbuild.py (96%) rename tests/unit/workflows/{nodejs_npm => nodejs_npm_esbuild}/test_utils.py (78%) create mode 100644 tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py diff --git a/aws_lambda_builders/workflows/__init__.py b/aws_lambda_builders/workflows/__init__.py index fd9c9c141..9d58eed19 100644 --- a/aws_lambda_builders/workflows/__init__.py +++ b/aws_lambda_builders/workflows/__init__.py @@ -11,3 +11,4 @@ import aws_lambda_builders.workflows.java_maven import aws_lambda_builders.workflows.dotnet_clipackage import aws_lambda_builders.workflows.custom_make +import aws_lambda_builders.workflows.nodejs_npm_esbuild diff --git a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md index b947573a9..72f7aa44d 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md +++ b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md @@ -323,7 +323,7 @@ directly. } ``` -For a full example, see the [`with-deps-esbuild`](../../../tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/) test project. +For a full example, see the [`with-deps-esbuild`](../../../tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/) test project. #### Building typescript @@ -350,7 +350,7 @@ as in the example below. There is no transpiling process needed upfront. } ``` -For a full example, see the [`with-deps-esbuild-typescript`](../../../tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/) test project. +For a full example, see the [`with-deps-esbuild-typescript`](../../../tests/integration/workflows/nodejs_npm_esbuild/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 . diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index cb220d7f3..a7bfd13fa 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -6,7 +6,6 @@ from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError from .npm import NpmExecutionError -from .esbuild import EsbuildExecutionError LOG = logging.getLogger(__name__) @@ -287,79 +286,3 @@ 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 "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 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] - - LOG.debug("NODEJS building %s using esbuild to %s", entry_paths, self.artifacts_dir) - - 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 = entry_points + ["--bundle", "--platform=node", "--format=cjs"] - minify = self.bundler_config.get("minify", True) - sourcemap = self.bundler_config.get("sourcemap", True) - target = self.bundler_config.get("target", "es2020") - if minify: - args.append("--minify") - if sourcemap: - args.append("--sourcemap") - args.append("--target={}".format(target)) - args.append("--outdir={}".format(self.artifacts_dir)) - try: - 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/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py index dc114ba06..aebfb3b7b 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/utils.py +++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py @@ -9,8 +9,6 @@ import shutil import json -EXPERIMENTAL_FLAG_ESBUILD = "experimentalEsbuild" - class OSUtils(object): @@ -55,10 +53,3 @@ 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 b8f209c20..f432d3d51 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -3,8 +3,6 @@ """ import logging -import json -from typing import List from aws_lambda_builders.path_resolver import PathResolver from aws_lambda_builders.workflow import BaseWorkflow, Capability @@ -13,22 +11,17 @@ CleanUpAction, CopyDependenciesAction, MoveDependenciesAction, - BaseAction, ) -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 .utils import OSUtils, is_experimental_esbuild_scope +from .utils import OSUtils from .npm import SubprocessNpm -from .esbuild import SubprocessEsbuild LOG = logging.getLogger(__name__) @@ -64,16 +57,9 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim subprocess_npm = SubprocessNpm(osutils) - manifest_config = self.get_manifest_config(osutils, manifest_path) - - 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 - ) - else: - self.actions = self.actions_without_bundler( - source_dir, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm - ) + 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): """ @@ -147,67 +133,6 @@ 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) - 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) - 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] - - 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) - 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: - raise WorkflowFailedError(workflow_name=self.NAME, action_name="ParseManifest", reason=str(ex)) - def get_resolvers(self): """ specialized path resolver that just returns the list of executable for the runtime on the path. diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/__init__.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/__init__.py new file mode 100644 index 000000000..50086a886 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/__init__.py @@ -0,0 +1,5 @@ +""" +Builds NodeJS Lambda functions using NPM dependency manager with esbuild bundler +""" + +from .workflow import NodejsNpmEsbuildWorkflow diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py new file mode 100644 index 000000000..f620ff717 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py @@ -0,0 +1,84 @@ +""" +Actions specific to the esbuild bundler +""" +import logging +from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError +from .esbuild import EsbuildExecutionError + +LOG = logging.getLogger(__name__) + + +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 "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 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] + + LOG.debug("NODEJS building %s using esbuild to %s", entry_paths, self.artifacts_dir) + + 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 = entry_points + ["--bundle", "--platform=node", "--format=cjs"] + minify = self.bundler_config.get("minify", True) + sourcemap = self.bundler_config.get("sourcemap", True) + target = self.bundler_config.get("target", "es2020") + if minify: + args.append("--minify") + if sourcemap: + args.append("--sourcemap") + args.append("--target={}".format(target)) + args.append("--outdir={}".format(self.artifacts_dir)) + try: + 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/esbuild.py similarity index 100% rename from aws_lambda_builders/workflows/nodejs_npm/esbuild.py rename to aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild.py diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/utils.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/utils.py new file mode 100644 index 000000000..2d1aadb57 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/utils.py @@ -0,0 +1,12 @@ +""" +esbuild specific utilities and feature flag +""" + +EXPERIMENTAL_FLAG_ESBUILD = "experimentalEsbuild" + + +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_esbuild/workflow.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py new file mode 100644 index 000000000..729d1a2a7 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py @@ -0,0 +1,130 @@ +""" +NodeJS NPM Workflow using the esbuild bundler +""" + +import logging +import json + +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 ( + EsbuildBundleAction, +) +from .utils import is_experimental_esbuild_scope +from .esbuild import SubprocessEsbuild, EsbuildExecutionError +from ..nodejs_npm.actions import NodejsNpmCIAction, NodejsNpmInstallAction +from ..nodejs_npm.npm import SubprocessNpm +from ..nodejs_npm.utils import OSUtils +from ...path_resolver import PathResolver + +LOG = logging.getLogger(__name__) + + +class NodejsNpmEsbuildWorkflow(BaseWorkflow): + + """ + A Lambda builder workflow that knows how to pack + NodeJS projects using NPM. + """ + + NAME = "NodejsNpmEsbuildBuilder" + + CAPABILITY = Capability(language="nodejs", dependency_manager="npm-esbuild", application_framework=None) + + EXCLUDED_FILES = (".aws-sam", ".git") + + CONFIG_PROPERTY = "aws_sam" + + def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): + + super(NodejsNpmEsbuildWorkflow, 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 not is_experimental_esbuild_scope(self.experimental_flags): + raise EsbuildExecutionError(message="Feature flag must be enabled to use this workflow") + + self.actions = self.actions_with_bundler(source_dir, artifacts_dir, manifest_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 + + :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) + 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) + 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] + + 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) + 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: + raise WorkflowFailedError(workflow_name=self.NAME, action_name="ParseManifest", reason=str(ex)) + + def get_resolvers(self): + """ + specialized path resolver that just returns the list of executable for the runtime on the path. + """ + return [PathResolver(runtime=self.runtime, binary="npm")] diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index 02a85cf8a..82da7b576 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -135,21 +135,6 @@ def test_fails_if_npm_cannot_resolve_dependencies(self): self.assertIn("No matching version found for minimal-request-promise@0.0.0-NON_EXISTENT", str(ctx.exception)) - def test_fails_if_package_json_is_broken(self): - - source_dir = os.path.join(self.TEST_DATA_FOLDER, "broken-package") - - with self.assertRaises(WorkflowFailedError) as ctx: - self.builder.build( - source_dir, - self.artifacts_dir, - self.scratch_dir, - os.path.join(source_dir, "package.json"), - runtime=self.runtime, - ) - - self.assertIn("NodejsNpmBuilder:ParseManifest", str(ctx.exception)) - def test_builds_project_with_remote_dependencies_without_download_dependencies_with_dependencies_dir(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "npm-deps") diff --git a/tests/integration/workflows/nodejs_npm_esbuild/__init__.py b/tests/integration/workflows/nodejs_npm_esbuild/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py similarity index 72% rename from tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py rename to tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py index 0de36f41d..927b9ea2e 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py +++ b/tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py @@ -3,8 +3,11 @@ 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, EXPERIMENTAL_FLAG_ESBUILD +from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils +from aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild import EsbuildExecutionError +from aws_lambda_builders.workflows.nodejs_npm_esbuild.utils import EXPERIMENTAL_FLAG_ESBUILD class TestNodejsNpmWorkflowWithEsbuild(TestCase): @@ -20,27 +23,25 @@ def setUp(self): 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.builder = LambdaBuilder(language="nodejs", dependency_manager="npm-esbuild", application_framework=None) self.runtime = "nodejs14.x" def tearDown(self): shutil.rmtree(self.artifacts_dir) shutil.rmtree(self.scratch_dir) - def test_invokes_old_builder_without_feature_flag(self): + def test_doesnt_build_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) + with self.assertRaises(EsbuildExecutionError) as context: + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + ) + self.assertEqual(str(context.exception), "Esbuild Failed: Feature flag must be enabled to use this workflow") def test_builds_javascript_project_with_dependencies(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild") @@ -113,3 +114,19 @@ 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_fails_if_package_json_is_broken(self): + + source_dir = os.path.join(self.TEST_DATA_FOLDER, "broken-package") + + with self.assertRaises(WorkflowFailedError) as ctx: + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + self.assertIn("NodejsNpmEsbuildBuilder:ParseManifest", str(ctx.exception)) diff --git a/tests/integration/workflows/nodejs_npm/testdata/broken-package/excluded.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/excluded.js similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/broken-package/excluded.js rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/excluded.js diff --git a/tests/integration/workflows/nodejs_npm/testdata/broken-package/included.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/included.js similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/broken-package/included.js rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/included.js diff --git a/tests/integration/workflows/nodejs_npm/testdata/broken-package/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/package.json similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/broken-package/package.json rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/package.json diff --git a/tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package-lock.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-binary/package-lock.json similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package-lock.json rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-binary/package-lock.json diff --git a/tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-binary/package.json similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/esbuild-binary/package.json rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-binary/package.json diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/.gitignore b/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/.gitignore similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/.gitignore rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/.gitignore diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/excluded.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/excluded.js similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/excluded.js rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/excluded.js diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/included.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/included.js similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/included.js rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/included.js diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/package.json similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/no-deps-esbuild/package.json rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/package.json diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/.gitignore b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/.gitignore similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/.gitignore rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/.gitignore diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/excluded.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/excluded.js similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/excluded.js rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/excluded.js diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/included.js similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included.js rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/included.js diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included2.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/included2.js similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/included2.js rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/included2.js diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/package.json similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-multiple-entrypoints/package.json rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/package.json diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/included.ts b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-typescript/included.ts similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/included.ts rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-typescript/included.ts diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package-lock.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-typescript/package-lock.json similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package-lock.json rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-typescript/package-lock.json diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-typescript/package.json similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild-typescript/package.json rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-typescript/package.json diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/excluded.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/excluded.js similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/excluded.js rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/excluded.js diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/included.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/included.js similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/included.js rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/included.js diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package-lock.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/package-lock.json similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package-lock.json rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/package-lock.json diff --git a/tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/package.json similarity index 100% rename from tests/integration/workflows/nodejs_npm/testdata/with-deps-esbuild/package.json rename to tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/package.json diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index f1b48ab66..191191378 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -9,7 +9,6 @@ NodejsNpmrcCleanUpAction, NodejsNpmLockFileCleanUpAction, NodejsNpmCIAction, - EsbuildBundleAction, ) from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError @@ -229,171 +228,3 @@ def test_raises_action_failed_when_removing_fails(self, OSUtilMock): 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, True] - - 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", {"entry_points": ["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_single_entrypoint_exists(self): - - action = EsbuildBundleAction( - "source", "artifacts", {"entry_points": ["x.js"]}, self.osutils, self.subprocess_esbuild - ) - self.osutils.file_exists.side_effect = [False] - - 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", {"entry_points": ["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", {"entry_points": ["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", {"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", - "--minify", - "--sourcemap", - "--target=node14", - "--outdir=artifacts", - ], - cwd="source", - ) diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 9d2469f70..6f558d715 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -1,21 +1,16 @@ from unittest import TestCase -from mock import patch, call +from mock import patch -from aws_lambda_builders.exceptions import WorkflowFailedError from aws_lambda_builders.actions import CopySourceAction, CleanUpAction, CopyDependenciesAction, MoveDependenciesAction from aws_lambda_builders.architecture import ARM64 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, - NodejsNpmCIAction, - EsbuildBundleAction, ) -from aws_lambda_builders.workflows.nodejs_npm.utils import EXPERIMENTAL_FLAG_ESBUILD class FakePopen: @@ -165,135 +160,6 @@ def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): 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): - - self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] - - self.osutils.file_exists.side_effect = [True, False, False] - - workflow = NodejsNpmWorkflow( - "source", - "artifacts", - "scratch_dir", - "manifest", - osutils=self.osutils, - experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], - ) - - 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_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" - self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] - - 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") - - 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"], - experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], - ) - - 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, True] - - workflow = NodejsNpmWorkflow( - "source", - "artifacts", - "scratch_dir", - "manifest", - osutils=self.osutils, - experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], - ) - - 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 = [True, False, True] - - workflow = NodejsNpmWorkflow( - "source", - "artifacts", - "scratch_dir", - "manifest", - osutils=self.osutils, - experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], - ) - - 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")] - ) - def test_must_validate_architecture(self): self.osutils.is_windows.side_effect = [False, False] workflow = NodejsNpmWorkflow( diff --git a/tests/unit/workflows/nodejs_npm_esbuild/__init__.py b/tests/unit/workflows/nodejs_npm_esbuild/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py b/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py new file mode 100644 index 000000000..3db988190 --- /dev/null +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py @@ -0,0 +1,173 @@ +from unittest import TestCase +from mock import patch + +from aws_lambda_builders.actions import ActionFailedError +from aws_lambda_builders.workflows.nodejs_npm_esbuild.actions import EsbuildBundleAction + + +class TestEsbuildBundleAction(TestCase): + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + @patch("aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild.SubprocessEsbuild") + def setUp(self, OSUtilMock, SubprocessEsbuildMock): + self.osutils = OSUtilMock.return_value + self.subprocess_esbuild = SubprocessEsbuildMock.return_value + self.osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) + self.osutils.file_exists.side_effect = [True, True] + + def test_raises_error_if_entrypoints_not_specified(self): + action = EsbuildBundleAction("source", "artifacts", {"config": "param"}, self.osutils, self.subprocess_esbuild) + 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", {"entry_points": ["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_single_entrypoint_exists(self): + + action = EsbuildBundleAction( + "source", "artifacts", {"entry_points": ["x.js"]}, self.osutils, self.subprocess_esbuild + ) + self.osutils.file_exists.side_effect = [False] + + 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", {"entry_points": ["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", {"entry_points": ["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", {"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", + "--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_esbuild/test_esbuild.py similarity index 96% rename from tests/unit/workflows/nodejs_npm/test_esbuild.py rename to tests/unit/workflows/nodejs_npm_esbuild/test_esbuild.py index 4a3f6493b..44f94387e 100644 --- a/tests/unit/workflows/nodejs_npm/test_esbuild.py +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_esbuild.py @@ -1,7 +1,7 @@ from unittest import TestCase from mock import patch -from aws_lambda_builders.workflows.nodejs_npm.esbuild import SubprocessEsbuild, EsbuildExecutionError +from aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild import SubprocessEsbuild, EsbuildExecutionError class FakePopen: diff --git a/tests/unit/workflows/nodejs_npm/test_utils.py b/tests/unit/workflows/nodejs_npm_esbuild/test_utils.py similarity index 78% rename from tests/unit/workflows/nodejs_npm/test_utils.py rename to tests/unit/workflows/nodejs_npm_esbuild/test_utils.py index 80d92a9a5..f0dce1e2f 100644 --- a/tests/unit/workflows/nodejs_npm/test_utils.py +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_utils.py @@ -1,7 +1,10 @@ from unittest import TestCase from parameterized import parameterized -from aws_lambda_builders.workflows.nodejs_npm.utils import EXPERIMENTAL_FLAG_ESBUILD, is_experimental_esbuild_scope +from aws_lambda_builders.workflows.nodejs_npm_esbuild.utils import ( + EXPERIMENTAL_FLAG_ESBUILD, + is_experimental_esbuild_scope, +) class TestNodejsUtils(TestCase): diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py new file mode 100644 index 000000000..e76dde681 --- /dev/null +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py @@ -0,0 +1,176 @@ +from unittest import TestCase +from mock import patch, call + +from aws_lambda_builders.exceptions import WorkflowFailedError +from aws_lambda_builders.architecture import ARM64 +from aws_lambda_builders.workflows.nodejs_npm.actions import NodejsNpmInstallAction, NodejsNpmCIAction +from aws_lambda_builders.workflows.nodejs_npm_esbuild import NodejsNpmEsbuildWorkflow +from aws_lambda_builders.workflows.nodejs_npm_esbuild.actions import EsbuildBundleAction +from aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild import SubprocessEsbuild +from aws_lambda_builders.workflows.nodejs_npm_esbuild.utils import EXPERIMENTAL_FLAG_ESBUILD + + +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 TestNodejsNpmEsbuildWorkflow(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 + """ + + @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_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 = [True, False, False] + + workflow = NodejsNpmEsbuildWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + 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_workflow_fails_if_manifest_parsing_fails(self): + + self.osutils.parse_json.side_effect = OSError("boom!") + + with self.assertRaises(WorkflowFailedError) as raised: + NodejsNpmEsbuildWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + + self.assertEqual(raised.exception.args[0], "NodejsNpmEsbuildBuilder: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" + self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] + + workflow = NodejsNpmEsbuildWorkflow( + "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") + 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 = NodejsNpmEsbuildWorkflow( + "source", + "artifacts", + "scratch_dir", + "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") + 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, True] + + workflow = NodejsNpmEsbuildWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + 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 = [True, False, True] + + workflow = NodejsNpmEsbuildWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + 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")] + ) + + def test_must_validate_architecture(self): + self.osutils.is_windows.side_effect = [False, False] + self.osutils.popen.side_effect = [self.popen, self.popen] + + workflow = NodejsNpmEsbuildWorkflow( + "source", + "artifacts", + "scratch", + "manifest", + options={"artifact_executable_name": "foo"}, + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + workflow_with_arm = NodejsNpmEsbuildWorkflow( + "source", + "artifacts", + "scratch", + "manifest", + architecture=ARM64, + osutils=self.osutils, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + self.assertEqual(workflow.architecture, "x86_64") + self.assertEqual(workflow_with_arm.architecture, "arm64") From 9d37cc84c9520445c15bafb419ff0ffb047be9cb Mon Sep 17 00:00:00 2001 From: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Date: Wed, 2 Feb 2022 16:28:07 -0800 Subject: [PATCH 3/6] feat: java build changes (#318) * feat: java build changes related to maven scope and layers - use "runtime" scope for maven builds if experimental flag is provided - copy created jar files for layer code instead of classes * Update the maven DESIGN.md to explain changes. * add maven integration tests * add gradle integration tests * fix formatting & unit tests * fix issues after merge * correct dependency with maven tests * change type from function to Callable Co-authored-by: Wilton_ * disable too-many-locals for workflow.py only Co-authored-by: Phillip Rower Co-authored-by: Wilton_ --- Makefile | 2 +- aws_lambda_builders/builder.py | 6 ++ aws_lambda_builders/utils.py | 18 +++++- aws_lambda_builders/workflow.py | 7 +++ aws_lambda_builders/workflows/java/utils.py | 36 ++++++++---- .../workflows/java_gradle/actions.py | 26 ++++++++- .../workflows/java_gradle/workflow.py | 13 ++++- .../workflows/java_maven/DESIGN.md | 22 +++++++- .../workflows/java_maven/actions.py | 32 +++++++++++ .../workflows/java_maven/maven.py | 7 ++- .../workflows/java_maven/workflow.py | 24 ++++++-- tests/functional/test_utils.py | 13 +++++ .../workflows/common_test_utils.py | 7 +++ .../workflows/java_gradle/test_java_gradle.py | 54 +++++++++++++++++- .../testdata/single-build/layer/build.gradle | 31 ++++++++++ .../single-build/layer/settings.gradle | 1 + .../java/aws/lambdabuilders/CommonCode.java | 10 ++++ .../single-build/with-layer-deps/build.gradle | 28 ++++++++++ .../main/java/aws/lambdabuilders/Main.java | 16 ++++++ .../workflows/java_maven/test_java_maven.py | 56 ++++++++++++++++++- .../testdata/single-build/layer/pom.xml | 26 +++++++++ .../java/aws/lambdabuilders/CommonCode.java | 10 ++++ .../single-build/with-layer-deps/pom.xml | 22 ++++++++ .../main/java/aws/lambdabuilders/Main.java | 16 ++++++ tests/unit/test_builder.py | 4 ++ tests/unit/workflows/java/test_utils.py | 33 +++++++++++ .../workflows/java_gradle/test_actions.py | 25 ++++++++- .../workflows/java_gradle/test_workflow.py | 23 +++++++- .../unit/workflows/java_maven/test_actions.py | 45 ++++++++++++++- tests/unit/workflows/java_maven/test_maven.py | 12 ++++ .../workflows/java_maven/test_workflow.py | 25 +++++++++ 31 files changed, 617 insertions(+), 33 deletions(-) create mode 100644 tests/integration/workflows/java_gradle/testdata/single-build/layer/build.gradle create mode 100644 tests/integration/workflows/java_gradle/testdata/single-build/layer/settings.gradle create mode 100644 tests/integration/workflows/java_gradle/testdata/single-build/layer/src/main/java/aws/lambdabuilders/CommonCode.java create mode 100644 tests/integration/workflows/java_gradle/testdata/single-build/with-layer-deps/build.gradle create mode 100644 tests/integration/workflows/java_gradle/testdata/single-build/with-layer-deps/src/main/java/aws/lambdabuilders/Main.java create mode 100644 tests/integration/workflows/java_maven/testdata/single-build/layer/pom.xml create mode 100644 tests/integration/workflows/java_maven/testdata/single-build/layer/src/main/java/aws/lambdabuilders/CommonCode.java create mode 100644 tests/integration/workflows/java_maven/testdata/single-build/with-layer-deps/pom.xml create mode 100644 tests/integration/workflows/java_maven/testdata/single-build/with-layer-deps/src/main/java/aws/lambdabuilders/Main.java create mode 100644 tests/unit/workflows/java/test_utils.py diff --git a/Makefile b/Makefile index 883aba1b1..abaf957ec 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ init: test: # Run unit tests # Fail if coverage falls below 94% - LAMBDA_BUILDERS_DEV=1 pytest --cov aws_lambda_builders --cov-report term-missing --cov-fail-under 94 tests/unit tests/functional + LAMBDA_BUILDERS_DEV=1 pytest -vv --cov aws_lambda_builders --cov-report term-missing --cov-fail-under 94 tests/unit tests/functional func-test: LAMBDA_BUILDERS_DEV=1 pytest tests/functional diff --git a/aws_lambda_builders/builder.py b/aws_lambda_builders/builder.py index bcacb67f7..1538cc0d6 100644 --- a/aws_lambda_builders/builder.py +++ b/aws_lambda_builders/builder.py @@ -69,6 +69,7 @@ def build( dependencies_dir=None, combine_dependencies=True, architecture=X86_64, + is_building_layer=False, experimental_flags=None, ): # pylint: disable-msg=too-many-locals @@ -130,6 +131,10 @@ def build( :param architecture: Type of architecture x86_64 and arm64 for Lambda Function + :type is_building_layer: bool + :param is_building_layer: + Boolean flag which will be set True if current build operation is being executed for layers + :type experimental_flags: list :param experimental_flags: List of strings, which will indicate enabled experimental flags for the current build session @@ -152,6 +157,7 @@ def build( dependencies_dir=dependencies_dir, combine_dependencies=combine_dependencies, architecture=architecture, + is_building_layer=is_building_layer, experimental_flags=experimental_flags, ) diff --git a/aws_lambda_builders/utils.py b/aws_lambda_builders/utils.py index 5c130d86b..791b68669 100644 --- a/aws_lambda_builders/utils.py +++ b/aws_lambda_builders/utils.py @@ -12,7 +12,7 @@ LOG = logging.getLogger(__name__) -def copytree(source, destination, ignore=None): +def copytree(source, destination, ignore=None, include=None): """ Similar to shutil.copytree except that it removes the limitation that the destination directory should be present. @@ -29,6 +29,12 @@ def copytree(source, destination, ignore=None): :param ignore: A function that returns a set of file names to ignore, given a list of available file names. Similar to the ``ignore`` property of ``shutils.copytree`` method + + :type include: Callable[[str], bool] + :param include: + A function that will decide whether a file should be copied or skipped it. It accepts file name as parameter + and return True or False. Returning True will continue copy operation, returning False will skip copy operation + for that file """ if not os.path.exists(source): @@ -36,10 +42,12 @@ def copytree(source, destination, ignore=None): return if not os.path.exists(destination): + LOG.debug("Creating target folders at %s", destination) os.makedirs(destination) try: # Let's try to copy the directory metadata from source to destination + LOG.debug("Copying directory metadata from source (%s) to destination (%s)", source, destination) shutil.copystat(source, destination) except OSError as ex: # Can't copy file access times in Windows @@ -54,14 +62,20 @@ def copytree(source, destination, ignore=None): for name in names: # Skip ignored names if name in ignored_names: + LOG.debug("File (%s) is in ignored set, skipping it", name) continue new_source = os.path.join(source, name) new_destination = os.path.join(destination, name) + if include and not os.path.isdir(new_source) and not include(name): + LOG.debug("File (%s) doesn't satisfy the include rule, skipping it", name) + continue + if os.path.isdir(new_source): - copytree(new_source, new_destination, ignore=ignore) + copytree(new_source, new_destination, ignore=ignore, include=include) else: + LOG.debug("Copying source file (%s) to destination (%s)", new_source, new_destination) shutil.copy2(new_source, new_destination) diff --git a/aws_lambda_builders/workflow.py b/aws_lambda_builders/workflow.py index e979a8392..d58f0c430 100644 --- a/aws_lambda_builders/workflow.py +++ b/aws_lambda_builders/workflow.py @@ -164,8 +164,10 @@ def __init__( dependencies_dir=None, combine_dependencies=True, architecture=X86_64, + is_building_layer=False, experimental_flags=None, ): + # pylint: disable-msg=too-many-locals """ Initialize the builder with given arguments. These arguments together form the "public API" that each build action must support at the minimum. @@ -201,6 +203,10 @@ 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 + + is_building_layer: bool, optional + Boolean flag which will be set True if current build operation is being executed for layers + experimental_flags: list, optional List of strings, which will indicate enabled experimental flags for the current build session """ @@ -218,6 +224,7 @@ def __init__( self.dependencies_dir = dependencies_dir self.combine_dependencies = combine_dependencies self.architecture = architecture + self.is_building_layer = is_building_layer self.experimental_flags = experimental_flags if experimental_flags else [] # Actions are registered by the subclasses as they seem fit diff --git a/aws_lambda_builders/workflows/java/utils.py b/aws_lambda_builders/workflows/java/utils.py index 7503cb531..fec8750c8 100644 --- a/aws_lambda_builders/workflows/java/utils.py +++ b/aws_lambda_builders/workflows/java/utils.py @@ -6,7 +6,10 @@ import platform import shutil import subprocess -from aws_lambda_builders.utils import which +from aws_lambda_builders.utils import which, copytree + + +EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG = "experimentalMavenScopeAndLayer" class OSUtils(object): @@ -37,17 +40,8 @@ def exists(self, p): def which(self, executable, executable_search_paths=None): return which(executable, executable_search_paths=executable_search_paths) - def copytree(self, source, destination): - if not os.path.exists(destination): - self.makedirs(destination) - names = self.listdir(source) - for name in names: - new_source = os.path.join(source, name) - new_destination = os.path.join(destination, name) - if os.path.isdir(new_source): - self.copytree(new_source, new_destination) - else: - self.copy(new_source, new_destination) + def copytree(self, source, destination, ignore=None, include=None): + copytree(source, destination, ignore=ignore, include=include) def makedirs(self, d): return os.makedirs(d) @@ -58,3 +52,21 @@ def rmtree(self, d): @property def pipe(self): return subprocess.PIPE + + +def jar_file_filter(file_name): + """ + A function that will filter .jar files for copy operation + + :type file_name: str + :param file_name: + Name of the file that will be checked against if it ends with .jar or not + """ + return bool(file_name) and isinstance(file_name, str) and file_name.endswith(".jar") + + +def is_experimental_maven_scope_and_layers_active(experimental_flags): + """ + A function which will determine if experimental maven scope and layer changes are active + """ + return bool(experimental_flags) and EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG in experimental_flags diff --git a/aws_lambda_builders/workflows/java_gradle/actions.py b/aws_lambda_builders/workflows/java_gradle/actions.py index ddf8ded9c..e92dcb16b 100644 --- a/aws_lambda_builders/workflows/java_gradle/actions.py +++ b/aws_lambda_builders/workflows/java_gradle/actions.py @@ -5,6 +5,7 @@ import os from aws_lambda_builders.actions import ActionFailedError, BaseAction, Purpose from .gradle import GradleExecutionError +from ..java.utils import jar_file_filter class JavaGradleBuildAction(BaseAction): @@ -56,7 +57,7 @@ def _build_project(self, init_script_file): class JavaGradleCopyArtifactsAction(BaseAction): - NAME = "CopyArtifacts" + NAME = "JavaGradleCopyArtifacts" DESCRIPTION = "Copying the built artifacts" PURPOSE = Purpose.COPY_SOURCE @@ -77,3 +78,26 @@ def _copy_artifacts(self): self.os_utils.copytree(lambda_build_output, self.artifacts_dir) except Exception as ex: raise ActionFailedError(str(ex)) + + +class JavaGradleCopyLayerArtifactsAction(JavaGradleCopyArtifactsAction): + """ + Java layers does not support using .class files in it. + This action (different from the parent one) copies contents of the layer as jar files and place it + into the artifact folder + """ + + NAME = "JavaGradleCopyLayerArtifacts" + + def _copy_artifacts(self): + lambda_build_output = os.path.join(self.build_dir, "build", "libs") + layer_dependencies = os.path.join(self.build_dir, "build", "distributions", "lambda-build", "lib") + try: + if not self.os_utils.exists(self.artifacts_dir): + self.os_utils.makedirs(self.artifacts_dir) + self.os_utils.copytree( + lambda_build_output, os.path.join(self.artifacts_dir, "lib"), include=jar_file_filter + ) + self.os_utils.copytree(layer_dependencies, os.path.join(self.artifacts_dir, "lib"), include=jar_file_filter) + except Exception as ex: + raise ActionFailedError(str(ex)) diff --git a/aws_lambda_builders/workflows/java_gradle/workflow.py b/aws_lambda_builders/workflows/java_gradle/workflow.py index eacd04fa7..a31c22107 100644 --- a/aws_lambda_builders/workflows/java_gradle/workflow.py +++ b/aws_lambda_builders/workflows/java_gradle/workflow.py @@ -6,9 +6,9 @@ from aws_lambda_builders.actions import CleanUpAction from aws_lambda_builders.workflow import BaseWorkflow, Capability from aws_lambda_builders.workflows.java.actions import JavaCopyDependenciesAction, JavaMoveDependenciesAction -from aws_lambda_builders.workflows.java.utils import OSUtils +from aws_lambda_builders.workflows.java.utils import OSUtils, is_experimental_maven_scope_and_layers_active -from .actions import JavaGradleBuildAction, JavaGradleCopyArtifactsAction +from .actions import JavaGradleBuildAction, JavaGradleCopyArtifactsAction, JavaGradleCopyLayerArtifactsAction from .gradle import SubprocessGradle from .gradle_resolver import GradleResolver from .gradle_validator import GradleValidator @@ -33,9 +33,16 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, **kwar subprocess_gradle = SubprocessGradle(gradle_binary=self.binaries["gradle"], os_utils=self.os_utils) + copy_artifacts_action = JavaGradleCopyArtifactsAction( + source_dir, artifacts_dir, self.build_output_dir, self.os_utils + ) + if self.is_building_layer and is_experimental_maven_scope_and_layers_active(self.experimental_flags): + copy_artifacts_action = JavaGradleCopyLayerArtifactsAction( + source_dir, artifacts_dir, self.build_output_dir, self.os_utils + ) self.actions = [ JavaGradleBuildAction(source_dir, manifest_path, subprocess_gradle, scratch_dir, self.os_utils), - JavaGradleCopyArtifactsAction(source_dir, artifacts_dir, self.build_output_dir, self.os_utils), + copy_artifacts_action, ] if self.dependencies_dir: diff --git a/aws_lambda_builders/workflows/java_maven/DESIGN.md b/aws_lambda_builders/workflows/java_maven/DESIGN.md index 1ec7aeaf5..99c78b117 100644 --- a/aws_lambda_builders/workflows/java_maven/DESIGN.md +++ b/aws_lambda_builders/workflows/java_maven/DESIGN.md @@ -79,12 +79,28 @@ source directory. ```bash mvn clean install -mvn dependency:copy-dependencies -DincludeScope=compile +mvn dependency:copy-dependencies -DincludeScope=runtime ``` +Building artifact for an `AWS::Serverless::LayerVersion` requires different packaging than a +`AWS::Serverless::Function`. [Layers](https://docs.aws.amazon.com/lambda/latest/dg/configuration-layers.html) + use only artifacts under `java/lib/` which differs from Functions in that they in addition allow classes at +the root level similar to normal jar packaging. `JavaMavenLayersWorkflow` handles packaging for Layers and +`JavaMavenWorkflow` handles packaging for Functions. + #### Step 4: Copy to artifact directory -Built Java classes and dependencies are copied from `scratch_dir/target/classes` and `scratch_dir/target/dependency` -to `artifact_dir` and `artifact_dir/lib` respectively. +Built Java classes and dependencies for Functions are copied from `scratch_dir/target/classes` and `scratch_dir/target/dependency` +to `artifact_dir` and `artifact_dir/lib` respectively. Built Java classes and dependencies for Layers are copied from +`scratch_dir/target/*.jar` and `scratch_dir/target/dependency` to `artifact_dir/lib`. Copy all the artifacts +required for runtime execution. + +### Notes on changes of original implementation + +The original implementation was not handling Layers well. Maven has provided a scope called `provided` which is +used to declare that a particular dependency is required for compilation but should not be packaged with the +declaring project artifact. Naturally this is the scope a maven java project would use for artifacts +provided by Layers. Original implementation would package those `provided` scoped entities with the Function, +and thus if a project was using Layers it would have the artifact both in the Layer and in the Function. [Gradle Lambda Builder]:https://github.com/awslabs/aws-lambda-builders/blob/develop/aws_lambda_builders/workflows/java_gradle/DESIGN.md \ No newline at end of file diff --git a/aws_lambda_builders/workflows/java_maven/actions.py b/aws_lambda_builders/workflows/java_maven/actions.py index 169459cda..090ae3259 100644 --- a/aws_lambda_builders/workflows/java_maven/actions.py +++ b/aws_lambda_builders/workflows/java_maven/actions.py @@ -4,9 +4,11 @@ import os import logging +import shutil from aws_lambda_builders.actions import ActionFailedError, BaseAction, Purpose from .maven import MavenExecutionError +from ..java.utils import jar_file_filter LOG = logging.getLogger(__name__) @@ -81,3 +83,33 @@ def _copy_artifacts(self): self.os_utils.copytree(dependency_output, os.path.join(self.artifacts_dir, "lib")) except Exception as ex: raise ActionFailedError(str(ex)) + + +class JavaMavenCopyLayerArtifactsAction(JavaMavenCopyArtifactsAction): + """ + Java layers does not support using .class files in it. + This action (different from the parent one) copies contents of the layer as jar files and place it + into the artifact folder + """ + + NAME = "MavenCopyLayerArtifacts" + IGNORED_FOLDERS = ["classes", "dependency", "generated-sources", "maven-archiver", "maven-status"] + + def _copy_artifacts(self): + lambda_build_output = os.path.join(self.scratch_dir, "target") + dependency_output = os.path.join(self.scratch_dir, "target", "dependency") + + if not self.os_utils.exists(lambda_build_output): + raise ActionFailedError("Required target/classes directory was not produced from 'mvn package'") + + try: + self.os_utils.copytree( + lambda_build_output, + os.path.join(self.artifacts_dir, "lib"), + ignore=shutil.ignore_patterns(*self.IGNORED_FOLDERS), + include=jar_file_filter, + ) + if self.os_utils.exists(dependency_output): + self.os_utils.copytree(dependency_output, os.path.join(self.artifacts_dir, "lib")) + except Exception as ex: + raise ActionFailedError(str(ex)) diff --git a/aws_lambda_builders/workflows/java_maven/maven.py b/aws_lambda_builders/workflows/java_maven/maven.py index da02f7bbb..d50809a92 100644 --- a/aws_lambda_builders/workflows/java_maven/maven.py +++ b/aws_lambda_builders/workflows/java_maven/maven.py @@ -16,13 +16,14 @@ def __init__(self, **kwargs): class SubprocessMaven(object): - def __init__(self, maven_binary, os_utils=None): + def __init__(self, maven_binary, os_utils=None, is_experimental_maven_scope_enabled=False): if maven_binary is None: raise ValueError("Must provide Maven BinaryPath") self.maven_binary = maven_binary if os_utils is None: raise ValueError("Must provide OSUtils") self.os_utils = os_utils + self.is_experimental_maven_scope_enabled = is_experimental_maven_scope_enabled def build(self, scratch_dir): args = ["clean", "install"] @@ -34,7 +35,9 @@ def build(self, scratch_dir): raise MavenExecutionError(message=stdout.decode("utf8").strip()) def copy_dependency(self, scratch_dir): - args = ["dependency:copy-dependencies", "-DincludeScope=compile", "-Dmdep.prependGroupId=true"] + include_scope = "runtime" if self.is_experimental_maven_scope_enabled else "compile" + LOG.debug("Running copy_dependency with scope: %s", include_scope) + args = ["dependency:copy-dependencies", f"-DincludeScope={include_scope}", "-Dmdep.prependGroupId=true"] ret_code, stdout, _ = self._run(args, scratch_dir) if ret_code != 0: diff --git a/aws_lambda_builders/workflows/java_maven/workflow.py b/aws_lambda_builders/workflows/java_maven/workflow.py index 36d6dbb1c..86d20777b 100644 --- a/aws_lambda_builders/workflows/java_maven/workflow.py +++ b/aws_lambda_builders/workflows/java_maven/workflow.py @@ -4,9 +4,14 @@ from aws_lambda_builders.workflow import BaseWorkflow, Capability from aws_lambda_builders.actions import CopySourceAction, CleanUpAction from aws_lambda_builders.workflows.java.actions import JavaCopyDependenciesAction, JavaMoveDependenciesAction -from aws_lambda_builders.workflows.java.utils import OSUtils +from aws_lambda_builders.workflows.java.utils import OSUtils, is_experimental_maven_scope_and_layers_active -from .actions import JavaMavenBuildAction, JavaMavenCopyDependencyAction, JavaMavenCopyArtifactsAction +from .actions import ( + JavaMavenBuildAction, + JavaMavenCopyDependencyAction, + JavaMavenCopyArtifactsAction, + JavaMavenCopyLayerArtifactsAction, +) from .maven import SubprocessMaven from .maven_resolver import MavenResolver from .maven_validator import MavenValidator @@ -29,13 +34,24 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, **kwar self.os_utils = OSUtils() # Assuming root_dir is the same as source_dir for now root_dir = source_dir - subprocess_maven = SubprocessMaven(maven_binary=self.binaries["mvn"], os_utils=self.os_utils) + is_experimental_maven_scope_and_layers_enabled = is_experimental_maven_scope_and_layers_active( + self.experimental_flags + ) + subprocess_maven = SubprocessMaven( + maven_binary=self.binaries["mvn"], + os_utils=self.os_utils, + is_experimental_maven_scope_enabled=is_experimental_maven_scope_and_layers_enabled, + ) + + copy_artifacts_action = JavaMavenCopyArtifactsAction(scratch_dir, artifacts_dir, self.os_utils) + if self.is_building_layer and is_experimental_maven_scope_and_layers_enabled: + copy_artifacts_action = JavaMavenCopyLayerArtifactsAction(scratch_dir, artifacts_dir, self.os_utils) self.actions = [ CopySourceAction(root_dir, scratch_dir, excludes=self.EXCLUDED_FILES), JavaMavenBuildAction(scratch_dir, subprocess_maven), JavaMavenCopyDependencyAction(scratch_dir, subprocess_maven), - JavaMavenCopyArtifactsAction(scratch_dir, artifacts_dir, self.os_utils), + copy_artifacts_action, ] if self.dependencies_dir: diff --git a/tests/functional/test_utils.py b/tests/functional/test_utils.py index fe5c433c6..d4b80b9ad 100644 --- a/tests/functional/test_utils.py +++ b/tests/functional/test_utils.py @@ -40,6 +40,19 @@ def test_must_respect_excludes_list(self): self.assertEqual(set(os.listdir(os.path.join(self.dest, "a"))), {"c"}) self.assertEqual(set(os.listdir(os.path.join(self.dest, "a"))), {"c"}) + def test_must_respect_include_function(self): + file(self.source, "nested", "folder", "file.txt") + file(self.source, "main.pyc") + file(self.source, "file.txt") + + def _include_check(file_name): + return file_name.endswith(".txt") + + copytree(self.source, self.dest, include=_include_check) + self.assertTrue(os.path.exists(os.path.join(self.dest, "nested", "folder", "file.txt"))) + self.assertTrue(os.path.exists(os.path.join(self.dest, "file.txt"))) + self.assertFalse(os.path.exists(os.path.join(self.dest, "main.pyc"))) + def test_must_skip_if_source_folder_does_not_exist(self): copytree(os.path.join(self.source, "some-random-file"), self.dest) self.assertEqual(set(os.listdir(self.dest)), set()) diff --git a/tests/integration/workflows/common_test_utils.py b/tests/integration/workflows/common_test_utils.py index d558eb4c9..80e7f2cda 100644 --- a/tests/integration/workflows/common_test_utils.py +++ b/tests/integration/workflows/common_test_utils.py @@ -2,6 +2,13 @@ from zipfile import ZipFile +def folder_should_not_contain_files(folder, files): + for f in files: + if does_folder_contain_file(folder, f): + return False + return True + + def does_folder_contain_all_files(folder, files): for f in files: if not does_folder_contain_file(folder, f): diff --git a/tests/integration/workflows/java_gradle/test_java_gradle.py b/tests/integration/workflows/java_gradle/test_java_gradle.py index a4aa98840..941ec0f2e 100644 --- a/tests/integration/workflows/java_gradle/test_java_gradle.py +++ b/tests/integration/workflows/java_gradle/test_java_gradle.py @@ -9,7 +9,12 @@ from aws_lambda_builders.builder import LambdaBuilder from aws_lambda_builders.exceptions import WorkflowFailedError -from tests.integration.workflows.common_test_utils import does_folder_contain_all_files, does_folder_contain_file +from aws_lambda_builders.workflows.java.utils import EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG +from tests.integration.workflows.common_test_utils import ( + does_folder_contain_all_files, + does_folder_contain_file, + folder_should_not_contain_files, +) class TestJavaGradle(TestCase): @@ -182,3 +187,50 @@ def test_build_single_build_with_deps_dir_wtihout_combine_dependencies(self): self.assertTrue(does_folder_contain_all_files(self.artifacts_dir, artifact_expected_files)) self.assertTrue(does_folder_contain_all_files(self.dependencies_dir, dependencies_expected_files)) + + def test_build_with_layers_and_scope(self): + # first build layer and validate + self.validate_layer_build() + # then build function which uses this layer as dependency with provided scope + self.validate_function_build() + + def validate_layer_build(self): + layer_source_dir = join(self.SINGLE_BUILD_TEST_DATA_DIR, "layer") + layer_manifest_path = join(layer_source_dir, "build.gradle") + self.builder.build( + layer_source_dir, + self.artifacts_dir, + self.scratch_dir, + layer_manifest_path, + runtime=self.runtime, + is_building_layer=True, + experimental_flags=[EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG], + ) + artifact_expected_files = [ + join("lib", "aws-lambda-java-core-1.2.0.jar"), + join("lib", "common-layer-gradle-1.0.jar"), + ] + self.assertTrue(does_folder_contain_all_files(self.artifacts_dir, artifact_expected_files)) + + def validate_function_build(self): + self.setUp() # re-initialize folders + function_source_dir = join(self.SINGLE_BUILD_TEST_DATA_DIR, "with-layer-deps") + function_manifest_path = join(function_source_dir, "build.gradle") + self.builder.build( + function_source_dir, + self.artifacts_dir, + self.scratch_dir, + function_manifest_path, + runtime=self.runtime, + is_building_layer=False, + experimental_flags=[EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG], + ) + artifact_expected_files = [ + join("aws", "lambdabuilders", "Main.class"), + ] + artifact_not_expected_files = [ + join("lib", "com.amazonaws.aws-lambda-java-core-1.2.0.jar"), + join("lib", "common-layer-1.0.jar"), + ] + self.assertTrue(does_folder_contain_all_files(self.artifacts_dir, artifact_expected_files)) + self.assertTrue(folder_should_not_contain_files(self.artifacts_dir, artifact_not_expected_files)) diff --git a/tests/integration/workflows/java_gradle/testdata/single-build/layer/build.gradle b/tests/integration/workflows/java_gradle/testdata/single-build/layer/build.gradle new file mode 100644 index 000000000..004530e2e --- /dev/null +++ b/tests/integration/workflows/java_gradle/testdata/single-build/layer/build.gradle @@ -0,0 +1,31 @@ +plugins { + id 'java' + id 'java-library' + id 'maven-publish' +} + +repositories { + mavenLocal() + maven { + url = uri('https://repo.maven.apache.org/maven2/') + } +} + +dependencies { + api 'com.amazonaws:aws-lambda-java-core:1.2.0' +} + +group = 'aws.lambdabuilders' +version = '1.0' +description = 'common-layer-gradle' +java.sourceCompatibility = JavaVersion.VERSION_1_8 + +build.finalizedBy publishToMavenLocal + +publishing { + publications { + maven(MavenPublication) { + from(components.java) + } + } +} diff --git a/tests/integration/workflows/java_gradle/testdata/single-build/layer/settings.gradle b/tests/integration/workflows/java_gradle/testdata/single-build/layer/settings.gradle new file mode 100644 index 000000000..c7a980f9f --- /dev/null +++ b/tests/integration/workflows/java_gradle/testdata/single-build/layer/settings.gradle @@ -0,0 +1 @@ +rootProject.name = 'common-layer-gradle' \ No newline at end of file diff --git a/tests/integration/workflows/java_gradle/testdata/single-build/layer/src/main/java/aws/lambdabuilders/CommonCode.java b/tests/integration/workflows/java_gradle/testdata/single-build/layer/src/main/java/aws/lambdabuilders/CommonCode.java new file mode 100644 index 000000000..64466d0e9 --- /dev/null +++ b/tests/integration/workflows/java_gradle/testdata/single-build/layer/src/main/java/aws/lambdabuilders/CommonCode.java @@ -0,0 +1,10 @@ +package aws.lambdabuilders; + +import com.amazonaws.services.lambda.runtime.LambdaLogger; + +public class CommonCode { + + public static void doSomethingOnLayer(final LambdaLogger logger, final String s) { + logger.log("Doing something on layer" + s); + } +} diff --git a/tests/integration/workflows/java_gradle/testdata/single-build/with-layer-deps/build.gradle b/tests/integration/workflows/java_gradle/testdata/single-build/with-layer-deps/build.gradle new file mode 100644 index 000000000..2e1f347a3 --- /dev/null +++ b/tests/integration/workflows/java_gradle/testdata/single-build/with-layer-deps/build.gradle @@ -0,0 +1,28 @@ +plugins { + id 'java' + id 'maven-publish' +} + +repositories { + mavenLocal() + maven { + url = uri('https://repo.maven.apache.org/maven2/') + } +} + +dependencies { + compileOnly 'aws.lambdabuilders:common-layer-gradle:1.0' +} + +group = 'helloworld' +version = '1.0' +description = 'A sample Hello World created for SAM CLI.' +java.sourceCompatibility = JavaVersion.VERSION_1_8 + +publishing { + publications { + maven(MavenPublication) { + from(components.java) + } + } +} diff --git a/tests/integration/workflows/java_gradle/testdata/single-build/with-layer-deps/src/main/java/aws/lambdabuilders/Main.java b/tests/integration/workflows/java_gradle/testdata/single-build/with-layer-deps/src/main/java/aws/lambdabuilders/Main.java new file mode 100644 index 000000000..d68611b84 --- /dev/null +++ b/tests/integration/workflows/java_gradle/testdata/single-build/with-layer-deps/src/main/java/aws/lambdabuilders/Main.java @@ -0,0 +1,16 @@ +package aws.lambdabuilders; + +import com.amazonaws.services.lambda.runtime.Context; +import com.amazonaws.services.lambda.runtime.LambdaLogger; +import com.amazonaws.services.lambda.runtime.RequestHandler; + +import aws.lambdabuilders.CommonCode; + +public class Main implements RequestHandler { + public Object handleRequest(final Object input, final Context context) { + final LambdaLogger logger = context.getLogger(); + CommonCode.doSomethingOnLayer(logger, "fromLambdaFunction"); + System.out.println("Hello AWS Lambda Builders!"); + return "Done"; + } +} diff --git a/tests/integration/workflows/java_maven/test_java_maven.py b/tests/integration/workflows/java_maven/test_java_maven.py index 4aa1bd6f9..d661b2ed6 100644 --- a/tests/integration/workflows/java_maven/test_java_maven.py +++ b/tests/integration/workflows/java_maven/test_java_maven.py @@ -8,7 +8,12 @@ from aws_lambda_builders.builder import LambdaBuilder from aws_lambda_builders.exceptions import WorkflowFailedError -from tests.integration.workflows.common_test_utils import does_folder_contain_all_files, does_folder_contain_file +from aws_lambda_builders.workflows.java.utils import EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG +from tests.integration.workflows.common_test_utils import ( + does_folder_contain_all_files, + does_folder_contain_file, + folder_should_not_contain_files, +) class TestJavaMaven(TestCase): @@ -112,3 +117,52 @@ def test_build_single_build_with_deps_resources_exclude_test_jars_deps_dir_witho self.assertTrue(does_folder_contain_all_files(self.dependencies_dir, dependencies_expected_files)) self.assertFalse(does_folder_contain_file(self.artifacts_dir, join("lib", "junit-4.12.jar"))) self.assert_src_dir_not_touched(source_dir) + + def test_build_with_layers_and_scope(self): + # first build layer and validate + self.validate_layer_build() + # then build function which uses this layer as dependency with provided scope + self.validate_function_build() + + def validate_layer_build(self): + layer_source_dir = join(self.SINGLE_BUILD_TEST_DATA_DIR, "layer") + layer_manifest_path = join(layer_source_dir, "pom.xml") + self.builder.build( + layer_source_dir, + self.artifacts_dir, + self.scratch_dir, + layer_manifest_path, + runtime=self.runtime, + is_building_layer=True, + experimental_flags=[EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG], + ) + artifact_expected_files = [ + join("lib", "com.amazonaws.aws-lambda-java-core-1.2.0.jar"), + join("lib", "common-layer-1.0.jar"), + ] + self.assertTrue(does_folder_contain_all_files(self.artifacts_dir, artifact_expected_files)) + self.assert_src_dir_not_touched(layer_source_dir) + + def validate_function_build(self): + self.setUp() # re-initialize folders + function_source_dir = join(self.SINGLE_BUILD_TEST_DATA_DIR, "with-layer-deps") + function_manifest_path = join(function_source_dir, "pom.xml") + self.builder.build( + function_source_dir, + self.artifacts_dir, + self.scratch_dir, + function_manifest_path, + runtime=self.runtime, + is_building_layer=False, + experimental_flags=[EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG], + ) + artifact_expected_files = [ + join("aws", "lambdabuilders", "Main.class"), + ] + artifact_not_expected_files = [ + join("lib", "com.amazonaws.aws-lambda-java-core-1.2.0.jar"), + join("lib", "common-layer-1.0.jar"), + ] + self.assertTrue(does_folder_contain_all_files(self.artifacts_dir, artifact_expected_files)) + self.assertTrue(folder_should_not_contain_files(self.artifacts_dir, artifact_not_expected_files)) + self.assert_src_dir_not_touched(function_source_dir) diff --git a/tests/integration/workflows/java_maven/testdata/single-build/layer/pom.xml b/tests/integration/workflows/java_maven/testdata/single-build/layer/pom.xml new file mode 100644 index 000000000..7252c868e --- /dev/null +++ b/tests/integration/workflows/java_maven/testdata/single-build/layer/pom.xml @@ -0,0 +1,26 @@ + + + 4.0.0 + + aws.lambdabuilders + common-layer + 1.0 + jar + + + 1.8 + 1.8 + + + + + + com.amazonaws + aws-lambda-java-core + 1.2.0 + + + + \ No newline at end of file diff --git a/tests/integration/workflows/java_maven/testdata/single-build/layer/src/main/java/aws/lambdabuilders/CommonCode.java b/tests/integration/workflows/java_maven/testdata/single-build/layer/src/main/java/aws/lambdabuilders/CommonCode.java new file mode 100644 index 000000000..64466d0e9 --- /dev/null +++ b/tests/integration/workflows/java_maven/testdata/single-build/layer/src/main/java/aws/lambdabuilders/CommonCode.java @@ -0,0 +1,10 @@ +package aws.lambdabuilders; + +import com.amazonaws.services.lambda.runtime.LambdaLogger; + +public class CommonCode { + + public static void doSomethingOnLayer(final LambdaLogger logger, final String s) { + logger.log("Doing something on layer" + s); + } +} diff --git a/tests/integration/workflows/java_maven/testdata/single-build/with-layer-deps/pom.xml b/tests/integration/workflows/java_maven/testdata/single-build/with-layer-deps/pom.xml new file mode 100644 index 000000000..a932b0253 --- /dev/null +++ b/tests/integration/workflows/java_maven/testdata/single-build/with-layer-deps/pom.xml @@ -0,0 +1,22 @@ + + 4.0.0 + helloworld + HelloWorld + 1.0 + jar + A sample Hello World created for SAM CLI. + + 1.8 + 1.8 + + + + + aws.lambdabuilders + common-layer + 1.0 + provided + + + diff --git a/tests/integration/workflows/java_maven/testdata/single-build/with-layer-deps/src/main/java/aws/lambdabuilders/Main.java b/tests/integration/workflows/java_maven/testdata/single-build/with-layer-deps/src/main/java/aws/lambdabuilders/Main.java new file mode 100644 index 000000000..d68611b84 --- /dev/null +++ b/tests/integration/workflows/java_maven/testdata/single-build/with-layer-deps/src/main/java/aws/lambdabuilders/Main.java @@ -0,0 +1,16 @@ +package aws.lambdabuilders; + +import com.amazonaws.services.lambda.runtime.Context; +import com.amazonaws.services.lambda.runtime.LambdaLogger; +import com.amazonaws.services.lambda.runtime.RequestHandler; + +import aws.lambdabuilders.CommonCode; + +public class Main implements RequestHandler { + public Object handleRequest(final Object input, final Context context) { + final LambdaLogger logger = context.getLogger(); + CommonCode.doSomethingOnLayer(logger, "fromLambdaFunction"); + System.out.println("Hello AWS Lambda Builders!"); + return "Done"; + } +} diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 2dd9b63ed..3b6c55047 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -125,6 +125,7 @@ def setUp(self): [True, False], # download_dependencies [None, "dependency_dir"], # dependency_dir [True, False], # combine_dependencies + [True, False], # is_building_layer [None, [], ["a", "b"]], # experimental flags ) ) @@ -136,6 +137,7 @@ def test_with_mocks( download_dependencies, dependency_dir, combine_dependencies, + is_building_layer, experimental_flags, get_workflow_mock, os_mock, @@ -163,6 +165,7 @@ def test_with_mocks( download_dependencies=download_dependencies, dependencies_dir=dependency_dir, combine_dependencies=combine_dependencies, + is_building_layer=is_building_layer, experimental_flags=experimental_flags, ) @@ -180,6 +183,7 @@ def test_with_mocks( download_dependencies=download_dependencies, dependencies_dir=dependency_dir, combine_dependencies=combine_dependencies, + is_building_layer=is_building_layer, experimental_flags=experimental_flags, ) workflow_instance.run.assert_called_once() diff --git a/tests/unit/workflows/java/test_utils.py b/tests/unit/workflows/java/test_utils.py new file mode 100644 index 000000000..473b686a5 --- /dev/null +++ b/tests/unit/workflows/java/test_utils.py @@ -0,0 +1,33 @@ +from unittest import TestCase + +from parameterized import parameterized + +from aws_lambda_builders.workflows.java.utils import ( + jar_file_filter, + EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG, + is_experimental_maven_scope_and_layers_active, +) + + +class TestJavaUtils(TestCase): + @parameterized.expand( + [ + (None, False), + (123, False), + ("not_a_jar_file.txt", False), + ("jar_file.jar", True), + ] + ) + def test_jar_file_filter(self, file_name, expected): + self.assertEqual(jar_file_filter(file_name), expected) + + @parameterized.expand( + [ + (None, False), + ([], False), + ([EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG], True), + ([EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG, "SomeOtherFlag"], True), + ] + ) + def test_experimental_maven_scope_and_layers_check(self, experimental_flags, expected): + self.assertEqual(is_experimental_maven_scope_and_layers_active(experimental_flags), expected) diff --git a/tests/unit/workflows/java_gradle/test_actions.py b/tests/unit/workflows/java_gradle/test_actions.py index 430446786..01a2be459 100644 --- a/tests/unit/workflows/java_gradle/test_actions.py +++ b/tests/unit/workflows/java_gradle/test_actions.py @@ -1,12 +1,14 @@ from unittest import TestCase -from mock import patch +from mock import patch, call import os from aws_lambda_builders.actions import ActionFailedError +from aws_lambda_builders.workflows.java.utils import jar_file_filter from aws_lambda_builders.workflows.java_gradle.actions import ( JavaGradleBuildAction, JavaGradleCopyArtifactsAction, GradleExecutionError, + JavaGradleCopyLayerArtifactsAction, ) @@ -89,3 +91,24 @@ def test_error_in_artifact_copy_raises_action_error(self): with self.assertRaises(ActionFailedError) as raised: action.execute() self.assertEqual(raised.exception.args[0], "scandir failed!") + + +class TestJavaGradleCopyLayerArtifactsAction(TestJavaGradleCopyArtifactsAction): + def test_copies_artifacts(self): + action = JavaGradleCopyLayerArtifactsAction(self.source_dir, self.artifacts_dir, self.build_dir, self.os_utils) + action.execute() + + self.os_utils.copytree.assert_has_calls( + [ + call( + os.path.join(self.build_dir, "build", "libs"), + os.path.join(self.artifacts_dir, "lib"), + include=jar_file_filter, + ), + call( + os.path.join(self.build_dir, "build", "distributions", "lambda-build", "lib"), + os.path.join(self.artifacts_dir, "lib"), + include=jar_file_filter, + ), + ] + ) diff --git a/tests/unit/workflows/java_gradle/test_workflow.py b/tests/unit/workflows/java_gradle/test_workflow.py index d72ef0d3c..f538b8db4 100644 --- a/tests/unit/workflows/java_gradle/test_workflow.py +++ b/tests/unit/workflows/java_gradle/test_workflow.py @@ -5,8 +5,13 @@ from aws_lambda_builders.actions import CleanUpAction from aws_lambda_builders.workflows.java.actions import JavaMoveDependenciesAction, JavaCopyDependenciesAction +from aws_lambda_builders.workflows.java.utils import EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG from aws_lambda_builders.workflows.java_gradle.workflow import JavaGradleWorkflow -from aws_lambda_builders.workflows.java_gradle.actions import JavaGradleBuildAction, JavaGradleCopyArtifactsAction +from aws_lambda_builders.workflows.java_gradle.actions import ( + JavaGradleBuildAction, + JavaGradleCopyArtifactsAction, + JavaGradleCopyLayerArtifactsAction, +) from aws_lambda_builders.workflows.java_gradle.gradle_resolver import GradleResolver from aws_lambda_builders.workflows.java_gradle.gradle_validator import GradleValidator from aws_lambda_builders.architecture import ARM64 @@ -92,3 +97,19 @@ def test_must_validate_architecture(self): self.assertEqual(workflow.architecture, "x86_64") self.assertEqual(workflow_with_arm.architecture, "arm64") + + def test_workflow_sets_up_gradle_actions_for_layers_experimental(self): + workflow = JavaGradleWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + is_building_layer=True, + experimental_flags=[EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG], + ) + + self.assertEqual(len(workflow.actions), 2) + + self.assertIsInstance(workflow.actions[0], JavaGradleBuildAction) + + self.assertIsInstance(workflow.actions[1], JavaGradleCopyLayerArtifactsAction) diff --git a/tests/unit/workflows/java_maven/test_actions.py b/tests/unit/workflows/java_maven/test_actions.py index 38a720365..2aded7f4f 100644 --- a/tests/unit/workflows/java_maven/test_actions.py +++ b/tests/unit/workflows/java_maven/test_actions.py @@ -1,13 +1,16 @@ +import shutil from unittest import TestCase -from mock import patch, call +from mock import patch, call, ANY import os from aws_lambda_builders.actions import ActionFailedError +from aws_lambda_builders.workflows.java.utils import jar_file_filter from aws_lambda_builders.workflows.java_maven.actions import ( JavaMavenBuildAction, JavaMavenCopyArtifactsAction, JavaMavenCopyDependencyAction, MavenExecutionError, + JavaMavenCopyLayerArtifactsAction, ) @@ -103,3 +106,43 @@ def test_missing_required_target_class_directory_raises_action_error(self): self.assertEqual( raised.exception.args[0], "Required target/classes directory was not " "produced from 'mvn package'" ) + + +class TestJavaMavenCopyLayerArtifactsAction(TestJavaMavenCopyArtifactsAction): + def test_copies_artifacts_no_deps(self): + self.os_utils.exists.return_value = True + + action = JavaMavenCopyLayerArtifactsAction(self.scratch_dir, self.artifacts_dir, self.os_utils) + action.execute() + + self.os_utils.copytree.assert_has_calls( + [ + call( + os.path.join(self.scratch_dir, "target"), + os.path.join(self.artifacts_dir, "lib"), + ignore=ANY, + include=jar_file_filter, + ) + ] + ) + + def test_copies_artifacts_with_deps(self): + self.os_utils.exists.return_value = True + os.path.join(self.scratch_dir, "target", "dependency") + + action = JavaMavenCopyLayerArtifactsAction(self.scratch_dir, self.artifacts_dir, self.os_utils) + action.execute() + self.os_utils.copytree.assert_has_calls( + [ + call( + os.path.join(self.scratch_dir, "target"), + os.path.join(self.artifacts_dir, "lib"), + ignore=ANY, + include=jar_file_filter, + ), + call( + os.path.join(self.scratch_dir, "target", "dependency"), + os.path.join(self.artifacts_dir, "lib"), + ), + ] + ) diff --git a/tests/unit/workflows/java_maven/test_maven.py b/tests/unit/workflows/java_maven/test_maven.py index d7b5c51e5..28a3ca9d3 100644 --- a/tests/unit/workflows/java_maven/test_maven.py +++ b/tests/unit/workflows/java_maven/test_maven.py @@ -74,3 +74,15 @@ def test_copy_dependency_raises_exception_if_retcode_not_0(self): with self.assertRaises(MavenExecutionError) as err: maven.copy_dependency(self.source_dir) self.assertEqual(err.exception.args[0], "Maven Failed: Some Error Message") + + def test_experimental_scope(self): + maven = SubprocessMaven( + maven_binary=self.maven_binary, os_utils=self.os_utils, is_experimental_maven_scope_enabled=True + ) + maven.copy_dependency(self.source_dir) + self.os_utils.popen.assert_called_with( + [self.maven_path, "dependency:copy-dependencies", "-DincludeScope=runtime", "-Dmdep.prependGroupId=true"], + cwd=self.source_dir, + stderr=subprocess.PIPE, + stdout=subprocess.PIPE, + ) diff --git a/tests/unit/workflows/java_maven/test_workflow.py b/tests/unit/workflows/java_maven/test_workflow.py index 2bea4d114..3e843c75e 100644 --- a/tests/unit/workflows/java_maven/test_workflow.py +++ b/tests/unit/workflows/java_maven/test_workflow.py @@ -1,11 +1,14 @@ from unittest import TestCase +from mock import patch, ANY from aws_lambda_builders.workflows.java.actions import JavaCopyDependenciesAction, JavaMoveDependenciesAction +from aws_lambda_builders.workflows.java.utils import EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG from aws_lambda_builders.workflows.java_maven.workflow import JavaMavenWorkflow from aws_lambda_builders.workflows.java_maven.actions import ( JavaMavenBuildAction, JavaMavenCopyArtifactsAction, JavaMavenCopyDependencyAction, + JavaMavenCopyLayerArtifactsAction, ) from aws_lambda_builders.actions import CopySourceAction, CleanUpAction from aws_lambda_builders.workflows.java_maven.maven_resolver import MavenResolver @@ -102,3 +105,25 @@ def test_must_validate_architecture(self): self.assertEqual(workflow.architecture, "x86_64") self.assertEqual(workflow_with_arm.architecture, "arm64") + + @patch("aws_lambda_builders.workflows.java_maven.workflow.SubprocessMaven") + def test_workflow_sets_up_maven_actions_with_combine_dependencies(self, patched_maven_process): + workflow = JavaMavenWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + is_building_layer=True, + experimental_flags=[EXPERIMENTAL_MAVEN_SCOPE_AND_LAYER_FLAG], + ) + + patched_maven_process.assert_called_with( + maven_binary=ANY, os_utils=ANY, is_experimental_maven_scope_enabled=True + ) + + self.assertEqual(len(workflow.actions), 4) + + self.assertIsInstance(workflow.actions[0], CopySourceAction) + self.assertIsInstance(workflow.actions[1], JavaMavenBuildAction) + self.assertIsInstance(workflow.actions[2], JavaMavenCopyDependencyAction) + self.assertIsInstance(workflow.actions[3], JavaMavenCopyLayerArtifactsAction) From cc1336d782bf54244828258a837e37e6559b2541 Mon Sep 17 00:00:00 2001 From: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Date: Thu, 3 Feb 2022 15:28:19 -0800 Subject: [PATCH 4/6] chore: update integration tests dependencies (#323) --- .../go_dep/data/src/failed-remote/Gopkg.lock | 73 +++++++++++++++++-- .../go_dep/data/src/failed-remote/Gopkg.toml | 5 +- .../workflows/nodejs_npm/test_nodejs_npm.py | 2 +- .../testdata/broken-deps/package.json | 2 +- .../workflows/python_pip/test_python_pip.py | 4 +- .../testdata/requirements-invalid.txt | 2 +- 6 files changed, 75 insertions(+), 13 deletions(-) diff --git a/tests/integration/workflows/go_dep/data/src/failed-remote/Gopkg.lock b/tests/integration/workflows/go_dep/data/src/failed-remote/Gopkg.lock index 5f571753d..91b5ced1c 100644 --- a/tests/integration/workflows/go_dep/data/src/failed-remote/Gopkg.lock +++ b/tests/integration/workflows/go_dep/data/src/failed-remote/Gopkg.lock @@ -2,16 +2,77 @@ [[projects]] - digest = "1:69b1cc331fca23d702bd72f860c6a647afd0aa9fcbc1d0659b1365e26546dd70" - name = "not-really-a-git-repo.com/pkg/log" - packages = ["."] + digest = "1:e49c533579c1f736baa81f294d2c8e1cf911b47ee2611a0b7ee1b84b32ff2acc" + name = "github.com/aws/aws-sdk-go-v2" + packages = [ + ".", + "aws", + "aws/defaults", + "aws/middleware", + "aws/protocol/query", + "aws/protocol/restjson", + "aws/protocol/xml", + "aws/ratelimit", + "aws/retry", + "aws/signer/internal/v4", + "aws/signer/v4", + "aws/transport/http", + "config", + "credentials", + "credentials/ec2rolecreds", + "credentials/endpointcreds", + "credentials/endpointcreds/internal/client", + "credentials/processcreds", + "credentials/ssocreds", + "credentials/stscreds", + "feature/ec2/imds", + "feature/ec2/imds/internal/config", + "internal/configsources", + "internal/endpoints/v2", + "internal/ini", + "internal/rand", + "internal/sdk", + "internal/sdkio", + "internal/strings", + "internal/sync/singleflight", + "internal/timeconv", + "service/internal/presigned-url", + "service/sso", + "service/sso/internal/endpoints", + "service/sso/types", + "service/sts", + "service/sts/internal/endpoints", + "service/sts/types", + ] pruneopts = "UT" - revision = "bcd833dfe83d3cebad139e4a29ed79cb2318bf95" - version = "v1.0.0" + revision = "e10c0d2c8db721bd0e3b16070f3a74f6bc7171de" + version = "v1.13.0" + +[[projects]] + digest = "1:eecb561f241c16676e4afb9e35ea545b5bb815edd403713727e3b4920fa667dd" + name = "github.com/aws/smithy-go" + packages = [ + ".", + "document", + "encoding", + "encoding/httpbinding", + "encoding/xml", + "io", + "logging", + "middleware", + "ptr", + "rand", + "time", + "transport/http", + "transport/http/internal/io", + ] + pruneopts = "UT" + revision = "7d70c9b6e1b77bb0d8df4e2e6d5e787cd76aedf6" + version = "v1.10.0" [solve-meta] analyzer-name = "dep" analyzer-version = 1 - input-imports = ["not-really-a-git-repo.com/pkg/log"] + input-imports = ["github.com/aws/aws-sdk-go-v2/config"] solver-name = "gps-cdcl" solver-version = 1 diff --git a/tests/integration/workflows/go_dep/data/src/failed-remote/Gopkg.toml b/tests/integration/workflows/go_dep/data/src/failed-remote/Gopkg.toml index ec6996018..688ed46d0 100644 --- a/tests/integration/workflows/go_dep/data/src/failed-remote/Gopkg.toml +++ b/tests/integration/workflows/go_dep/data/src/failed-remote/Gopkg.toml @@ -1,7 +1,8 @@ [[constraint]] - name = "not-really-a-git-repo.com/pkg/log" - version = "1.0.0" + name = "github.com/aws/aws-sdk-go-v2" + version = "1.12.99" [prune] go-tests = true unused-packages = true + diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index 82da7b576..4a4cdb000 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -133,7 +133,7 @@ def test_fails_if_npm_cannot_resolve_dependencies(self): runtime=self.runtime, ) - self.assertIn("No matching version found for minimal-request-promise@0.0.0-NON_EXISTENT", str(ctx.exception)) + self.assertIn("No matching version found for aws-sdk@2.997.999", str(ctx.exception)) def test_builds_project_with_remote_dependencies_without_download_dependencies_with_dependencies_dir(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "npm-deps") diff --git a/tests/integration/workflows/nodejs_npm/testdata/broken-deps/package.json b/tests/integration/workflows/nodejs_npm/testdata/broken-deps/package.json index d7c526360..287373526 100644 --- a/tests/integration/workflows/nodejs_npm/testdata/broken-deps/package.json +++ b/tests/integration/workflows/nodejs_npm/testdata/broken-deps/package.json @@ -7,6 +7,6 @@ "author": "", "license": "APACHE2.0", "dependencies": { - "minimal-request-promise": "0.0.0-NON_EXISTENT" + "aws-sdk": "2.997.999" } } diff --git a/tests/integration/workflows/python_pip/test_python_pip.py b/tests/integration/workflows/python_pip/test_python_pip.py index 43ac86ffc..de759be47 100644 --- a/tests/integration/workflows/python_pip/test_python_pip.py +++ b/tests/integration/workflows/python_pip/test_python_pip.py @@ -164,9 +164,9 @@ def test_must_fail_to_resolve_dependencies(self): # In Python2 a 'u' is now added to the exception string. To account for this, we see if either one is in the # output - message_in_exception = "Invalid requirement: 'adfasf=1.2.3'" in str( + message_in_exception = "Invalid requirement: 'boto3=1.19.99'" in str( ctx.exception - ) or "Invalid requirement: u'adfasf=1.2.3'" in str(ctx.exception) + ) or "Invalid requirement: u'boto3=1.19.99'" in str(ctx.exception) self.assertTrue(message_in_exception) def test_must_log_warning_if_requirements_not_found(self): diff --git a/tests/integration/workflows/python_pip/testdata/requirements-invalid.txt b/tests/integration/workflows/python_pip/testdata/requirements-invalid.txt index 3a61c5980..4d8477cac 100644 --- a/tests/integration/workflows/python_pip/testdata/requirements-invalid.txt +++ b/tests/integration/workflows/python_pip/testdata/requirements-invalid.txt @@ -1 +1 @@ -adfasf=1.2.3 \ No newline at end of file +boto3=1.19.99 \ No newline at end of file From 9c0a45add1217af62ccc6bf00dfa3eb216aa229d Mon Sep 17 00:00:00 2001 From: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Date: Fri, 4 Feb 2022 09:06:21 -0800 Subject: [PATCH 5/6] feat: Esbuild Implementation Changes (#322) * Use scratch dir. Read props from options * Docstrings and black reformat * Add tests * Add logic for accepting implicit entry points * Uncomment feature flag * Black reformat * Address comments * Fix integration test Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> --- .../workflows/nodejs_npm/DESIGN.md | 161 ------------------ .../workflows/nodejs_npm_esbuild/DESIGN.md | 140 +++++++++++++++ .../workflows/nodejs_npm_esbuild/actions.py | 62 +++++-- .../workflows/nodejs_npm_esbuild/esbuild.py | 11 +- .../workflows/nodejs_npm_esbuild/workflow.py | 75 ++++---- .../test_nodejs_npm_with_esbuild.py | 65 ++++++- .../testdata/broken-package/excluded.js | 2 - .../testdata/broken-package/included.js | 2 - .../testdata/broken-package/package.json | 9 - .../testdata/implicit-file-types/implicit.js | 1 + .../testdata/implicit-file-types/included.ts | 9 + .../implicit-file-types/package-lock.json | 46 +++++ .../testdata/implicit-file-types/package.json | 14 ++ .../testdata/no-deps-esbuild/package.json | 6 +- .../testdata/no-package-esbuild/included.js | 1 + .../package.json | 4 - .../with-deps-esbuild-typescript/package.json | 4 - .../testdata/with-deps-esbuild/package.json | 4 - .../nodejs_npm_esbuild/test_actions.py | 40 +++++ .../nodejs_npm_esbuild/test_workflow.py | 41 ++--- 20 files changed, 419 insertions(+), 278 deletions(-) create mode 100644 aws_lambda_builders/workflows/nodejs_npm_esbuild/DESIGN.md delete mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/excluded.js delete mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/included.js delete mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/package.json create mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/implicit.js create mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/included.ts create mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/package-lock.json create mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/package.json create mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/no-package-esbuild/included.js diff --git a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md index 72f7aa44d..b5aa470ce 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md +++ b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md @@ -119,8 +119,6 @@ always downloading all the dependencies. 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 @@ -130,9 +128,6 @@ 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. @@ -165,7 +160,6 @@ 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 @@ -226,158 +220,3 @@ _(out of scope for the current version)_ To fully support dependencies that download or compile binaries for a target platform, this step 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. - - -### 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 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` -in the artifacts folder. - -```json -{ - "name": "nodeps-esbuild", - "version": "1.0.0", - "license": "APACHE2.0", - "aws_sam": { - "bundler": "esbuild", - "entry_points": ["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", - "entry_points": ["lambda.js"] - }, - "devDependencies": { - "esbuild": "^0.11.23" - } -} -``` - -For a full example, see the [`with-deps-esbuild`](../../../tests/integration/workflows/nodejs_npm_esbuild/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", - "entry_points": ["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_esbuild/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", - "entry_points": ["included.ts"], - "target": "node10", - "minify": false, - "sourcemap": false - } -} diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm_esbuild/DESIGN.md new file mode 100644 index 000000000..948eacef2 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/DESIGN.md @@ -0,0 +1,140 @@ +## NodeJS - NPM Lambda Builder Using `esbuild` + +### Scope + +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 +deployable to AWS Lambda. It will then bundle the code using `esbuild` with the properties +passed in through the builder options field. + +### Additional Tools + +Packaging with a bundler requires installing additional tools (eg `esbuild`). + +### 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. + +### Activating the bundler workflow + +The workflow can be activated by using the `("nodejs", "npm-esbuild", None)` Capability. +The distinguishing factor being the `npm-esbuild` dependency-manager property of the builder. + +An entrypoint or entrypoints array must be included in the options passed +to Lambda Builders for this workflow to succeed. + +The following example is a minimal options object that can be passed to +the esbuild workflow, starting from `lambda.js`. It will produce a bundled `lambda.js` +in the artifacts folder. + +```json +{ + "options": { + "entry_points": ["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", + "devDependencies": { + "esbuild": "^0.11.23" + } +} +``` + +For a full example, see the [`with-deps-esbuild`](../../../tests/integration/workflows/nodejs_npm_esbuild/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. + +If no file type is provided for the entrypoint, esbuild will first look for a +TypeScript file, and the a JavaScript file with the given filename. + +```js +{ + "name": "with-deps-esbuild-typescript", + "version": "1.0.0", + "license": "APACHE2.0", + "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_esbuild/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 +{ + "entry_points": ["included.ts"], + "target": "node10", + "minify": false, + "sourcemap": false +} diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py index f620ff717..ebcaf5227 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py @@ -2,6 +2,8 @@ Actions specific to the esbuild bundler """ import logging +from pathlib import Path + from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError from .esbuild import EsbuildExecutionError @@ -19,11 +21,12 @@ class EsbuildBundleAction(BaseAction): 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 + ENTRY_POINTS = "entry_points" + def __init__(self, scratch_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild): + """ + :type scratch_dir: str + :param scratch_dir: an existing (writable) directory for temporary files :type artifacts_dir: str :param artifacts_dir: an existing (writable) directory where to store the output. @@ -36,7 +39,7 @@ def __init__(self, source_dir, artifacts_dir, bundler_config, osutils, subproces :param subprocess_esbuild: An instance of the Esbuild process wrapper """ super(EsbuildBundleAction, self).__init__() - self.source_dir = source_dir + self.scratch_dir = scratch_dir self.artifacts_dir = artifacts_dir self.bundler_config = bundler_config self.osutils = osutils @@ -49,26 +52,26 @@ def execute(self): :raises lambda_builders.actions.ActionFailedError: when esbuild packaging fails """ - if "entry_points" not in self.bundler_config: - raise ActionFailedError("entry_points not set ({})".format(self.bundler_config)) + if self.ENTRY_POINTS not in self.bundler_config: + raise ActionFailedError(f"{self.ENTRY_POINTS} not set ({self.bundler_config})") - entry_points = self.bundler_config["entry_points"] + entry_points = self.bundler_config[self.ENTRY_POINTS] if not isinstance(entry_points, list): - raise ActionFailedError("entry_points must be a list ({})".format(self.bundler_config)) + raise ActionFailedError(f"{self.ENTRY_POINTS} must be a list ({self.bundler_config})") if not entry_points: - raise ActionFailedError("entry_points must not be empty ({})".format(self.bundler_config)) + raise ActionFailedError(f"{self.ENTRY_POINTS} must not be empty ({self.bundler_config})") - entry_paths = [self.osutils.joinpath(self.source_dir, entry_point) for entry_point in entry_points] + entry_paths = [self.osutils.joinpath(self.scratch_dir, entry_point) for entry_point in entry_points] LOG.debug("NODEJS building %s using esbuild to %s", entry_paths, self.artifacts_dir) - for entry_point in entry_paths: - if not self.osutils.file_exists(entry_point): - raise ActionFailedError("entry point {} does not exist".format(entry_point)) + explicit_entry_points = [] + for entry_path, entry_point in zip(entry_paths, entry_points): + explicit_entry_points.append(self._get_explicit_file_type(entry_point, entry_path)) - args = entry_points + ["--bundle", "--platform=node", "--format=cjs"] + args = explicit_entry_points + ["--bundle", "--platform=node", "--format=cjs"] minify = self.bundler_config.get("minify", True) sourcemap = self.bundler_config.get("sourcemap", True) target = self.bundler_config.get("target", "es2020") @@ -79,6 +82,33 @@ def execute(self): args.append("--target={}".format(target)) args.append("--outdir={}".format(self.artifacts_dir)) try: - self.subprocess_esbuild.run(args, cwd=self.source_dir) + self.subprocess_esbuild.run(args, cwd=self.scratch_dir) except EsbuildExecutionError as ex: raise ActionFailedError(str(ex)) + + def _get_explicit_file_type(self, entry_point, entry_path): + """ + Get an entry point with an explicit .ts or .js suffix. + + :type entry_point: str + :param entry_point: path to entry file from code uri + + :type entry_path: str + :param entry_path: full path of entry file + + :rtype: str + :return: entry point with appropriate file extension + + :raises lambda_builders.actions.ActionFailedError: when esbuild packaging fails + """ + if Path(entry_point).suffix: + if self.osutils.file_exists(entry_path): + return entry_point + raise ActionFailedError("entry point {} does not exist".format(entry_path)) + + for ext in [".ts", ".js"]: + entry_path_with_ext = entry_path + ext + if self.osutils.file_exists(entry_path_with_ext): + return entry_point + ext + + raise ActionFailedError("entry point {} does not exist".format(entry_path)) diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild.py index 992c49fe5..d1bf92b25 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild.py +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild.py @@ -4,21 +4,20 @@ import logging +from aws_lambda_builders.exceptions import LambdaBuilderError + LOG = logging.getLogger(__name__) -class EsbuildExecutionError(Exception): +class EsbuildExecutionError(LambdaBuilderError): """ - Exception raised in case NPM execution fails. - It will pass on the standard error output from the NPM console. + Exception raised in case esbuild execution fails. + It will pass on the standard error output from the esbuild console. """ MESSAGE = "Esbuild Failed: {message}" - def __init__(self, **kwargs): - Exception.__init__(self, self.MESSAGE.format(**kwargs)) - class SubprocessEsbuild(object): diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py index 729d1a2a7..280b8c14e 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py @@ -10,7 +10,6 @@ CopySourceAction, ) from aws_lambda_builders.utils import which -from aws_lambda_builders.exceptions import WorkflowFailedError from .actions import ( EsbuildBundleAction, ) @@ -27,8 +26,8 @@ class NodejsNpmEsbuildWorkflow(BaseWorkflow): """ - A Lambda builder workflow that knows how to pack - NodeJS projects using NPM. + A Lambda builder workflow that uses esbuild to bundle Node.js and transpile TS + NodeJS projects using NPM with esbuild. """ NAME = "NodejsNpmEsbuildBuilder" @@ -48,27 +47,35 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim 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) + subprocess_esbuild = self._get_esbuild_subprocess(subprocess_npm, scratch_dir, osutils) - manifest_config = self.get_manifest_config(osutils, manifest_path) + bundler_config = self.get_build_properties() + + if not osutils.file_exists(manifest_path): + LOG.warning("package.json file not found. Bundling source without dependencies.") + self.actions = [EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild)] + return if not is_experimental_esbuild_scope(self.experimental_flags): raise EsbuildExecutionError(message="Feature flag must be enabled to use this workflow") - self.actions = self.actions_with_bundler(source_dir, artifacts_dir, manifest_config, osutils, subprocess_npm) + self.actions = self.actions_with_bundler( + source_dir, scratch_dir, artifacts_dir, bundler_config, osutils, subprocess_npm, subprocess_esbuild + ) - def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutils, subprocess_npm): + def actions_with_bundler( + self, source_dir, scratch_dir, artifacts_dir, bundler_config, osutils, subprocess_npm, subprocess_esbuild + ): """ 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 scratch_dir: str + :param scratch_dir: an existing (writable) directory for temporary files + :type artifacts_dir: str :param artifacts_dir: an existing (writable) directory where to store the output. @@ -81,50 +88,46 @@ def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutil :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm :param subprocess_npm: An instance of the NPM process wrapper + :type subprocess_esbuild: aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild.SubprocessEsbuild + :param subprocess_esbuild: An instance of the esbuild 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) - 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) + + copy_action = CopySourceAction(source_dir, scratch_dir, excludes=self.EXCLUDED_FILES) if osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path): - install_action = NodejsNpmCIAction(source_dir, subprocess_npm=subprocess_npm) + install_action = NodejsNpmCIAction(scratch_dir, subprocess_npm=subprocess_npm) else: - install_action = NodejsNpmInstallAction(source_dir, subprocess_npm=subprocess_npm, is_production=False) + install_action = NodejsNpmInstallAction(scratch_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] + esbuild_action = EsbuildBundleAction(scratch_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) + return [copy_action, install_action, esbuild_action] - def get_manifest_config(self, osutils, manifest_path): + def get_build_properties(self): """ 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) - 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: - raise WorkflowFailedError(workflow_name=self.NAME, action_name="ParseManifest", reason=str(ex)) + if self.options and isinstance(self.options, dict): + LOG.debug("Lambda Builders found the following esbuild properties:\n%s", json.dumps(self.options)) + return self.options + return {} def get_resolvers(self): """ specialized path resolver that just returns the list of executable for the runtime on the path. """ return [PathResolver(runtime=self.runtime, binary="npm")] + + def _get_esbuild_subprocess(self, subprocess_npm, scratch_dir, osutils) -> SubprocessEsbuild: + npm_bin_path = subprocess_npm.run(["bin"], cwd=scratch_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 + return SubprocessEsbuild(osutils, executable_search_paths, which=which) diff --git a/tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py index 927b9ea2e..f81f742a6 100644 --- a/tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py +++ b/tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py @@ -46,12 +46,15 @@ def test_doesnt_build_without_feature_flag(self): def test_builds_javascript_project_with_dependencies(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild") + options = {"entry_points": ["included.js"]} + self.builder.build( source_dir, self.artifacts_dir, self.scratch_dir, os.path.join(source_dir, "package.json"), runtime=self.runtime, + options=options, experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) @@ -62,12 +65,15 @@ def test_builds_javascript_project_with_dependencies(self): def test_builds_javascript_project_with_multiple_entrypoints(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild-multiple-entrypoints") + options = {"entry_points": ["included.js", "included2.js"]} + self.builder.build( source_dir, self.artifacts_dir, self.scratch_dir, os.path.join(source_dir, "package.json"), runtime=self.runtime, + options=options, experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) @@ -78,12 +84,15 @@ def test_builds_javascript_project_with_multiple_entrypoints(self): def test_builds_typescript_projects(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild-typescript") + options = {"entry_points": ["included.ts"]} + self.builder.build( source_dir, self.artifacts_dir, self.scratch_dir, os.path.join(source_dir, "package.json"), runtime=self.runtime, + options=options, experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) @@ -101,12 +110,15 @@ def test_builds_with_external_esbuild(self): binpath = npm.run(["bin"], cwd=esbuild_dir) + options = {"entry_points": ["included.js"]} + self.builder.build( source_dir, self.artifacts_dir, self.scratch_dir, os.path.join(source_dir, "package.json"), runtime=self.runtime, + options=options, executable_search_paths=[binpath], experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) @@ -115,11 +127,10 @@ def test_builds_with_external_esbuild(self): output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) - def test_fails_if_package_json_is_broken(self): - - source_dir = os.path.join(self.TEST_DATA_FOLDER, "broken-package") + def test_no_options_passed_to_esbuild(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild") - with self.assertRaises(WorkflowFailedError) as ctx: + with self.assertRaises(WorkflowFailedError) as context: self.builder.build( source_dir, self.artifacts_dir, @@ -129,4 +140,48 @@ def test_fails_if_package_json_is_broken(self): experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) - self.assertIn("NodejsNpmEsbuildBuilder:ParseManifest", str(ctx.exception)) + self.assertEqual(str(context.exception), "NodejsNpmEsbuildBuilder:EsbuildBundle - entry_points not set ({})") + + def test_bundle_with_implicit_file_types(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "implicit-file-types") + + options = {"entry_points": ["included", "implicit"]} + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + options=options, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + ) + + expected_files = {"included.js.map", "implicit.js.map", "implicit.js", "included.js"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + def test_bundles_project_without_dependencies(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "no-package-esbuild") + options = {"entry_points": ["included"]} + + osutils = OSUtils() + npm = SubprocessNpm(osutils) + 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, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + options=options, + experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + executable_search_paths=[binpath], + ) + + expected_files = {"included.js.map", "included.js"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/excluded.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/excluded.js deleted file mode 100644 index 8bf8be437..000000000 --- a/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/excluded.js +++ /dev/null @@ -1,2 +0,0 @@ -//excluded -const x = 1; diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/included.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/included.js deleted file mode 100644 index e8f963aee..000000000 --- a/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/included.js +++ /dev/null @@ -1,2 +0,0 @@ -//included -const x = 1; diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/package.json deleted file mode 100644 index 14157d2dd..000000000 --- a/tests/integration/workflows/nodejs_npm_esbuild/testdata/broken-package/package.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "name": "broken-package-json", - "version": "1.0.0", - "description": "", - "author": "", - "license": "APACHE2.0", - "dependencies": { - -} diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/implicit.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/implicit.js new file mode 100644 index 000000000..13e9a6662 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/implicit.js @@ -0,0 +1 @@ +const x = 10 \ No newline at end of file diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/included.ts b/tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/included.ts new file mode 100644 index 000000000..82397888a --- /dev/null +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/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_esbuild/testdata/implicit-file-types/package-lock.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/package-lock.json new file mode 100644 index 000000000..7230f5534 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/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_esbuild/testdata/implicit-file-types/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/package.json new file mode 100644 index 000000000..0bf39aef2 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/implicit-file-types/package.json @@ -0,0 +1,14 @@ +{ + "name": "with-deps-esbuild-typescript", + "version": "1.0.0", + "description": "", + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "@types/aws-lambda": "^8.10.76" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } +} diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/package.json index 997d05815..9e6e5f4c9 100644 --- a/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/package.json +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-deps-esbuild/package.json @@ -4,9 +4,5 @@ "description": "", "keywords": [], "author": "", - "license": "APACHE2.0", - "aws_sam": { - "bundler": "esbuild", - "entry_points": ["included.js"] - } + "license": "APACHE2.0" } diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-package-esbuild/included.js b/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-package-esbuild/included.js new file mode 100644 index 000000000..3fc841985 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/no-package-esbuild/included.js @@ -0,0 +1 @@ +const number = 100 \ No newline at end of file diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/package.json index e85d55506..e74abd5da 100644 --- a/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/package.json +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-multiple-entrypoints/package.json @@ -5,10 +5,6 @@ "keywords": [], "author": "", "license": "APACHE2.0", - "aws_sam": { - "bundler": "esbuild", - "entry_points": ["included.js", "included2.js"] - }, "dependencies": { "minimal-request-promise": "^1.5.0" }, diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-typescript/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-typescript/package.json index 43827dd4c..0bf39aef2 100644 --- a/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-typescript/package.json +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild-typescript/package.json @@ -5,10 +5,6 @@ "keywords": [], "author": "", "license": "APACHE2.0", - "aws_sam": { - "bundler": "esbuild", - "entry_points": ["included.ts"] - }, "dependencies": { "@types/aws-lambda": "^8.10.76" }, diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/package.json index c4f10260c..c42ce7ea4 100644 --- a/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/package.json +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/with-deps-esbuild/package.json @@ -5,10 +5,6 @@ "keywords": [], "author": "", "license": "APACHE2.0", - "aws_sam": { - "bundler": "esbuild", - "entry_points": ["included.js"] - }, "dependencies": { "minimal-request-promise": "^1.5.0" }, diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py b/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py index 3db988190..a67edaf95 100644 --- a/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_actions.py @@ -1,5 +1,6 @@ from unittest import TestCase from mock import patch +from parameterized import parameterized from aws_lambda_builders.actions import ActionFailedError from aws_lambda_builders.workflows.nodejs_npm_esbuild.actions import EsbuildBundleAction @@ -171,3 +172,42 @@ def test_includes_multiple_entry_points_if_requested(self): ], cwd="source", ) + + +class TestImplicitFileTypeResolution(TestCase): + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + @patch("aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild.SubprocessEsbuild") + def setUp(self, OSUtilMock, SubprocessEsbuildMock): + self.osutils = OSUtilMock.return_value + self.subprocess_esbuild = SubprocessEsbuildMock.return_value + self.action = EsbuildBundleAction( + "source", + "artifacts", + {}, + self.osutils, + self.subprocess_esbuild, + ) + + @parameterized.expand( + [ + ([True], "file.ts", "file.ts"), + ([False, True], "file", "file.js"), + ([True], "file", "file.ts"), + ] + ) + def test_implicit_and_explicit_file_types(self, file_exists, entry_point, expected): + self.osutils.file_exists.side_effect = file_exists + explicit_entry_point = self.action._get_explicit_file_type(entry_point, "") + self.assertEqual(expected, explicit_entry_point) + + @parameterized.expand( + [ + ([False], "file.ts"), + ([False, False], "file"), + ] + ) + def test_throws_exception_entry_point_not_found(self, file_exists, entry_point): + self.osutils.file_exists.side_effect = file_exists + with self.assertRaises(ActionFailedError) as context: + self.action._get_explicit_file_type(entry_point, "invalid") + self.assertEqual(str(context.exception), "entry point invalid does not exist") diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py index e76dde681..11bb8afab 100644 --- a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py @@ -1,6 +1,7 @@ from unittest import TestCase from mock import patch, call +from aws_lambda_builders.actions import CopySourceAction from aws_lambda_builders.exceptions import WorkflowFailedError from aws_lambda_builders.architecture import ARM64 from aws_lambda_builders.workflows.nodejs_npm.actions import NodejsNpmInstallAction, NodejsNpmCIAction @@ -50,24 +51,14 @@ def test_workflow_sets_up_npm_actions_with_bundler_if_manifest_requests_it(self) experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) - 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.assertEqual(len(workflow.actions), 3) + self.assertIsInstance(workflow.actions[0], CopySourceAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmInstallAction) + self.assertIsInstance(workflow.actions[2], EsbuildBundleAction) self.osutils.file_exists.assert_has_calls( [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: - NodejsNpmEsbuildWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) - - self.assertEqual(raised.exception.args[0], "NodejsNpmEsbuildBuilder: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" @@ -82,8 +73,8 @@ def test_sets_up_esbuild_search_path_from_npm_bin(self): experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) - self.osutils.popen.assert_called_with(["npm", "bin"], stdout="PIPE", stderr="PIPE", cwd="source") - esbuild = workflow.actions[1].subprocess_esbuild + self.osutils.popen.assert_called_with(["npm", "bin"], stdout="PIPE", stderr="PIPE", cwd="scratch_dir") + esbuild = workflow.actions[2].subprocess_esbuild self.assertIsInstance(esbuild, SubprocessEsbuild) self.assertEqual(esbuild.executable_search_paths, ["project/bin"]) @@ -103,8 +94,8 @@ def test_sets_up_esbuild_search_path_with_workflow_executable_search_paths_after experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) - self.osutils.popen.assert_called_with(["npm", "bin"], stdout="PIPE", stderr="PIPE", cwd="source") - esbuild = workflow.actions[1].subprocess_esbuild + self.osutils.popen.assert_called_with(["npm", "bin"], stdout="PIPE", stderr="PIPE", cwd="scratch_dir") + esbuild = workflow.actions[2].subprocess_esbuild self.assertIsInstance(esbuild, SubprocessEsbuild) self.assertEqual(esbuild.executable_search_paths, ["project/bin", "other/bin"]) @@ -122,9 +113,10 @@ def test_workflow_uses_npm_ci_if_lockfile_exists(self): experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) - self.assertEqual(len(workflow.actions), 2) - self.assertIsInstance(workflow.actions[0], NodejsNpmCIAction) - self.assertIsInstance(workflow.actions[1], EsbuildBundleAction) + self.assertEqual(len(workflow.actions), 3) + self.assertIsInstance(workflow.actions[0], CopySourceAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmCIAction) + self.assertIsInstance(workflow.actions[2], EsbuildBundleAction) self.osutils.file_exists.assert_has_calls([call("source/package-lock.json")]) def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): @@ -141,9 +133,10 @@ def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], ) - self.assertEqual(len(workflow.actions), 2) - self.assertIsInstance(workflow.actions[0], NodejsNpmCIAction) - self.assertIsInstance(workflow.actions[1], EsbuildBundleAction) + self.assertEqual(len(workflow.actions), 3) + self.assertIsInstance(workflow.actions[0], CopySourceAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmCIAction) + self.assertIsInstance(workflow.actions[2], EsbuildBundleAction) self.osutils.file_exists.assert_has_calls( [call("source/package-lock.json"), call("source/npm-shrinkwrap.json")] ) From fe72f731e37af40608b826d2ed19236a9961e577 Mon Sep 17 00:00:00 2001 From: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Date: Tue, 8 Feb 2022 15:10:41 -0800 Subject: [PATCH 6/6] chore: bump version to 1.11.0 (#325) --- aws_lambda_builders/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws_lambda_builders/__init__.py b/aws_lambda_builders/__init__.py index 69aeed5f8..bb83800ad 100644 --- a/aws_lambda_builders/__init__.py +++ b/aws_lambda_builders/__init__.py @@ -1,5 +1,5 @@ """ AWS Lambda Builder Library """ -__version__ = "1.10.0" +__version__ = "1.11.0" RPC_PROTOCOL_VERSION = "0.3"