diff --git a/.appveyor.yml b/.appveyor.yml index 1e458eb55..a6954b9e5 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -13,6 +13,9 @@ environment: build: off install: +# To run Nodejs workflow integ tests +- ps: Install-Product node 8.10 + - "set PATH=%PYTHON%\\Scripts;%PYTHON%\\bin;%PATH%" - "%PYTHON%\\python.exe -m pip install -r requirements/dev.txt" - "%PYTHON%\\python.exe -m pip install -e ." diff --git a/.travis.yml b/.travis.yml index 62494ef7e..c22f0aebc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,11 @@ matrix: dist: xenial sudo: true install: + + # To run Nodejs workflow integ tests + - nvm install 8.10.0 + - nvm use 8.10.0 + # Install the code requirements - make init script: diff --git a/aws_lambda_builders/__init__.py b/aws_lambda_builders/__init__.py index 5f38ffaaa..4f3d72955 100644 --- a/aws_lambda_builders/__init__.py +++ b/aws_lambda_builders/__init__.py @@ -1,5 +1,5 @@ """ AWS Lambda Builder Library """ -__version__ = '0.0.3' +__version__ = '0.0.4' RPC_PROTOCOL_VERSION = "0.1" diff --git a/aws_lambda_builders/builder.py b/aws_lambda_builders/builder.py index 57eda2f9a..1872bfe29 100644 --- a/aws_lambda_builders/builder.py +++ b/aws_lambda_builders/builder.py @@ -3,6 +3,7 @@ """ import importlib +import os import logging from aws_lambda_builders.registry import get_workflow, DEFAULT_REGISTRY @@ -93,6 +94,8 @@ def build(self, source_dir, artifacts_dir, scratch_dir, manifest_path, if runtime: self._validate_runtime(runtime) + if not os.path.exists(scratch_dir): + os.makedirs(scratch_dir) workflow = self.selected_workflow_cls(source_dir, artifacts_dir, diff --git a/aws_lambda_builders/workflows/__init__.py b/aws_lambda_builders/workflows/__init__.py index 25258ca3e..5df1b9f15 100644 --- a/aws_lambda_builders/workflows/__init__.py +++ b/aws_lambda_builders/workflows/__init__.py @@ -3,3 +3,4 @@ """ import aws_lambda_builders.workflows.python_pip +import aws_lambda_builders.workflows.nodejs_npm diff --git a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md new file mode 100644 index 000000000..1a7746756 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md @@ -0,0 +1,137 @@ +## NodeJS - NPM Lambda Builder + +### 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 +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. + +### Challenges + +NPM normally stores all dependencies in a `node_modules` subdirectory. It +supports several dependency categories, such as development dependencies +(usually third-party build utilities and test resources), optional dependencies +(usually required for local execution but already available on the production +environment, or peer-dependencies for optional third-party packages) and +production dependencies (normally the minimum required for correct execution). +All these dependency types are mixed in the same directory. + +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. + +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. +This makes deployment significantly faster for single-file lambdas, for +example. Although this is not good from a consistency and compatibility +perspective (as the version of the API used in production might be different +from what was used during testing), people do this frequently enough that the +packager should handle it in some way. A common way of marking this with ClaudiaJS +is to include `aws-sdk` as an optional dependency, then deploy without optional +dependencies. + +Other runtimes do not have this flexibility, so instead of adding a specific +parameter to the SAM CLI, the packager should support a flag to include or +exclude optional dependencies through environment variables. + +NPM also provides support for running user-defined scripts as part of the build +process, so this packager needs to support standard NPM script execution. + +NPM, since version 5, uses symbolic links to optimise disk space usage, so +cross-project dependencies will just be linked to elsewhere on the local disk +instead of included in the `node_modules` directory. This means that just copying +the `node_modules` directory (even if symlinks would be resolved to actual paths) +far from optimal to create a stand-alone module. Copying would lead to significantly +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 +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 +(development/optional/production). Although the official tool recommends +including this file in the version control, as a way to pin down dependency +versions, when using on several machines with different project layout it can +lead to uninstallable dependencies. + +NPM dependencies are usually plain javascript libraries, but they may include +native binaries precompiled for a particular platform, or require some system +libraries to be installed. A notable example is `sharp`, a popular image +manipulation library, that uses symbolic links to system libraries. Another +notable example is `puppeteer`, a library to control a headless Chrome browser, +that downloads a Chromium binary for the target platform during installation. + +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 + +The general algorithm for preparing a node package for use on AWS Lambda +is as follows. + +#### Step 1: Prepare a clean copy of the project source files + +Execute `npm pack` to perform project-specific packaging using the supplied +`package.json` manifest, which will automatically exclude temporary files, +test resources and other source files unnecessary for running in a production +environment. + +This will produce a `tar` archive that needs to be unpacked into the artifacts +directory. Note that the archive will actually contain a `package` +subdirectory containing the files, so it's not enough to just directly unpack +files. + +#### Step 2: Rewrite local dependencies + +_(out of scope for the current version)_ + +To optimise disk space and avoid including development dependencies from other +locally linked packages, inspect the `package.json` manifest looking for dependencies +referring to local file paths (can be identified as they start with `.` or `file:`), +then for each dependency recursively execute the packaging process + +Local dependencies may include other local dependencies themselves, this is a very +common way of sharing configuration or development utilities such as linting or testing +tools. This means that for each packaged local dependency this packager needs to +recursively apply the packaging process. It also means that the packager needs to +track local paths and avoid re-packaging directories it already visited. + +NPM produces a `tar` archive while packaging that can be directly included as a +dependency. This will make NPM unpack and install a copy correctly. Once the +packager produces all `tar` archives required by local dependencies, rewrite +the manifest to point to `tar` files instead of the original location. + +If the project contains a package lock file, this will cause NPM to ignore changes +to the package.json manifest. In this case, the packager will need to remove +`package-lock.json` so that dependency rewrites take effect. +_(out of scope for the current version)_ + +#### Step 3: Install dependencies + +The packager should then run `npm install` to download an expand all dependencies to +the local `node_modules` subdirectory. This has to be executed in the directory with +a clean copy of the source files. + +Note that NPM can be configured to use proxies or local company repositories using +a local file, `.npmrc`. The packaging process from step 1 normally excludes this file, so it may +need to be copied additionally before dependency installation, and then removed. +_(out of scope for the current version)_ + +Some users may want to exclude optional dependencies, or even include development dependencies. +To avoid incompatible flags in the `sam` CLI, the packager should allow users to specify +options for the `npm install` command using an environment variable. +_(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)_ + diff --git a/aws_lambda_builders/workflows/nodejs_npm/__init__.py b/aws_lambda_builders/workflows/nodejs_npm/__init__.py new file mode 100644 index 000000000..e62562dc9 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm/__init__.py @@ -0,0 +1,5 @@ +""" +Builds NodeJS Lambda functions using NPM dependency manager +""" + +from .workflow import NodejsNpmWorkflow diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py new file mode 100644 index 000000000..e3e9aad43 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -0,0 +1,113 @@ +""" +Action to resolve NodeJS dependencies using NPM +""" + +import logging + +from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError +from .npm import NpmExecutionError + +LOG = logging.getLogger(__name__) + + +class NodejsNpmPackAction(BaseAction): + + """ + A Lambda Builder Action that packages a Node.js package using NPM to extract the source and remove test resources + """ + + NAME = 'NpmPack' + DESCRIPTION = "Packaging source using NPM" + PURPOSE = Purpose.COPY_SOURCE + + def __init__(self, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm): + """ + :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 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 + """ + super(NodejsNpmPackAction, self).__init__() + self.artifacts_dir = artifacts_dir + self.manifest_path = manifest_path + self.scratch_dir = scratch_dir + self.osutils = osutils + self.subprocess_npm = subprocess_npm + + def execute(self): + """ + Runs the action. + + :raises lambda_builders.actions.ActionFailedError: when NPM packaging fails + """ + try: + package_path = "file:{}".format(self.osutils.abspath(self.osutils.dirname(self.manifest_path))) + + LOG.debug("NODEJS packaging %s to %s", package_path, self.scratch_dir) + + tarfile_name = self.subprocess_npm.run(['pack', '-q', package_path], cwd=self.scratch_dir) + + LOG.debug("NODEJS packed to %s", tarfile_name) + + tarfile_path = self.osutils.joinpath(self.scratch_dir, tarfile_name) + + LOG.debug("NODEJS extracting to %s", self.artifacts_dir) + + self.osutils.extract_tarfile(tarfile_path, self.artifacts_dir) + + except NpmExecutionError as ex: + raise ActionFailedError(str(ex)) + + +class NodejsNpmInstallAction(BaseAction): + + """ + A Lambda Builder Action that installs NPM project dependencies + """ + + NAME = 'NpmInstall' + DESCRIPTION = "Installing dependencies from NPM" + 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(NodejsNpmInstallAction, 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 in: %s", self.artifacts_dir) + + self.subprocess_npm.run( + ['install', '-q', '--no-audit', '--no-save', '--production'], + cwd=self.artifacts_dir + ) + + except NpmExecutionError as ex: + raise ActionFailedError(str(ex)) diff --git a/aws_lambda_builders/workflows/nodejs_npm/npm.py b/aws_lambda_builders/workflows/nodejs_npm/npm.py new file mode 100644 index 000000000..b737ff4d1 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm/npm.py @@ -0,0 +1,90 @@ +""" +Wrapper around calling npm through a subprocess. +""" + +import logging + +LOG = logging.getLogger(__name__) + + +class NpmExecutionError(Exception): + + """ + Exception raised in case NPM execution fails. + It will pass on the standard error output from the NPM console. + """ + + MESSAGE = "NPM Failed: {message}" + + def __init__(self, **kwargs): + Exception.__init__(self, self.MESSAGE.format(**kwargs)) + + +class SubprocessNpm(object): + + """ + Wrapper around the NPM command line utility, making it + easy to consume execution results. + """ + + def __init__(self, osutils, npm_exe=None): + """ + :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils + :param osutils: An instance of OS Utilities for file manipulation + + :type npm_exe: str + :param npm_exe: Path to the NPM binary. If not set, + the default executable path npm will be used + """ + self.osutils = osutils + + if npm_exe is None: + if osutils.is_windows(): + npm_exe = 'npm.cmd' + else: + npm_exe = 'npm' + + self.npm_exe = npm_exe + + def run(self, args, cwd=None): + + """ + Runs the action. + + :type args: list + :param args: Command line arguments to pass to NPM + + :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.NpmExecutionError: + 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_npm = [self.npm_exe] + args + + LOG.debug("executing NPM: %s", invoke_npm) + + p = self.osutils.popen(invoke_npm, + stdout=self.osutils.pipe, + stderr=self.osutils.pipe, + cwd=cwd) + + out, err = p.communicate() + + if p.returncode != 0: + raise NpmExecutionError(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 new file mode 100644 index 000000000..dcff97272 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py @@ -0,0 +1,40 @@ +""" +Commonly used utilities +""" + +import os +import platform +import tarfile +import subprocess + + +class OSUtils(object): + + """ + Wrapper around file system functions, to make it easy to + unit test actions in memory + """ + + def extract_tarfile(self, tarfile_path, unpack_dir): + with tarfile.open(tarfile_path, 'r:*') as tar: + tar.extractall(unpack_dir) + + def joinpath(self, *args): + return os.path.join(*args) + + def popen(self, command, stdout=None, stderr=None, env=None, cwd=None): + p = subprocess.Popen(command, stdout=stdout, stderr=stderr, env=env, cwd=cwd) + return p + + @property + def pipe(self): + return subprocess.PIPE + + def dirname(self, path): + return os.path.dirname(path) + + def abspath(self, path): + return os.path.abspath(path) + + def is_windows(self): + return platform.system().lower() == 'windows' diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py new file mode 100644 index 000000000..b55bc9cc1 --- /dev/null +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -0,0 +1,62 @@ +""" +NodeJS NPM Workflow +""" + +from aws_lambda_builders.workflow import BaseWorkflow, Capability +from aws_lambda_builders.actions import CopySourceAction +from .actions import NodejsNpmPackAction, NodejsNpmInstallAction +from .utils import OSUtils +from .npm import SubprocessNpm + + +class NodejsNpmWorkflow(BaseWorkflow): + + """ + A Lambda builder workflow that knows how to pack + NodeJS projects using NPM. + """ + NAME = "NodejsNpmBuilder" + + CAPABILITY = Capability(language="nodejs", + dependency_manager="npm", + application_framework=None) + + EXCLUDED_FILES = (".aws-sam") + + def __init__(self, + source_dir, + artifacts_dir, + scratch_dir, + manifest_path, + runtime=None, + osutils=None, + **kwargs): + + super(NodejsNpmWorkflow, self).__init__(source_dir, + artifacts_dir, + scratch_dir, + manifest_path, + runtime=runtime, + **kwargs) + + if osutils is None: + osutils = OSUtils() + + subprocess_npm = SubprocessNpm(osutils) + + tar_dest_dir = osutils.joinpath(scratch_dir, 'unpacked') + tar_package_dir = osutils.joinpath(tar_dest_dir, 'package') + + npm_pack = NodejsNpmPackAction(tar_dest_dir, + scratch_dir, + manifest_path, + osutils=osutils, + subprocess_npm=subprocess_npm) + + npm_install = NodejsNpmInstallAction(artifacts_dir, + subprocess_npm=subprocess_npm) + self.actions = [ + npm_pack, + CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES), + npm_install, + ] diff --git a/requirements/base.txt b/requirements/base.txt index 09a93af1b..0150babf3 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -1 +1 @@ -six~=1.11.0 +six~=1.11 diff --git a/tests/functional/test_builder.py b/tests/functional/test_builder.py index 39e79ad01..553887c0e 100644 --- a/tests/functional/test_builder.py +++ b/tests/functional/test_builder.py @@ -19,6 +19,7 @@ def setUp(self): self.source_dir = tempfile.mkdtemp() self.artifacts_dir = tempfile.mkdtemp() + self.scratch_dir = os.path.join(tempfile.mkdtemp(), "scratch") self.hello_builder = LambdaBuilder(language="test", dependency_manager="test", application_framework="test", @@ -34,6 +35,7 @@ def tearDown(self): self.hello_builder._clear_workflows() shutil.rmtree(self.source_dir) shutil.rmtree(self.artifacts_dir) + shutil.rmtree(self.scratch_dir) # Remove the workflows folder from PYTHONPATH sys.path.remove(self.TEST_WORKFLOWS_FOLDER) @@ -42,7 +44,7 @@ def test_run_hello_workflow(self): self.hello_builder.build(self.source_dir, self.artifacts_dir, - "/ignored", + self.scratch_dir, "/ignored") self.assertTrue(os.path.exists(self.expected_filename)) diff --git a/tests/functional/test_cli.py b/tests/functional/test_cli.py index f72180e78..275ccd411 100644 --- a/tests/functional/test_cli.py +++ b/tests/functional/test_cli.py @@ -19,6 +19,7 @@ def setUp(self): self.source_dir = tempfile.mkdtemp() self.artifacts_dir = tempfile.mkdtemp() + self.scratch_dir = os.path.join(tempfile.mkdtemp(), "scratch") # Capabilities supported by the Hello workflow self.language = "test" @@ -38,6 +39,7 @@ def setUp(self): def tearDown(self): shutil.rmtree(self.source_dir) shutil.rmtree(self.artifacts_dir) + shutil.rmtree(self.scratch_dir) @parameterized.expand([ ("request_through_stdin"), @@ -58,7 +60,7 @@ def test_run_hello_workflow(self, flavor): "supported_workflows": [self.HELLO_WORKFLOW_MODULE], "source_dir": self.source_dir, "artifacts_dir": self.artifacts_dir, - "scratch_dir": "/ignored", + "scratch_dir": self.scratch_dir, "manifest_path": "/ignored", "runtime": "ignored", "optimizations": {}, diff --git a/tests/functional/testdata/cwd.py b/tests/functional/testdata/cwd.py new file mode 100644 index 000000000..ea065561f --- /dev/null +++ b/tests/functional/testdata/cwd.py @@ -0,0 +1,3 @@ +import os + +print(os.getcwd()) diff --git a/tests/functional/workflows/nodejs_npm/test_data/test.tgz b/tests/functional/workflows/nodejs_npm/test_data/test.tgz new file mode 100644 index 000000000..8c2c2fc72 Binary files /dev/null and b/tests/functional/workflows/nodejs_npm/test_data/test.tgz differ diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py new file mode 100644 index 000000000..b84bc1e8f --- /dev/null +++ b/tests/functional/workflows/nodejs_npm/test_utils.py @@ -0,0 +1,77 @@ +import os +import shutil +import sys +import tempfile + +from unittest import TestCase + +from aws_lambda_builders.workflows.nodejs_npm import utils + + +class TestOSUtils(TestCase): + + def setUp(self): + + self.osutils = utils.OSUtils() + + def test_extract_tarfile_unpacks_a_tar(self): + + test_tar = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz") + + test_dir = tempfile.mkdtemp() + + self.osutils.extract_tarfile(test_tar, test_dir) + + output_files = set(os.listdir(test_dir)) + + shutil.rmtree(test_dir) + + self.assertEqual({"test_utils.py"}, output_files) + + def test_dirname_returns_directory_for_path(self): + dirname = self.osutils.dirname(sys.executable) + + self.assertEqual(dirname, os.path.dirname(sys.executable)) + + def test_abspath_returns_absolute_path(self): + + result = self.osutils.abspath('.') + + self.assertTrue(os.path.isabs(result)) + + self.assertEqual(result, os.path.abspath('.')) + + def test_joinpath_joins_path_components(self): + + result = self.osutils.joinpath('a', 'b', 'c') + + self.assertEqual(result, os.path.join('a', 'b', 'c')) + + def test_popen_runs_a_process_and_returns_outcome(self): + + cwd_py = os.path.join(os.path.dirname(__file__), '..', '..', 'testdata', 'cwd.py') + + p = self.osutils.popen([sys.executable, cwd_py], + stdout=self.osutils.pipe, + stderr=self.osutils.pipe) + + out, err = p.communicate() + + self.assertEqual(p.returncode, 0) + + self.assertEqual(out.decode('utf8').strip(), os.getcwd()) + + def test_popen_can_accept_cwd(self): + + testdata_dir = os.path.join(os.path.dirname(__file__), '..', '..', 'testdata') + + p = self.osutils.popen([sys.executable, 'cwd.py'], + stdout=self.osutils.pipe, + stderr=self.osutils.pipe, + cwd=testdata_dir) + + out, err = p.communicate() + + self.assertEqual(p.returncode, 0) + + self.assertEqual(out.decode('utf8').strip(), os.path.abspath(testdata_dir)) diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py new file mode 100644 index 000000000..980281e16 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -0,0 +1,79 @@ +import os +import shutil +import tempfile + +from unittest import TestCase + +from aws_lambda_builders.builder import LambdaBuilder +from aws_lambda_builders.exceptions import WorkflowFailedError + + +class TestNodejsNpmWorkflow(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") + + self.builder = LambdaBuilder(language="nodejs", + dependency_manager="npm", + application_framework=None) + self.runtime = "nodejs8.10" + + def tearDown(self): + shutil.rmtree(self.artifacts_dir) + shutil.rmtree(self.scratch_dir) + + def test_builds_project_without_dependencies(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "no-deps") + + self.builder.build(source_dir, self.artifacts_dir, self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime) + + expected_files = {"package.json", "included.js"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEquals(expected_files, output_files) + + def test_builds_project_with_remote_dependencies(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "npm-deps") + + self.builder.build(source_dir, self.artifacts_dir, self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime) + + expected_files = {"package.json", "included.js", "node_modules"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEquals(expected_files, output_files) + + expected_modules = {"minimal-request-promise"} + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) + self.assertEquals(expected_modules, output_modules) + + def test_fails_if_npm_cannot_resolve_dependencies(self): + + source_dir = os.path.join(self.TEST_DATA_FOLDER, "broken-deps") + + 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("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("Unexpected end of JSON input", str(ctx.exception)) diff --git a/tests/integration/workflows/nodejs_npm/testdata/broken-deps/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/broken-deps/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/broken-deps/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/broken-deps/included.js b/tests/integration/workflows/nodejs_npm/testdata/broken-deps/included.js new file mode 100644 index 000000000..e8f963aee --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/broken-deps/included.js @@ -0,0 +1,2 @@ +//included +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/broken-deps/package.json b/tests/integration/workflows/nodejs_npm/testdata/broken-deps/package.json new file mode 100644 index 000000000..d7c526360 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/broken-deps/package.json @@ -0,0 +1,12 @@ +{ + "name": "brokendeps", + "version": "1.0.0", + "description": "", + "files": ["included.js"], + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "0.0.0-NON_EXISTENT" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/broken-package/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/broken-package/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/broken-package/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/broken-package/included.js b/tests/integration/workflows/nodejs_npm/testdata/broken-package/included.js new file mode 100644 index 000000000..e8f963aee --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/broken-package/included.js @@ -0,0 +1,2 @@ +//included +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/broken-package/package.json b/tests/integration/workflows/nodejs_npm/testdata/broken-package/package.json new file mode 100644 index 000000000..14157d2dd --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/broken-package/package.json @@ -0,0 +1,9 @@ +{ + "name": "broken-package-json", + "version": "1.0.0", + "description": "", + "author": "", + "license": "APACHE2.0", + "dependencies": { + +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/no-deps/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/no-deps/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps/included.js b/tests/integration/workflows/nodejs_npm/testdata/no-deps/included.js new file mode 100644 index 000000000..e8f963aee --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/no-deps/included.js @@ -0,0 +1,2 @@ +//included +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/no-deps/package.json b/tests/integration/workflows/nodejs_npm/testdata/no-deps/package.json new file mode 100644 index 000000000..b7874b36e --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/no-deps/package.json @@ -0,0 +1,9 @@ +{ + "name": "nodeps", + "version": "1.0.0", + "description": "", + "files": ["included.js"], + "keywords": [], + "author": "", + "license": "APACHE2.0" +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/npm-deps/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/npm-deps/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/npm-deps/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/npm-deps/included.js b/tests/integration/workflows/nodejs_npm/testdata/npm-deps/included.js new file mode 100644 index 000000000..e8f963aee --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/npm-deps/included.js @@ -0,0 +1,2 @@ +//included +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/npm-deps/package.json b/tests/integration/workflows/nodejs_npm/testdata/npm-deps/package.json new file mode 100644 index 000000000..05e779165 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/npm-deps/package.json @@ -0,0 +1,12 @@ +{ + "name": "npmdeps", + "version": "1.0.0", + "description": "", + "files": ["included.js"], + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "*" + } +} diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 965f7bcce..f611fe1d4 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -1,6 +1,7 @@ from unittest import TestCase from mock import patch, call, Mock +from parameterized import parameterized, param from aws_lambda_builders.builder import LambdaBuilder from aws_lambda_builders.workflow import Capability, BaseWorkflow @@ -99,12 +100,19 @@ def setUp(self): self.lang_framework = "pip" self.app_framework = "chalice" + @parameterized.expand([ + param(True), + param(False) + ]) + @patch('aws_lambda_builders.builder.os') @patch('aws_lambda_builders.builder.importlib') @patch('aws_lambda_builders.builder.get_workflow') - def test_with_mocks(self, get_workflow_mock, importlib_mock): + def test_with_mocks(self, scratch_dir_exists, get_workflow_mock, importlib_mock, os_mock): workflow_cls = Mock() workflow_instance = workflow_cls.return_value = Mock() + os_mock.path.exists.return_value = scratch_dir_exists + get_workflow_mock.return_value = workflow_cls with patch.object(LambdaBuilder, "_validate_runtime"): @@ -116,3 +124,8 @@ def test_with_mocks(self, get_workflow_mock, importlib_mock): workflow_cls.assert_called_with("source_dir", "artifacts_dir", "scratch_dir", "manifest_path", runtime="runtime", optimizations="optimizations", options="options") workflow_instance.run.assert_called_once() + os_mock.path.exists.assert_called_once_with("scratch_dir") + if scratch_dir_exists: + os_mock.makedirs.not_called() + else: + os_mock.makedirs.assert_called_once_with("scratch_dir") diff --git a/tests/unit/workflows/nodejs_npm/__init__.py b/tests/unit/workflows/nodejs_npm/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py new file mode 100644 index 000000000..f6d0a46fb --- /dev/null +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -0,0 +1,80 @@ +from unittest import TestCase +from mock import patch + +from aws_lambda_builders.actions import ActionFailedError +from aws_lambda_builders.workflows.nodejs_npm.actions import NodejsNpmPackAction, NodejsNpmInstallAction +from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError + + +class TestNodejsNpmPackAction(TestCase): + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm") + def test_tars_and_unpacks_npm_project(self, OSUtilMock, SubprocessNpmMock): + osutils = OSUtilMock.return_value + subprocess_npm = SubprocessNpmMock.return_value + + action = NodejsNpmPackAction("artifacts", "scratch_dir", + "manifest", + osutils=osutils, + subprocess_npm=subprocess_npm) + + osutils.dirname.side_effect = lambda value: "/dir:{}".format(value) + osutils.abspath.side_effect = lambda value: "/abs:{}".format(value) + osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) + + subprocess_npm.run.return_value = 'package.tar' + + action.execute() + + subprocess_npm.run.assert_called_with(['pack', '-q', 'file:/abs:/dir:manifest'], cwd='scratch_dir') + osutils.extract_tarfile.assert_called_with('scratch_dir/package.tar', 'artifacts') + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm") + def test_raises_action_failed_when_npm_fails(self, OSUtilMock, SubprocessNpmMock): + osutils = OSUtilMock.return_value + subprocess_npm = SubprocessNpmMock.return_value + + builder_instance = SubprocessNpmMock.return_value + builder_instance.run.side_effect = NpmExecutionError(message="boom!") + + action = NodejsNpmPackAction("artifacts", "scratch_dir", + "manifest", + osutils=osutils, subprocess_npm=subprocess_npm) + + with self.assertRaises(ActionFailedError) as raised: + action.execute() + + self.assertEqual(raised.exception.args[0], "NPM Failed: boom!") + + +class TestNodejsNpmInstallAction(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 = NodejsNpmInstallAction("artifacts", + subprocess_npm=subprocess_npm) + + action.execute() + + expected_args = ['install', '-q', '--no-audit', '--no-save', '--production'] + + 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 + + builder_instance = SubprocessNpmMock.return_value + builder_instance.run.side_effect = NpmExecutionError(message="boom!") + + action = NodejsNpmInstallAction("artifacts", + subprocess_npm=subprocess_npm) + + with self.assertRaises(ActionFailedError) as raised: + action.execute() + + self.assertEqual(raised.exception.args[0], "NPM Failed: boom!") diff --git a/tests/unit/workflows/nodejs_npm/test_npm.py b/tests/unit/workflows/nodejs_npm/test_npm.py new file mode 100644 index 000000000..8eae867da --- /dev/null +++ b/tests/unit/workflows/nodejs_npm/test_npm.py @@ -0,0 +1,91 @@ +from unittest import TestCase +from mock import patch + +from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm, NpmExecutionError + + +class FakePopen: + def __init__(self, out=b'out', err=b'err', retcode=0): + self.out = out + self.err = err + self.returncode = retcode + + def communicate(self): + return self.out, self.err + + +class TestSubprocessNpm(TestCase): + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + def setUp(self, OSUtilMock): + + self.osutils = OSUtilMock.return_value + self.osutils.pipe = 'PIPE' + self.popen = FakePopen() + self.osutils.popen.side_effect = [self.popen] + self.under_test = SubprocessNpm(self.osutils, npm_exe="/a/b/c/npm.exe") + + def test_run_executes_npm_on_nixes(self): + + self.osutils.is_windows.side_effect = [False] + + self.under_test = SubprocessNpm(self.osutils) + + self.under_test.run(['pack', '-q']) + + self.osutils.popen.assert_called_with(['npm', 'pack', '-q'], cwd=None, stderr='PIPE', stdout='PIPE') + + def test_run_executes_npm_cmd_on_windows(self): + + self.osutils.is_windows.side_effect = [True] + + self.under_test = SubprocessNpm(self.osutils) + + self.under_test.run(['pack', '-q']) + + self.osutils.popen.assert_called_with(['npm.cmd', 'pack', '-q'], cwd=None, stderr='PIPE', stdout='PIPE') + + def test_uses_custom_npm_path_if_supplied(self): + + self.under_test.run(['pack', '-q']) + + self.osutils.popen.assert_called_with(['/a/b/c/npm.exe', 'pack', '-q'], cwd=None, stderr='PIPE', stdout='PIPE') + + def test_uses_cwd_if_supplied(self): + + self.under_test.run(['pack', '-q'], cwd='/a/cwd') + + self.osutils.popen.assert_called_with(['/a/b/c/npm.exe', 'pack', '-q'], + cwd='/a/cwd', stderr='PIPE', stdout='PIPE') + + def test_returns_popen_out_decoded_if_retcode_is_0(self): + + self.popen.out = b'some encoded text\n\n' + + result = self.under_test.run(['pack']) + + self.assertEqual(result, 'some encoded text') + + def test_raises_NpmExecutionError_with_err_text_if_retcode_is_not_0(self): + + self.popen.returncode = 1 + self.popen.err = b'some error text\n\n' + + with self.assertRaises(NpmExecutionError) as raised: + self.under_test.run(['pack']) + + self.assertEqual(raised.exception.args[0], "NPM Failed: some error text") + + def test_raises_ValueError_if_args_not_a_list(self): + + with self.assertRaises(ValueError) as raised: + self.under_test.run(('pack')) + + self.assertEqual(raised.exception.args[0], "args must be a list") + + def test_raises_ValueError_if_args_empty(self): + + with self.assertRaises(ValueError) as raised: + self.under_test.run([]) + + self.assertEqual(raised.exception.args[0], "requires at least one arg") diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py new file mode 100644 index 000000000..1d8e0d63c --- /dev/null +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -0,0 +1,25 @@ +from unittest import TestCase + +from aws_lambda_builders.actions import CopySourceAction +from aws_lambda_builders.workflows.nodejs_npm.workflow import NodejsNpmWorkflow +from aws_lambda_builders.workflows.nodejs_npm.actions import NodejsNpmPackAction, NodejsNpmInstallAction + + +class TestNodejsNpmWorkflow(TestCase): + + """ + the workflow requires an external utility (npm) to run, so it is extensively tested in integration tests. + this is just a quick wiring test to provide fast feedback if things are badly broken + """ + + def test_workflow_sets_up_npm_actions(self): + + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest") + + self.assertEqual(len(workflow.actions), 3) + + self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) + + self.assertIsInstance(workflow.actions[1], CopySourceAction) + + self.assertIsInstance(workflow.actions[2], NodejsNpmInstallAction)