diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index a7bfd13fa..423a18faf 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -161,14 +161,14 @@ def execute(self): raise ActionFailedError(str(ex)) -class NodejsNpmrcCopyAction(BaseAction): +class NodejsNpmrcAndLockfileCopyAction(BaseAction): """ - A Lambda Builder Action that copies NPM config file .npmrc + 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): @@ -184,7 +184,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 @@ -193,14 +193,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"]: + 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 f432d3d51..6cf515ef4 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -17,8 +17,9 @@ NodejsNpmPackAction, NodejsNpmLockFileCleanUpAction, NodejsNpmInstallAction, - NodejsNpmrcCopyAction, + NodejsNpmrcAndLockfileCopyAction, NodejsNpmrcCleanUpAction, + NodejsNpmCIAction, ) from .utils import OSUtils from .npm import SubprocessNpm @@ -91,17 +92,21 @@ def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manife 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) + + npm_copy_npmrc_and_lockfile = NodejsNpmrcAndLockfileCopyAction(tar_package_dir, source_dir, osutils=osutils) actions = [ npm_pack, - npm_copy_npmrc, + npm_copy_npmrc_and_lockfile, CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES), ] if self.download_dependencies: # installed the dependencies into artifact folder - actions.append(NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm)) + install_action = NodejsNpmWorkflow.get_install_action( + source_dir, artifacts_dir, subprocess_npm, osutils, self.options + ) + actions.append(install_action) # if dependencies folder exists, copy or move dependencies from artifact folder to dependencies folder # depends on the combine_dependencies flag @@ -138,3 +143,41 @@ 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, build_options, is_production=True): + """ + 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 + + :type build_options: Dict + :param build_options: Object containing build options configurations + + :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 + """ + lockfile_path = osutils.joinpath(source_dir, "package-lock.json") + shrinkwrap_path = osutils.joinpath(source_dir, "npm-shrinkwrap.json") + + npm_ci_option = False + if build_options and isinstance(build_options, dict): + npm_ci_option = build_options.get("use_npm_ci", False) + + if (osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path)) and npm_ci_option: + return NodejsNpmCIAction(artifacts_dir, subprocess_npm=subprocess_npm) + + return NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm, is_production=is_production) diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py index bb59c1545..fcd5de062 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py @@ -22,7 +22,7 @@ from .node import SubprocessNodejs from .utils import is_experimental_esbuild_scope from .esbuild import SubprocessEsbuild, EsbuildExecutionError -from ..nodejs_npm.actions import NodejsNpmCIAction, NodejsNpmInstallAction +from ..nodejs_npm import NodejsNpmWorkflow from ..nodejs_npm.npm import SubprocessNpm from ..nodejs_npm.utils import OSUtils from ...path_resolver import PathResolver @@ -101,9 +101,6 @@ def actions_with_bundler( :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") - actions: List[BaseAction] = [ CopySourceAction(source_dir, scratch_dir, excludes=self.EXCLUDED_FILES + tuple(["node_modules"])) ] @@ -126,10 +123,9 @@ def actions_with_bundler( ] esbuild_with_deps = EsbuildBundleAction(scratch_dir, artifacts_dir, bundler_config, osutils, subprocess_esbuild) - if osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path): - install_action = NodejsNpmCIAction(scratch_dir, subprocess_npm=subprocess_npm) - else: - install_action = NodejsNpmInstallAction(scratch_dir, subprocess_npm=subprocess_npm, is_production=False) + install_action = NodejsNpmWorkflow.get_install_action( + source_dir, scratch_dir, subprocess_npm, osutils, self.options, is_production=False + ) if self.download_dependencies and not self.dependencies_dir: return actions + [install_action, esbuild_with_deps] diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index 4a4cdb000..f11adec6c 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -6,6 +6,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 @@ -120,6 +122,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..8232a5420 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/shrinkwrap/npm-shrinkwrap.json @@ -0,0 +1,28 @@ +{ + "name": "shrinkwrap", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "name": "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/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 191191378..7b0ff51ba 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, NodejsNpmLockFileCleanUpAction, NodejsNpmCIAction, @@ -120,30 +121,34 @@ def test_raises_action_failed_when_npm_fails(self, SubprocessNpmMock): self.assertEqual(raised.exception.args[0], "NPM Failed: boom!") -class TestNodejsNpmrcCopyAction(TestCase): +class TestNodejsNpmrcAndLockfileCopyAction(TestCase): + @parameterized.expand( + [ + [False, False], + [True, False], + [False, 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, 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] 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, + } + 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): @@ -152,7 +157,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 6f558d715..6ebcc7162 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -1,5 +1,5 @@ from unittest import TestCase -from mock import patch +from mock import patch, call from aws_lambda_builders.actions import CopySourceAction, CleanUpAction, CopyDependenciesAction, MoveDependenciesAction from aws_lambda_builders.architecture import ARM64 @@ -7,9 +7,10 @@ from aws_lambda_builders.workflows.nodejs_npm.actions import ( NodejsNpmPackAction, NodejsNpmInstallAction, - NodejsNpmrcCopyAction, + NodejsNpmrcAndLockfileCopyAction, NodejsNpmrcCleanUpAction, NodejsNpmLockFileCleanUpAction, + NodejsNpmCIAction, ) @@ -42,11 +43,13 @@ def setUp(self, OSUtilMock): def test_workflow_sets_up_npm_actions_with_download_dependencies_without_dependencies_dir(self): self.osutils.file_exists.return_value = True + self.osutils.file_exists.side_effect = [True, False, False] + 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[1], NodejsNpmrcAndLockfileCopyAction) self.assertIsInstance(workflow.actions[2], CopySourceAction) self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) @@ -68,7 +71,7 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_with_depende 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], CopySourceAction) self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) @@ -77,12 +80,14 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_with_depende def test_workflow_sets_up_npm_actions_without_bundler_if_manifest_doesnt_request_it(self): + self.osutils.file_exists.side_effect = [True, False, False] + 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[1], NodejsNpmrcAndLockfileCopyAction) self.assertIsInstance(workflow.actions[2], CopySourceAction) self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) @@ -90,7 +95,7 @@ def test_workflow_sets_up_npm_actions_without_bundler_if_manifest_doesnt_request def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencies_dir(self): - self.osutils.file_exists.return_value = True + self.osutils.file_exists.side_effect = [True, False, False] workflow = NodejsNpmWorkflow( "source", @@ -105,7 +110,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencie self.assertEqual(len(workflow.actions), 9) 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) @@ -128,14 +133,14 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_and_without_ 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], NodejsNpmrcCleanUpAction) self.assertIsInstance(workflow.actions[4], NodejsNpmLockFileCleanUpAction) def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): - self.osutils.file_exists.return_value = True + self.osutils.file_exists.side_effect = [True, False, False] workflow = NodejsNpmWorkflow( "source", @@ -151,7 +156,7 @@ def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): self.assertEqual(len(workflow.actions), 9) 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) @@ -181,3 +186,49 @@ def test_must_validate_architecture(self): self.assertEqual(workflow.architecture, "x86_64") self.assertEqual(workflow_with_arm.architecture, "arm64") + + def test_workflow_uses_npm_ci_if_shrinkwrap_exists_and_npm_ci_enabled(self): + + self.osutils.file_exists.side_effect = [True, False, True] + + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + options={"use_npm_ci": True}, + ) + + self.assertEqual(len(workflow.actions), 6) + 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) + self.assertIsInstance(workflow.actions[5], NodejsNpmLockFileCleanUpAction) + self.osutils.file_exists.assert_has_calls( + [call("source/package-lock.json"), call("source/npm-shrinkwrap.json")] + ) + + def test_workflow_uses_npm_ci_if_lockfile_exists_and_npm_ci_enabled(self): + + self.osutils.file_exists.side_effect = [True, True] + + workflow = NodejsNpmWorkflow( + "source", + "artifacts", + "scratch_dir", + "manifest", + osutils=self.osutils, + options={"use_npm_ci": True}, + ) + + self.assertEqual(len(workflow.actions), 6) + 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) + self.assertIsInstance(workflow.actions[5], NodejsNpmLockFileCleanUpAction) + self.osutils.file_exists.assert_has_calls([call("source/package-lock.json")]) diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py index ccaaa4822..493318e52 100644 --- a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py @@ -38,7 +38,6 @@ def setUp(self, OSUtilMock): 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( @@ -61,7 +60,6 @@ def test_workflow_sets_up_npm_actions_with_bundler_if_manifest_requests_it(self) def test_sets_up_esbuild_search_path_from_npm_bin(self): self.popen.out = b"project/bin" - self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] workflow = NodejsNpmEsbuildWorkflow( "source", @@ -81,7 +79,6 @@ def test_sets_up_esbuild_search_path_from_npm_bin(self): def test_sets_up_esbuild_search_path_with_workflow_executable_search_paths_after_npm_bin(self): self.popen.out = b"project/bin" - self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] workflow = NodejsNpmEsbuildWorkflow( "source", @@ -100,7 +97,6 @@ def test_sets_up_esbuild_search_path_with_workflow_executable_search_paths_after def test_workflow_uses_npm_ci_if_lockfile_exists(self): - self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] self.osutils.file_exists.side_effect = [True, True] workflow = NodejsNpmEsbuildWorkflow( @@ -110,6 +106,7 @@ def test_workflow_uses_npm_ci_if_lockfile_exists(self): "manifest", osutils=self.osutils, experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + options={"use_npm_ci": True}, ) self.assertEqual(len(workflow.actions), 3) @@ -120,7 +117,6 @@ def test_workflow_uses_npm_ci_if_lockfile_exists(self): def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): - self.osutils.parse_json.side_effect = [{"aws_sam": {"bundler": "esbuild"}}] self.osutils.file_exists.side_effect = [True, False, True] workflow = NodejsNpmEsbuildWorkflow( @@ -130,6 +126,7 @@ def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): "manifest", osutils=self.osutils, experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD], + options={"use_npm_ci": True}, ) self.assertEqual(len(workflow.actions), 3) @@ -140,6 +137,27 @@ def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): [call("source/package-lock.json"), call("source/npm-shrinkwrap.json")] ) + def test_workflow_doesnt_use_npm_ci_no_options_config(self): + + 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), 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_must_validate_architecture(self): self.osutils.is_windows.side_effect = [False, False] self.osutils.popen.side_effect = [self.popen, self.popen] @@ -181,7 +199,7 @@ def test_workflow_sets_up_esbuild_actions_with_download_dependencies_without_dep self.assertEqual(len(workflow.actions), 3) self.assertIsInstance(workflow.actions[0], CopySourceAction) - self.assertIsInstance(workflow.actions[1], NodejsNpmCIAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmInstallAction) self.assertIsInstance(workflow.actions[2], EsbuildBundleAction) def test_workflow_sets_up_esbuild_actions_without_download_dependencies_with_dependencies_dir_combine_deps(self): @@ -243,7 +261,7 @@ def test_workflow_sets_up_esbuild_actions_with_download_dependencies_and_depende self.assertEqual(len(workflow.actions), 5) self.assertIsInstance(workflow.actions[0], CopySourceAction) - self.assertIsInstance(workflow.actions[1], NodejsNpmCIAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmInstallAction) self.assertIsInstance(workflow.actions[2], CleanUpAction) self.assertIsInstance(workflow.actions[3], EsbuildBundleAction) self.assertIsInstance(workflow.actions[4], CopyDependenciesAction) @@ -264,7 +282,7 @@ def test_workflow_sets_up_esbuild_actions_with_download_dependencies_and_depende self.assertEqual(len(workflow.actions), 6) self.assertIsInstance(workflow.actions[0], CopySourceAction) - self.assertIsInstance(workflow.actions[1], NodejsNpmCIAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmInstallAction) self.assertIsInstance(workflow.actions[2], CleanUpAction) self.assertIsInstance(workflow.actions[3], EsbuildCheckVersionAction) self.assertIsInstance(workflow.actions[4], EsbuildBundleAction)