From 4685d4c96871577b2532541e58c18d22762d7da9 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Thu, 20 Jan 2022 16:23:47 -0800 Subject: [PATCH 01/10] fix test_nodejs_npm integration tests --- tests/integration/workflows/nodejs_npm/test_nodejs_npm.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index dcd7397f8..e5e5a40da 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -97,7 +97,7 @@ def test_builds_project_with_remote_dependencies(self): output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) - expected_modules = {"minimal-request-promise"} + expected_modules = {"minimal-request-promise", ".package-lock.json"} output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) self.assertEqual(expected_modules, output_modules) @@ -117,7 +117,7 @@ def test_builds_project_with_npmrc(self): self.assertEqual(expected_files, output_files) - expected_modules = {"fake-http-request"} + expected_modules = {"fake-http-request", ".package-lock.json"} output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) self.assertEqual(expected_modules, output_modules) @@ -185,11 +185,11 @@ def test_builds_project_with_remote_dependencies_with_download_dependencies_and_ output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) - expected_modules = {"minimal-request-promise"} + expected_modules = {"minimal-request-promise", ".package-lock.json"} output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) self.assertEqual(expected_modules, output_modules) - expected_modules = {"minimal-request-promise"} + expected_modules = {"minimal-request-promise", ".package-lock.json"} output_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules"))) self.assertEqual(expected_modules, output_modules) From fa014d8c150dc7cb3389d6b4eb4cebd7e843c793 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Thu, 20 Jan 2022 16:24:56 -0800 Subject: [PATCH 02/10] respect lockfile --- .../workflows/nodejs_npm/actions.py | 62 ++++++++++++++++--- .../workflows/nodejs_npm/workflow.py | 20 ++++-- .../workflows/nodejs_npm/test_nodejs_npm.py | 27 ++++++++ .../package-lock-and-shrinkwrap/excluded.js | 2 + .../package-lock-and-shrinkwrap/included.js | 2 + .../npm-shrinkwrap.json | 28 +++++++++ .../package-lock.json | 28 +++++++++ .../package-lock-and-shrinkwrap/package.json | 12 ++++ .../nodejs_npm/testdata/package-lock.json | 6 ++ .../testdata/package-lock/excluded.js | 2 + .../testdata/package-lock/included.js | 2 + .../testdata/package-lock/package-lock.json | 28 +++++++++ .../testdata/package-lock/package.json | 12 ++++ .../testdata/shrinkwrap/excluded.js | 2 + .../testdata/shrinkwrap/included.js | 2 + .../testdata/shrinkwrap/npm-shrinkwrap.json | 16 +++++ .../testdata/shrinkwrap/package.json | 12 ++++ .../unit/workflows/nodejs_npm/test_actions.py | 52 +++++++++------- .../workflows/nodejs_npm/test_workflow.py | 36 ++++++++--- 19 files changed, 306 insertions(+), 45 deletions(-) create mode 100644 tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/excluded.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/included.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/npm-shrinkwrap.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/package-lock.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/package.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/package-lock.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/package-lock/excluded.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/package-lock/included.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/package-lock/package-lock.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/package-lock/package.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/excluded.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/included.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/npm-shrinkwrap.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/package.json diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index 43259bd8c..c9aca40c0 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -112,14 +112,55 @@ def execute(self): raise ActionFailedError(str(ex)) -class NodejsNpmrcCopyAction(BaseAction): +class NodejsNpmCIAction(BaseAction): """ - A Lambda Builder Action that copies NPM config file .npmrc + 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 NodejsNpmrcAndLockfileCopyAction(BaseAction): + + """ + A Lambda Builder Action that copies lockfile and NPM config file .npmrc """ - NAME = "CopyNpmrc" - DESCRIPTION = "Copying configuration from .npmrc" + NAME = "CopyNpmrcAndLockfile" + DESCRIPTION = "Copying configuration from .npmrc and dependencies from lockfile" PURPOSE = Purpose.COPY_SOURCE def __init__(self, artifacts_dir, source_dir, osutils): @@ -135,7 +176,7 @@ def __init__(self, artifacts_dir, source_dir, osutils): :param osutils: An instance of OS Utilities for file manipulation """ - super(NodejsNpmrcCopyAction, self).__init__() + super(NodejsNpmrcAndLockfileCopyAction, self).__init__() self.artifacts_dir = artifacts_dir self.source_dir = source_dir self.osutils = osutils @@ -144,14 +185,15 @@ def execute(self): """ Runs the action. - :raises lambda_builders.actions.ActionFailedError: when .npmrc copying fails + :raises lambda_builders.actions.ActionFailedError: when copying fails """ try: - npmrc_path = self.osutils.joinpath(self.source_dir, ".npmrc") - if self.osutils.file_exists(npmrc_path): - LOG.debug(".npmrc copying in: %s", self.artifacts_dir) - self.osutils.copy_file(npmrc_path, self.artifacts_dir) + for filename in [".npmrc", "package-lock.json", "npm-shrinkwrap.json"]: + file_path = self.osutils.joinpath(self.source_dir, filename) + if self.osutils.file_exists(file_path): + LOG.debug("%s copying in: %s", filename, self.artifacts_dir) + self.osutils.copy_file(file_path, self.artifacts_dir) except OSError as ex: raise ActionFailedError(str(ex)) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index b890ddcf1..fb962768f 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -6,7 +6,13 @@ 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 .actions import ( + NodejsNpmPackAction, + NodejsNpmInstallAction, + NodejsNpmrcAndLockfileCopyAction, + NodejsNpmrcCleanUpAction, + NodejsNpmCIAction, +) from .utils import OSUtils from .npm import SubprocessNpm @@ -49,17 +55,23 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim tar_dest_dir, scratch_dir, manifest_path, osutils=osutils, subprocess_npm=subprocess_npm ) - npm_copy_npmrc = NodejsNpmrcCopyAction(tar_package_dir, source_dir, osutils=osutils) + npm_copy = NodejsNpmrcAndLockfileCopyAction(tar_package_dir, source_dir, osutils=osutils) self.actions = [ npm_pack, - npm_copy_npmrc, + npm_copy, CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES), ] if self.download_dependencies: + lockfile_path = osutils.joinpath(source_dir, "package-lock.json") + shrinkwrap_path = osutils.joinpath(source_dir, "npm-shrinkwrap.json") + # installed the dependencies into artifact folder - self.actions.append(NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm)) + if osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path): + self.actions.append(NodejsNpmCIAction(artifacts_dir, subprocess_npm=subprocess_npm)) + else: + self.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 diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index e5e5a40da..f05969c48 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -7,6 +7,8 @@ from unittest import TestCase import mock +from parameterized import parameterized + from aws_lambda_builders.builder import LambdaBuilder from aws_lambda_builders.exceptions import WorkflowFailedError @@ -121,6 +123,31 @@ def test_builds_project_with_npmrc(self): output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) self.assertEqual(expected_modules, output_modules) + @parameterized.expand(["package-lock", "shrinkwrap", "package-lock-and-shrinkwrap"]) + def test_builds_project_with_lockfile(self, dir_name): + expected_files_common = {"package.json", "included.js", "node_modules"} + expected_files_by_dir_name = { + "package-lock": {"package-lock.json"}, + "shrinkwrap": {"npm-shrinkwrap.json"}, + "package-lock-and-shrinkwrap": {"package-lock.json", "npm-shrinkwrap.json"} + } + + source_dir = os.path.join(self.TEST_DATA_FOLDER, dir_name) + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime, + ) + + expected_files = expected_files_common.union(expected_files_by_dir_name[dir_name]) + + output_files = set(os.listdir(self.artifacts_dir)) + + self.assertEqual(expected_files, output_files) + def test_fails_if_npm_cannot_resolve_dependencies(self): source_dir = os.path.join(self.TEST_DATA_FOLDER, "broken-deps") diff --git a/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/included.js b/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/included.js new file mode 100644 index 000000000..e8f963aee --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/included.js @@ -0,0 +1,2 @@ +//included +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/npm-shrinkwrap.json b/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/npm-shrinkwrap.json new file mode 100644 index 000000000..f2030985b --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/npm-shrinkwrap.json @@ -0,0 +1,28 @@ +{ + "name": "package-lock-and-shrinkwrap", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "name": "package-lock-and-shrinkwrap", + "version": "1.0.0", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "*" + } + }, + "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": { + "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/package-lock-and-shrinkwrap/package-lock.json b/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/package-lock.json new file mode 100644 index 000000000..f2030985b --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/package-lock.json @@ -0,0 +1,28 @@ +{ + "name": "package-lock-and-shrinkwrap", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "name": "package-lock-and-shrinkwrap", + "version": "1.0.0", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "*" + } + }, + "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": { + "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/package-lock-and-shrinkwrap/package.json b/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/package.json new file mode 100644 index 000000000..134411558 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/package-lock-and-shrinkwrap/package.json @@ -0,0 +1,12 @@ +{ + "name": "package-lock-and-shrinkwrap", + "version": "1.0.0", + "description": "", + "files": ["included.js"], + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "*" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/package-lock.json b/tests/integration/workflows/nodejs_npm/testdata/package-lock.json new file mode 100644 index 000000000..428d1cf98 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/package-lock.json @@ -0,0 +1,6 @@ +{ + "name": "testdata", + "lockfileVersion": 2, + "requires": true, + "packages": {} +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/package-lock/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/package-lock/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/package-lock/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/package-lock/included.js b/tests/integration/workflows/nodejs_npm/testdata/package-lock/included.js new file mode 100644 index 000000000..e8f963aee --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/package-lock/included.js @@ -0,0 +1,2 @@ +//included +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/package-lock/package-lock.json b/tests/integration/workflows/nodejs_npm/testdata/package-lock/package-lock.json new file mode 100644 index 000000000..dfaca4edc --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/package-lock/package-lock.json @@ -0,0 +1,28 @@ +{ + "name": "package-lock", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "name": "package-lock", + "version": "1.0.0", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "*" + } + }, + "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": { + "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/package-lock/package.json b/tests/integration/workflows/nodejs_npm/testdata/package-lock/package.json new file mode 100644 index 000000000..54eebb556 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/package-lock/package.json @@ -0,0 +1,12 @@ +{ + "name": "package-lock", + "version": "1.0.0", + "description": "", + "files": ["included.js"], + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "*" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/included.js b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/included.js new file mode 100644 index 000000000..e8f963aee --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/included.js @@ -0,0 +1,2 @@ +//included +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/npm-shrinkwrap.json b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/npm-shrinkwrap.json new file mode 100644 index 000000000..beeb191f9 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/npm-shrinkwrap.json @@ -0,0 +1,16 @@ +{ + "name": "shrinkwrap", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "name": "shrinkwrap", + "version": "1.0.0", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "*" + } + } + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/package.json b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/package.json new file mode 100644 index 000000000..f5c0dc00d --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/package.json @@ -0,0 +1,12 @@ +{ + "name": "shrinkwrap", + "version": "1.0.0", + "description": "", + "files": ["included.js"], + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "*" + } +} diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index 39007a59c..7b87bb47a 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -1,11 +1,12 @@ from unittest import TestCase -from mock import patch +from mock import patch, call +from parameterized import parameterized from aws_lambda_builders.actions import ActionFailedError from aws_lambda_builders.workflows.nodejs_npm.actions import ( NodejsNpmPackAction, NodejsNpmInstallAction, - NodejsNpmrcCopyAction, + NodejsNpmrcAndLockfileCopyAction, NodejsNpmrcCleanUpAction, ) from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError @@ -80,30 +81,39 @@ def test_raises_action_failed_when_npm_fails(self, SubprocessNpmMock): self.assertEqual(raised.exception.args[0], "NPM Failed: boom!") -class TestNodejsNpmrcCopyAction(TestCase): +class TestNodejsCopyAction(TestCase): + @parameterized.expand( + [ + [False, False, False], + [True, False, False], + [False, True, False], + [False, False, True], + [True, True, False], + [True, False, True], + [False, True, True], + [True, True, True], + ] + ) @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") - def test_copies_npmrc_into_a_project(self, OSUtilMock): + def test_copies_into_a_project_if_file_exists( + self, npmrc_exists, package_lock_exists, shrinkwrap_exists, OSUtilMock + ): osutils = OSUtilMock.return_value osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) - action = NodejsNpmrcCopyAction("artifacts", "source", osutils=osutils) - osutils.file_exists.side_effect = [True] - action.execute() - - osutils.file_exists.assert_called_with("source/.npmrc") - osutils.copy_file.assert_called_with("source/.npmrc", "artifacts") - - @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") - def test_skips_copying_npmrc_into_a_project_if_npmrc_doesnt_exist(self, OSUtilMock): - osutils = OSUtilMock.return_value - osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) - - action = NodejsNpmrcCopyAction("artifacts", "source", osutils=osutils) - osutils.file_exists.side_effect = [False] + action = NodejsNpmrcAndLockfileCopyAction("artifacts", "source", osutils=osutils) + osutils.file_exists.side_effect = [npmrc_exists, package_lock_exists, shrinkwrap_exists] action.execute() - osutils.file_exists.assert_called_with("source/.npmrc") - osutils.copy_file.assert_not_called() + filename_exists = { + ".npmrc": npmrc_exists, + "package-lock.json": package_lock_exists, + "npm-shrinkwrap.json": shrinkwrap_exists, + } + file_exists_calls = [call("source/{}".format(filename)) for filename in filename_exists] + copy_file_calls = [call("source/{}".format(filename), "artifacts") for filename, exists in filename_exists.items() if exists] + osutils.file_exists.assert_has_calls(file_exists_calls) + osutils.copy_file.assert_has_calls(copy_file_calls) @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") def test_raises_action_failed_when_copying_fails(self, OSUtilMock): @@ -112,7 +122,7 @@ def test_raises_action_failed_when_copying_fails(self, OSUtilMock): osutils.copy_file.side_effect = OSError() - action = NodejsNpmrcCopyAction("artifacts", "source", osutils=osutils) + action = NodejsNpmrcAndLockfileCopyAction("artifacts", "source", osutils=osutils) with self.assertRaises(ActionFailedError): action.execute() diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index c47938fd0..c80c69354 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -2,14 +2,16 @@ from unittest import TestCase +from parameterized import parameterized + from aws_lambda_builders.actions import CopySourceAction, CopyDependenciesAction, MoveDependenciesAction, CleanUpAction from aws_lambda_builders.architecture import ARM64 from aws_lambda_builders.workflows.nodejs_npm.workflow import NodejsNpmWorkflow from aws_lambda_builders.workflows.nodejs_npm.actions import ( NodejsNpmPackAction, NodejsNpmInstallAction, - NodejsNpmrcCopyAction, - NodejsNpmrcCleanUpAction, + NodejsNpmrcAndLockfileCopyAction, + NodejsNpmrcCleanUpAction, NodejsNpmCIAction, ) from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils @@ -26,13 +28,13 @@ def setUp(self): def test_workflow_sets_up_npm_actions_with_download_dependencies_without_dependencies_dir(self): - self.osutils_mock.file_exists.return_value = True + self.osutils_mock.file_exists.side_effect = [True, False, False] workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils_mock) self.assertEqual(len(workflow.actions), 5) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) - self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction) self.assertIsInstance(workflow.actions[2], CopySourceAction) self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) @@ -54,14 +56,14 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_with_depende self.assertEqual(len(workflow.actions), 5) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) - self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction) self.assertIsInstance(workflow.actions[2], CopySourceAction) self.assertIsInstance(workflow.actions[3], CopySourceAction) self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencies_dir(self): - self.osutils_mock.file_exists.return_value = True + self.osutils_mock.file_exists.side_effect = [True, False, False] workflow = NodejsNpmWorkflow( "source", @@ -76,7 +78,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencie self.assertEqual(len(workflow.actions), 7) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) - self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction) self.assertIsInstance(workflow.actions[2], CopySourceAction) self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) self.assertIsInstance(workflow.actions[4], CleanUpAction) @@ -100,13 +102,13 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_and_without_ self.assertEqual(len(workflow.actions), 4) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) - self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction) self.assertIsInstance(workflow.actions[2], CopySourceAction) self.assertIsInstance(workflow.actions[3], NodejsNpmrcCleanUpAction) def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): - self.osutils_mock.file_exists.return_value = True + self.osutils_mock.file_exists.side_effect = [True, False, False] workflow = NodejsNpmWorkflow( "source", @@ -122,13 +124,27 @@ def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): self.assertEqual(len(workflow.actions), 7) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) - self.assertIsInstance(workflow.actions[1], NodejsNpmrcCopyAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction) self.assertIsInstance(workflow.actions[2], CopySourceAction) self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) self.assertIsInstance(workflow.actions[4], CleanUpAction) self.assertIsInstance(workflow.actions[5], MoveDependenciesAction) self.assertIsInstance(workflow.actions[6], NodejsNpmrcCleanUpAction) + @parameterized.expand([[True, False], [False, True], [True, True]]) + def test_workflow_sets_up_npm_actions_with_ci_if_lockfile_exists(self, package_lock_exists, shrinkwrap_exists): + + self.osutils_mock.file_exists.side_effect = [True, package_lock_exists, shrinkwrap_exists] + + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils_mock) + + self.assertEqual(len(workflow.actions), 5) + self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction) + self.assertIsInstance(workflow.actions[2], CopySourceAction) + self.assertIsInstance(workflow.actions[3], NodejsNpmCIAction) + self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) + def test_workflow_only_copy_action(self): self.osutils_mock.file_exists.return_value = False From cb360652cd003dbf813a41c3a8db3deaa9e31fc0 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Thu, 20 Jan 2022 16:37:35 -0800 Subject: [PATCH 03/10] black and ignore lint --- aws_lambda_builders/workflows/nodejs_npm/workflow.py | 1 + tests/integration/workflows/nodejs_npm/test_nodejs_npm.py | 2 +- tests/unit/workflows/nodejs_npm/test_actions.py | 4 +++- tests/unit/workflows/nodejs_npm/test_workflow.py | 3 ++- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index fb962768f..071399f99 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -32,6 +32,7 @@ class NodejsNpmWorkflow(BaseWorkflow): EXCLUDED_FILES = (".aws-sam", ".git") + # pylint: disable=too-many-statements def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=None, osutils=None, **kwargs): super(NodejsNpmWorkflow, self).__init__( diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index f05969c48..858e876c7 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -129,7 +129,7 @@ def test_builds_project_with_lockfile(self, dir_name): expected_files_by_dir_name = { "package-lock": {"package-lock.json"}, "shrinkwrap": {"npm-shrinkwrap.json"}, - "package-lock-and-shrinkwrap": {"package-lock.json", "npm-shrinkwrap.json"} + "package-lock-and-shrinkwrap": {"package-lock.json", "npm-shrinkwrap.json"}, } source_dir = os.path.join(self.TEST_DATA_FOLDER, dir_name) diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index 7b87bb47a..4725ab175 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -111,7 +111,9 @@ def test_copies_into_a_project_if_file_exists( "npm-shrinkwrap.json": shrinkwrap_exists, } file_exists_calls = [call("source/{}".format(filename)) for filename in filename_exists] - copy_file_calls = [call("source/{}".format(filename), "artifacts") for filename, exists in filename_exists.items() if exists] + copy_file_calls = [ + call("source/{}".format(filename), "artifacts") for filename, exists in filename_exists.items() if exists + ] osutils.file_exists.assert_has_calls(file_exists_calls) osutils.copy_file.assert_has_calls(copy_file_calls) diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index c80c69354..117432026 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -11,7 +11,8 @@ NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcAndLockfileCopyAction, - NodejsNpmrcCleanUpAction, NodejsNpmCIAction, + NodejsNpmrcCleanUpAction, + NodejsNpmCIAction, ) from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils From 3c245df748483af54f7fee0a57b1cbf8f65c8da5 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Thu, 20 Jan 2022 18:31:39 -0800 Subject: [PATCH 04/10] undo .package-lock.json integration test change --- tests/integration/workflows/nodejs_npm/test_nodejs_npm.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index 858e876c7..a3bd6737d 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -99,7 +99,7 @@ def test_builds_project_with_remote_dependencies(self): output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) - expected_modules = {"minimal-request-promise", ".package-lock.json"} + expected_modules = {"minimal-request-promise"} output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) self.assertEqual(expected_modules, output_modules) @@ -119,7 +119,7 @@ def test_builds_project_with_npmrc(self): self.assertEqual(expected_files, output_files) - expected_modules = {"fake-http-request", ".package-lock.json"} + expected_modules = {"fake-http-request"} output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) self.assertEqual(expected_modules, output_modules) @@ -212,11 +212,11 @@ def test_builds_project_with_remote_dependencies_with_download_dependencies_and_ output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) - expected_modules = {"minimal-request-promise", ".package-lock.json"} + expected_modules = {"minimal-request-promise"} output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) self.assertEqual(expected_modules, output_modules) - expected_modules = {"minimal-request-promise", ".package-lock.json"} + expected_modules = {"minimal-request-promise"} output_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules"))) self.assertEqual(expected_modules, output_modules) From 2b7c48893e4c3d2d2ad62aae768f58e1945537d6 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Thu, 20 Jan 2022 18:31:50 -0800 Subject: [PATCH 05/10] update npm version to support npm ci --- .appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.appveyor.yml b/.appveyor.yml index 94bf1e486..5bb0e6861 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -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@5.7.0 -g" - sh: "npm -v" - sh: "echo $PATH" - sh: "java --version" From 014fa08146279904f870f519d00db0b1709f907b Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Mon, 24 Jan 2022 17:04:45 -0800 Subject: [PATCH 06/10] no need to copy shrinkwrap file after updating npm version --- .../workflows/nodejs_npm/actions.py | 2 +- .../unit/workflows/nodejs_npm/test_actions.py | 19 ++++++------------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index 5566f38d6..4051b0cba 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -198,7 +198,7 @@ def execute(self): """ try: - for filename in [".npmrc", "package-lock.json", "npm-shrinkwrap.json"]: + for filename in [".npmrc", "package-lock.json"]: file_path = self.osutils.joinpath(self.source_dir, filename) if self.osutils.file_exists(file_path): LOG.debug("%s copying in: %s", filename, self.artifacts_dir) diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index e7160f4a8..71ae719ff 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -125,31 +125,24 @@ def test_raises_action_failed_when_npm_fails(self, SubprocessNpmMock): class TestNodejsNpmrcAndLockfileCopyAction(TestCase): @parameterized.expand( [ - [False, False, False], - [True, False, False], - [False, True, False], - [False, False, True], - [True, True, False], - [True, False, True], - [False, True, True], - [True, True, True], + [False, False], + [True, False], + [False, True], + [True, True], ] ) @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") - def test_copies_into_a_project_if_file_exists( - self, npmrc_exists, package_lock_exists, shrinkwrap_exists, OSUtilMock - ): + def test_copies_into_a_project_if_file_exists(self, npmrc_exists, package_lock_exists, OSUtilMock): osutils = OSUtilMock.return_value osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) action = NodejsNpmrcAndLockfileCopyAction("artifacts", "source", osutils=osutils) - osutils.file_exists.side_effect = [npmrc_exists, package_lock_exists, shrinkwrap_exists] + osutils.file_exists.side_effect = [npmrc_exists, package_lock_exists] action.execute() filename_exists = { ".npmrc": npmrc_exists, "package-lock.json": package_lock_exists, - "npm-shrinkwrap.json": shrinkwrap_exists, } file_exists_calls = [call("source/{}".format(filename)) for filename in filename_exists] copy_file_calls = [ From f2e1665d5e9e6357bf66651ea4e9fbed36a38aa9 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Mon, 24 Jan 2022 17:34:51 -0800 Subject: [PATCH 07/10] refactoring --- .../workflows/nodejs_npm/workflow.py | 57 +++++++++++++------ 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index 5f5553cde..f675d4a55 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -7,7 +7,13 @@ from aws_lambda_builders.path_resolver import PathResolver from aws_lambda_builders.workflow import BaseWorkflow, Capability -from aws_lambda_builders.actions import CopySourceAction, CleanUpAction, CopyDependenciesAction, MoveDependenciesAction +from aws_lambda_builders.actions import ( + CopySourceAction, + CleanUpAction, + CopyDependenciesAction, + MoveDependenciesAction, + BaseAction, +) from aws_lambda_builders.utils import which from aws_lambda_builders.exceptions import WorkflowFailedError from .actions import ( @@ -99,23 +105,18 @@ def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manife tar_dest_dir, scratch_dir, manifest_path, osutils=osutils, subprocess_npm=subprocess_npm ) - npm_copy = NodejsNpmrcAndLockfileCopyAction(tar_package_dir, source_dir, osutils=osutils) + npm_copy_npmrc_and_lockfile = NodejsNpmrcAndLockfileCopyAction(tar_package_dir, source_dir, osutils=osutils) actions = [ npm_pack, - npm_copy, + npm_copy_npmrc_and_lockfile, CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES), ] if self.download_dependencies: - lockfile_path = osutils.joinpath(source_dir, "package-lock.json") - shrinkwrap_path = osutils.joinpath(source_dir, "npm-shrinkwrap.json") - # installed the dependencies into artifact folder - if osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path): - actions.append(NodejsNpmCIAction(artifacts_dir, subprocess_npm=subprocess_npm)) - else: - actions.append(NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm)) + install_action = NodejsNpmWorkflow.get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils) + actions.append(install_action) # if dependencies folder exists, copy or move dependencies from artifact folder to dependencies folder # depends on the combine_dependencies flag @@ -169,19 +170,13 @@ def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutil :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) - + install_action = NodejsNpmWorkflow.get_install_action(source_dir, source_dir, subprocess_npm, osutils) esbuild_action = EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) return [install_action, esbuild_action] @@ -213,3 +208,31 @@ 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")] + + @staticmethod + def get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils): + """ + Get the install action used to install dependencies at artifacts_dir + + :type source_dir: str + :param source_dir: an existing (readable) directory containing source files + + :type artifacts_dir: str + :param artifacts_dir: 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 + + :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm + :param subprocess_npm: An instance of the NPM process wrapper + + :rtype: BaseAction + :return: Install action to use + """ + lockfile_path = osutils.joinpath(source_dir, "package-lock.json") + shrinkwrap_path = osutils.joinpath(source_dir, "npm-shrinkwrap.json") + + if osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path): + return NodejsNpmCIAction(artifacts_dir, subprocess_npm=subprocess_npm) + else: + return NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm) From d4de15602749975a91a2588b17b7f84b38e53cd6 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Tue, 25 Jan 2022 10:27:53 -0800 Subject: [PATCH 08/10] missed is_production when refactoring --- aws_lambda_builders/workflows/nodejs_npm/workflow.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index f675d4a55..c8a73f7a7 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -176,7 +176,7 @@ def actions_with_bundler(self, source_dir, artifacts_dir, bundler_config, osutil executable_search_paths = executable_search_paths + self.executable_search_paths subprocess_esbuild = SubprocessEsbuild(osutils, executable_search_paths, which=which) - install_action = NodejsNpmWorkflow.get_install_action(source_dir, source_dir, subprocess_npm, osutils) + install_action = NodejsNpmWorkflow.get_install_action(source_dir, source_dir, subprocess_npm, osutils, False) esbuild_action = EsbuildBundleAction(source_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) return [install_action, esbuild_action] @@ -210,7 +210,7 @@ def get_resolvers(self): return [PathResolver(runtime=self.runtime, binary="npm")] @staticmethod - def get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils): + def get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils, is_production=True): """ Get the install action used to install dependencies at artifacts_dir @@ -226,6 +226,9 @@ def get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils): :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) + :rtype: BaseAction :return: Install action to use """ @@ -235,4 +238,4 @@ def get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils): if osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path): return NodejsNpmCIAction(artifacts_dir, subprocess_npm=subprocess_npm) else: - return NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm) + return NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm, is_production=is_production) From 6ec5af5dbdf592803742b8b6654bef12459f272d Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Tue, 25 Jan 2022 10:45:13 -0800 Subject: [PATCH 09/10] fix test --- .../workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py index 0de36f41d..72de46f5e 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm_with_esbuild.py @@ -38,7 +38,7 @@ def test_invokes_old_builder_without_feature_flag(self): runtime=self.runtime, ) - expected_files = {"included.js", "node_modules", "excluded.js", "package.json"} + expected_files = {"included.js", "node_modules", "excluded.js", "package.json", "package-lock.json"} output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) From a3ad019c45ccadf65dca284374a9c82289096ca7 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Tue, 25 Jan 2022 11:32:16 -0800 Subject: [PATCH 10/10] fix shrinkwrap file --- .../testdata/shrinkwrap/npm-shrinkwrap.json | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/npm-shrinkwrap.json b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/npm-shrinkwrap.json index beeb191f9..8232a5420 100644 --- a/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/npm-shrinkwrap.json +++ b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/npm-shrinkwrap.json @@ -11,6 +11,18 @@ "dependencies": { "minimal-request-promise": "*" } + }, + "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": { + "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=" } } }