From b544882b29fe886140c068b99c4d6153f0402377 Mon Sep 17 00:00:00 2001 From: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Date: Wed, 7 Jun 2023 17:11:35 -0700 Subject: [PATCH 1/3] fix: Enable --manifest flag for esbuild and npm build (#513) * feat: support manifest exist outside the source directory for NPM. * feat: support manifest exist outside the source directory for NPM ESBuild. * remove print --- aws_lambda_builders/actions.py | 24 +- .../workflows/nodejs_npm/workflow.py | 36 ++- .../workflows/nodejs_npm_esbuild/workflow.py | 39 ++- .../workflows/nodejs_npm/test_nodejs_npm.py | 275 ++++++++++++++++++ .../manifest/package.json | 15 + .../src/excluded.js | 2 + .../src/included.js | 2 + .../manifest/package.json | 14 + .../manifest-outside-root/src/excluded.js | 2 + .../manifest-outside-root/src/included.js | 2 + .../test_nodejs_npm_with_esbuild.py | 123 ++++++++ .../manifest/package.json | 16 + .../src/included.ts | 12 + .../manifest/package.json | 15 + .../src/included.ts | 10 + tests/unit/test_actions.py | 77 ++++- .../workflows/nodejs_npm/test_workflow.py | 78 ++++- .../nodejs_npm_esbuild/test_workflow.py | 81 ++++-- 18 files changed, 768 insertions(+), 55 deletions(-) create mode 100644 tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/manifest/package.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/src/excluded.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/src/included.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/manifest/package.json create mode 100644 tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/src/excluded.js create mode 100644 tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/src/included.js create mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root-with-local-depends/manifest/package.json create mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root-with-local-depends/src/included.ts create mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root/manifest/package.json create mode 100644 tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root/src/included.ts diff --git a/aws_lambda_builders/actions.py b/aws_lambda_builders/actions.py index 3d4a520f7..ae3a85421 100644 --- a/aws_lambda_builders/actions.py +++ b/aws_lambda_builders/actions.py @@ -6,7 +6,7 @@ import os import shutil from pathlib import Path -from typing import Iterator, Set, Tuple, Union +from typing import Iterator, Optional, Set, Tuple, Union from aws_lambda_builders import utils from aws_lambda_builders.utils import copytree, create_symlink_or_copy @@ -169,14 +169,15 @@ class CopyDependenciesAction(BaseAction): PURPOSE = Purpose.COPY_DEPENDENCIES - def __init__(self, source_dir, artifact_dir, destination_dir, maintain_symlinks=False): + def __init__(self, source_dir, artifact_dir, destination_dir, maintain_symlinks=False, manifest_dir=None): self.source_dir = source_dir self.artifact_dir = artifact_dir self.dest_dir = destination_dir + self.manifest_dir = manifest_dir self.maintain_symlinks = maintain_symlinks def execute(self): - deps_manager = DependencyManager(self.source_dir, self.artifact_dir, self.dest_dir) + deps_manager = DependencyManager(self.source_dir, self.artifact_dir, self.dest_dir, self.manifest_dir) for dependencies_source, new_destination in deps_manager.yield_source_dest(): if os.path.islink(dependencies_source) and self.maintain_symlinks: @@ -198,13 +199,14 @@ class MoveDependenciesAction(BaseAction): PURPOSE = Purpose.MOVE_DEPENDENCIES - def __init__(self, source_dir, artifact_dir, destination_dir): + def __init__(self, source_dir, artifact_dir, destination_dir, manifest_dir=None): self.source_dir = source_dir self.artifact_dir = artifact_dir self.dest_dir = destination_dir + self.manifest_dir = manifest_dir def execute(self): - deps_manager = DependencyManager(self.source_dir, self.artifact_dir, self.dest_dir) + deps_manager = DependencyManager(self.source_dir, self.artifact_dir, self.dest_dir, self.manifest_dir) for dependencies_source, new_destination in deps_manager.yield_source_dest(): # shutil.move can't create subfolders if this is the first file in that folder @@ -256,10 +258,11 @@ class DependencyManager: # This allows for the installation of dependencies in the source directory IGNORE_LIST = ["node_modules"] - def __init__(self, source_dir, artifact_dir, destination_dir) -> None: + def __init__(self, source_dir, artifact_dir, destination_dir, manifest_dir=None) -> None: self._source_dir: str = source_dir self._artifact_dir: str = artifact_dir self._dest_dir: str = destination_dir + self._manifest_dir: Optional[str] = manifest_dir self._dependencies: Set[str] = set() def yield_source_dest(self) -> Iterator[Tuple[str, str]]: @@ -268,12 +271,13 @@ def yield_source_dest(self) -> Iterator[Tuple[str, str]]: yield os.path.join(self._artifact_dir, dep), os.path.join(self._dest_dir, dep) def _set_dependencies(self) -> None: - source = self._get_source_files_exclude_deps() + source = self._get_source_files_exclude_deps(self._source_dir) + manifest = self._get_source_files_exclude_deps(self._manifest_dir) if self._manifest_dir else set() artifact = set(os.listdir(self._artifact_dir)) - self._dependencies = artifact - source + self._dependencies = artifact - (source.union(manifest)) - def _get_source_files_exclude_deps(self) -> Set[str]: - source_files = set(os.listdir(self._source_dir)) + def _get_source_files_exclude_deps(self, dir_path: str) -> Set[str]: + source_files = set(os.listdir(dir_path)) for item in self.IGNORE_LIST: if item in source_files: source_files.remove(item) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index d12a095df..2a2edebed 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -74,25 +74,49 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim npm_copy_npmrc_and_lockfile = NodejsNpmrcAndLockfileCopyAction(tar_package_dir, source_dir, osutils=osutils) + self.manifest_dir = self.osutils.dirname(self.manifest_path) + is_building_in_source = self.build_dir == self.source_dir + is_external_manifest = self.manifest_dir != self.source_dir self.actions = [ npm_pack, npm_copy_npmrc_and_lockfile, CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES), ] + if is_external_manifest: + # npm pack only copies source code if the manifest is in the same directory as the source code, we need to + # copy the source code if the customer specified a different manifest path + self.actions.append(CopySourceAction(self.source_dir, artifacts_dir, excludes=self.EXCLUDED_FILES)) + if self.download_dependencies: self.actions.append( NodejsNpmWorkflow.get_install_action( source_dir=source_dir, - install_dir=self.build_dir, + # run npm install in the directory where the manifest (package.json) exists if customer is building + # in source, and manifest directory is different from source. + # This will let NPM find the local dependencies that are defined in the manifest file (they are + # usually defined as relative to the manifest location, and that is why we run `npm install` in the + # manifest directory instead of source directory). + # If customer is not building in source, so it is ok to run `npm install` in the build + # directory (the artifacts directory in this case), as the local dependencies are not supported. + install_dir=self.manifest_dir if is_building_in_source and is_external_manifest else self.build_dir, subprocess_npm=subprocess_npm, osutils=osutils, build_options=self.options, - install_links=self.build_dir == self.source_dir, + install_links=is_building_in_source, ) ) - if self.download_dependencies and self.build_dir == self.source_dir: + if is_building_in_source and is_external_manifest: + # Since we run `npm install` in the manifest directory, so we need to link the node_modules directory in + # the source directory. + source_dependencies_path = os.path.join(self.source_dir, "node_modules") + manifest_dependencies_path = os.path.join(self.manifest_dir, "node_modules") + self.actions.append( + LinkSinglePathAction(source=manifest_dependencies_path, dest=source_dependencies_path) + ) + + if self.download_dependencies and is_building_in_source: self.actions += self._actions_for_linking_source_dependencies_to_artifacts # if no dependencies dir, just cleanup artifacts and we're done @@ -107,9 +131,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim # then copy them into the artifacts dir elif self.combine_dependencies: self.actions.append( - CopySourceAction( - self.dependencies_dir, artifacts_dir, maintain_symlinks=self.build_dir == self.source_dir - ) + CopySourceAction(self.dependencies_dir, artifacts_dir, maintain_symlinks=is_building_in_source) ) self.actions += self._actions_for_cleanup @@ -145,6 +167,7 @@ def _actions_for_updating_dependencies_dir(self): artifact_dir=self.artifacts_dir, destination_dir=self.dependencies_dir, maintain_symlinks=self.build_dir == self.source_dir, + manifest_dir=self.manifest_dir, ) ) else: @@ -153,6 +176,7 @@ def _actions_for_updating_dependencies_dir(self): source_dir=self.source_dir, artifact_dir=self.artifacts_dir, destination_dir=self.dependencies_dir, + manifest_dir=self.manifest_dir, ) ) diff --git a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py index f2e41d7b2..fdb07ce54 100644 --- a/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py @@ -4,11 +4,13 @@ import json import logging +import os from pathlib import Path from aws_lambda_builders.actions import ( CleanUpAction, CopySourceAction, + LinkSinglePathAction, LinkSourceAction, MoveDependenciesAction, ) @@ -53,6 +55,10 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim self.osutils = osutils or OSUtils() self.subprocess_npm = SubprocessNpm(self.osutils) self.subprocess_esbuild = self._get_esbuild_subprocess() + self.manifest_dir = self.osutils.dirname(self.manifest_path) + + is_building_in_source = self.build_dir == self.source_dir + is_external_manifest = self.manifest_dir != self.source_dir bundler_config = self.get_build_properties() @@ -80,22 +86,45 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim # if we're building in the source directory, we don't have to copy the source code self.actions = ( [] - if self.build_dir == self.source_dir + if is_building_in_source else [CopySourceAction(source_dir=self.source_dir, dest_dir=self.build_dir, excludes=self.EXCLUDED_FILES)] ) + if is_external_manifest and not is_building_in_source: + # copy the manifest file (package.json) to the build directory in case if the manifest file is not in the + # same directory as the source code, and customer is not building in source. + self.actions.append( + CopySourceAction(source_dir=self.manifest_dir, dest_dir=self.build_dir, excludes=self.EXCLUDED_FILES) + ) + if self.download_dependencies: self.actions.append( NodejsNpmWorkflow.get_install_action( source_dir=source_dir, - install_dir=self.build_dir, + # run npm install in the directory where the manifest (package.json) exists if customer is building + # in source, and manifest directory is different from source. + # This will let NPM find the local dependencies that are defined in the manifest file (they are + # usually defined as relative to the manifest location, and that is why we run `npm install` in the + # manifest directory instead of source directory). + # If customer is not building in source, so it is ok to run `npm install` in the build + # directory (the artifacts directory in this case), as the local dependencies are not supported. + install_dir=self.manifest_dir if is_building_in_source and is_external_manifest else self.build_dir, subprocess_npm=self.subprocess_npm, osutils=self.osutils, build_options=self.options, - install_links=self.build_dir == self.source_dir, + install_links=is_building_in_source, ) ) + if is_building_in_source and is_external_manifest: + # Since we run `npm install` in the manifest directory, so we need to link the node_modules directory in + # the source directory. + source_dependencies_path = os.path.join(self.source_dir, "node_modules") + manifest_dependencies_path = os.path.join(self.manifest_dir, "node_modules") + self.actions.append( + LinkSinglePathAction(source=manifest_dependencies_path, dest=source_dependencies_path) + ) + bundle_action = EsbuildBundleAction( working_directory=self.build_dir, output_directory=self.artifacts_dir, @@ -109,7 +138,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim # If there's no dependencies_dir, just bundle and we're done. # Same thing if we're building in the source directory (since the dependencies persist in # the source directory, we don't want to move them or symlink them back to the source) - if not self.dependencies_dir or self.build_dir == self.source_dir: + if not self.dependencies_dir or is_building_in_source: self.actions.append(bundle_action) return @@ -118,7 +147,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim self.actions += [ bundle_action, CleanUpAction(self.dependencies_dir), - MoveDependenciesAction(self.source_dir, self.scratch_dir, self.dependencies_dir), + MoveDependenciesAction(self.source_dir, self.scratch_dir, self.dependencies_dir, self.manifest_dir), ] else: # if we're reusing dependencies, then we need to symlink them before bundling diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index 2cb5b58c0..f6a755dd7 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -464,3 +464,278 @@ def test_build_in_source_reuse_saved_dependencies_dir(self, runtime): expected_files = {"package.json", "included.js", "node_modules"} output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_project_with_manifest_outside_root(self, runtime): + base_dir = os.path.join(self.temp_testdata_dir, "manifest-outside-root") + source_dir = os.path.join(base_dir, "src") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + ) + + # expected output + expected_files = {"package.json", "included.js", "excluded.js", "node_modules"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + # expected dependencies + expected_modules = {"minimal-request-promise"} + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) + self.assertEqual(expected_modules, output_modules) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_project_with_manifest_outside_root_with_reuse_saved_dependencies_dir(self, runtime): + base_dir = os.path.join(self.temp_testdata_dir, "manifest-outside-root") + source_dir = os.path.join(base_dir, "src") + expected_modules = {"minimal-request-promise"} + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + dependencies_dir=self.dependencies_dir, + ) + + # expected dependencies in dependencies directory + dependencies_dir_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules"))) + self.assertEqual(expected_modules, dependencies_dir_modules) + + # cleanup artifacts_dir to make sure we use dependencies from dependencies_dir + for filename in os.listdir(self.artifacts_dir): + file_path = os.path.join(self.artifacts_dir, filename) + if os.path.isfile(file_path) or os.path.islink(file_path): + os.remove(file_path) + else: + shutil.rmtree(file_path) + + # build again without downloading dependencies + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + dependencies_dir=self.dependencies_dir, + download_dependencies=False, + ) + + # expected output + expected_files = {"package.json", "included.js", "excluded.js", "node_modules"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + # expected dependencies + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) + self.assertEqual(expected_modules, output_modules) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_project_with_manifest_outside_root_with_dependencies_dir_and_not_combine(self, runtime): + base_dir = os.path.join(self.temp_testdata_dir, "manifest-outside-root") + source_dir = os.path.join(base_dir, "src") + expected_modules = {"minimal-request-promise"} + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + dependencies_dir=self.dependencies_dir, + combine_dependencies=False, + ) + + # expected output + expected_files = {"package.json", "included.js", "excluded.js"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + # expected dependencies in dependencies directory + dependencies_dir_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules"))) + self.assertEqual(expected_modules, dependencies_dir_modules) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_project_with_manifest_outside_root_with_dependencies_dir_and_combine(self, runtime): + base_dir = os.path.join(self.temp_testdata_dir, "manifest-outside-root") + source_dir = os.path.join(base_dir, "src") + expected_modules = {"minimal-request-promise"} + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + dependencies_dir=self.dependencies_dir, + combine_dependencies=True, + ) + + # expected output + expected_files = {"package.json", "included.js", "excluded.js", "node_modules"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + # expected dependencies in dependencies directory + artifacts_dir_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) + self.assertEqual(expected_modules, artifacts_dir_modules) + + # expected dependencies in dependencies directory + dependencies_dir_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules"))) + self.assertEqual(expected_modules, dependencies_dir_modules) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_project_with_manifest_outside_root_and_local_dependencies(self, runtime): + base_dir = os.path.join(self.temp_testdata_dir, "manifest-outside-root-with-local-dependency") + source_dir = os.path.join(base_dir, "src") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + build_in_source=True, + ) + + # expected output + expected_files = {"package.json", "included.js", "excluded.js", "node_modules"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + # expected dependencies in artifact directory + expected_modules = {"minimal-request-promise", "local-dependency", "axios"} + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) + self.assertTrue(all(expected_module in output_modules for expected_module in expected_modules)) + + # expected dependencies in source directory + source_modules = set(os.listdir(os.path.join(source_dir, "node_modules"))) + self.assertTrue(all(expected_module in source_modules for expected_module in expected_modules)) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_project_with_manifest_outside_root_and_local_dependencies_with_reuse_saved_dependencies_dir( + self, runtime + ): + base_dir = os.path.join(self.temp_testdata_dir, "manifest-outside-root-with-local-dependency") + source_dir = os.path.join(base_dir, "src") + expected_modules = {"minimal-request-promise", "local-dependency", "axios"} + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + build_in_source=True, + dependencies_dir=self.dependencies_dir, + ) + + # expected dependencies in dependencies directory + dependencies_dir_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules"))) + self.assertTrue(all(expected_module in dependencies_dir_modules for expected_module in expected_modules)) + + # cleanup artifacts_dir to make sure we use dependencies from dependencies_dir + for filename in os.listdir(self.artifacts_dir): + file_path = os.path.join(self.artifacts_dir, filename) + if os.path.isfile(file_path) or os.path.islink(file_path): + os.remove(file_path) + else: + shutil.rmtree(file_path) + + # build again without downloading dependencies + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + build_in_source=True, + dependencies_dir=self.dependencies_dir, + download_dependencies=False, + ) + + # expected output + expected_files = {"package.json", "included.js", "excluded.js", "node_modules"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + # expected dependencies in artifacts directory + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) + self.assertTrue(all(expected_module in output_modules for expected_module in expected_modules)) + + # expected dependencies in source directory + source_modules = set(os.listdir(os.path.join(source_dir, "node_modules"))) + self.assertTrue(all(expected_module in source_modules for expected_module in expected_modules)) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_project_with_manifest_outside_root_and_local_dependencies_with_dependencies_dir_and_not_combine( + self, runtime + ): + base_dir = os.path.join(self.temp_testdata_dir, "manifest-outside-root-with-local-dependency") + source_dir = os.path.join(base_dir, "src") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + build_in_source=True, + dependencies_dir=self.dependencies_dir, + combine_dependencies=False, + ) + + # expected output + expected_files = {"package.json", "included.js", "excluded.js"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + # expected dependencies in dependencies directory + expected_modules = {"minimal-request-promise", "local-dependency", "axios"} + output_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules"))) + self.assertTrue(all(expected_module in output_modules for expected_module in expected_modules)) + + # expected dependencies in source directory + source_modules = set(os.listdir(os.path.join(source_dir, "node_modules"))) + self.assertTrue(all(expected_module in source_modules for expected_module in expected_modules)) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_project_with_manifest_outside_root_and_local_dependencies_with_dependencies_dir_and_combine( + self, runtime + ): + base_dir = os.path.join(self.temp_testdata_dir, "manifest-outside-root-with-local-dependency") + source_dir = os.path.join(base_dir, "src") + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + build_in_source=True, + dependencies_dir=self.dependencies_dir, + combine_dependencies=True, + ) + + # expected output + expected_files = {"package.json", "included.js", "excluded.js", "node_modules"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + # expected dependencies in dependencies directory + expected_modules = {"minimal-request-promise", "local-dependency", "axios"} + dependencies_dir_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules"))) + self.assertTrue(all(expected_module in dependencies_dir_modules for expected_module in expected_modules)) + + # expected dependencies in artifacts directory + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) + self.assertTrue(all(expected_module in output_modules for expected_module in expected_modules)) + + # expected dependencies in source directory + source_modules = set(os.listdir(os.path.join(source_dir, "node_modules"))) + self.assertTrue(all(expected_module in source_modules for expected_module in expected_modules)) diff --git a/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/manifest/package.json b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/manifest/package.json new file mode 100644 index 000000000..0a3c4acbc --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/manifest/package.json @@ -0,0 +1,15 @@ +{ + "name": "manifest-outside-root", + "version": "1.0.0", + "description": "", + "files": [ + "../src/included.js" + ], + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "axios": "*", + "local-dependency": "file:../../npm-deps" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/src/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/src/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/src/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/src/included.js b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/src/included.js new file mode 100644 index 000000000..e8f963aee --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root-with-local-dependency/src/included.js @@ -0,0 +1,2 @@ +//included +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/manifest/package.json b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/manifest/package.json new file mode 100644 index 000000000..835990bdf --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/manifest/package.json @@ -0,0 +1,14 @@ +{ + "name": "manifest-outside-root", + "version": "1.0.0", + "description": "", + "files": [ + "../src/included.js" + ], + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "minimal-request-promise": "*" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/src/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/src/excluded.js new file mode 100644 index 000000000..8bf8be437 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/src/excluded.js @@ -0,0 +1,2 @@ +//excluded +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/src/included.js b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/src/included.js new file mode 100644 index 000000000..e8f963aee --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/manifest-outside-root/src/included.js @@ -0,0 +1,2 @@ +//included +const x = 1; diff --git a/tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py b/tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py index 126ed55c1..35b877d9c 100644 --- a/tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py +++ b/tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py @@ -30,6 +30,11 @@ def setUp(self): self.osutils = OSUtils() self._set_esbuild_binary_path() + # use this so tests don't modify actual testdata, and we can parallelize + self.temp_dir = tempfile.mkdtemp() + self.temp_testdata_dir = os.path.join(self.temp_dir, "testdata") + shutil.copytree(self.TEST_DATA_FOLDER, self.temp_testdata_dir) + def _set_esbuild_binary_path(self): npm = SubprocessNpm(self.osutils) esbuild_dir = os.path.join(self.TEST_DATA_FOLDER, "esbuild-binary") @@ -40,6 +45,7 @@ def _set_esbuild_binary_path(self): def tearDown(self): shutil.rmtree(self.artifacts_dir) shutil.rmtree(self.scratch_dir) + shutil.rmtree(self.temp_dir) # clean up dependencies that were installed in source dir source_dependencies = os.path.join(self.source_dir, "node_modules") @@ -512,3 +518,120 @@ def test_builds_javascript_project_ignoring_relevant_flags(self, runtime): expected_files = {"included.js"} output_files = set(os.listdir(self.artifacts_dir)) self.assertEqual(expected_files, output_files) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_typescript_projects_with_external_manifest(self, runtime): + base_dir = os.path.join(self.TEST_DATA_FOLDER, "esbuild-manifest-outside-root") + source_dir = os.path.join(base_dir, "src") + + options = {"entry_points": ["included.ts"]} + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + options=options, + experimental_flags=[], + executable_search_paths=[self.binpath], + ) + + expected_files = {"included.js"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_typescript_projects_with_external_manifest_with_dependencies_dir_without_combine(self, runtime): + base_dir = os.path.join(self.TEST_DATA_FOLDER, "esbuild-manifest-outside-root") + source_dir = os.path.join(base_dir, "src") + + options = {"entry_points": ["included.ts"]} + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + options=options, + experimental_flags=[], + executable_search_paths=[self.binpath], + dependencies_dir=self.dependencies_dir, + download_dependencies=True, + combine_dependencies=False, + ) + + expected_files = {"included.js"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + expected_modules = "minimal-request-promise" + output_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules"))) + self.assertIn(expected_modules, output_modules) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_typescript_projects_with_external_manifest_with_dependencies_dir_with_combine(self, runtime): + base_dir = os.path.join(self.TEST_DATA_FOLDER, "esbuild-manifest-outside-root") + source_dir = os.path.join(base_dir, "src") + + options = {"entry_points": ["included.ts"]} + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + options=options, + experimental_flags=[], + executable_search_paths=[self.binpath], + dependencies_dir=self.dependencies_dir, + download_dependencies=True, + combine_dependencies=True, + ) + + expected_files = {"included.js"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + expected_modules = "minimal-request-promise" + output_modules = set(os.listdir(os.path.join(self.dependencies_dir, "node_modules"))) + self.assertIn(expected_modules, output_modules) + + @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",)]) + def test_builds_typescript_projects_with_external_manifest_and_local_depends(self, runtime): + base_dir = os.path.join(self.temp_testdata_dir, "esbuild-manifest-outside-root-with-local-depends") + source_dir = os.path.join(base_dir, "src") + + options = {"entry_points": ["included.ts"]} + + self.builder.build( + source_dir, + self.artifacts_dir, + self.scratch_dir, + os.path.join(base_dir, "manifest", "package.json"), + runtime=runtime, + options=options, + experimental_flags=[], + executable_search_paths=[self.binpath], + build_in_source=True, + ) + + expected_files = {"included.js"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEqual(expected_files, output_files) + + # dependencies installed in source folder + self.assertIn("node_modules", os.listdir(source_dir)) + self.assertIn("node_modules", os.listdir(os.path.join(base_dir, "manifest"))) + + expected_modules = ["minimal-request-promise", "axios"] + output_modules = set(os.listdir(os.path.join(source_dir, "node_modules"))) + self.assertTrue(all(expected_module in output_modules for expected_module in expected_modules)) + + output_modules = set(os.listdir(os.path.join(os.path.join(base_dir, "manifest"), "node_modules"))) + self.assertTrue(all(expected_module in output_modules for expected_module in expected_modules)) + + # dependencies not in scratch + self.assertNotIn("node_modules", os.listdir(self.scratch_dir)) diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root-with-local-depends/manifest/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root-with-local-depends/manifest/package.json new file mode 100644 index 000000000..d3f7f764c --- /dev/null +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root-with-local-depends/manifest/package.json @@ -0,0 +1,16 @@ +{ + "name": "with-deps-esbuild-typescript", + "version": "1.0.0", + "description": "", + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "@types/aws-lambda": "^8.10.76", + "axios": "*", + "local-dependency": "file:../../with-deps-esbuild" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } +} diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root-with-local-depends/src/included.ts b/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root-with-local-depends/src/included.ts new file mode 100644 index 000000000..e3ace99aa --- /dev/null +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root-with-local-depends/src/included.ts @@ -0,0 +1,12 @@ +import { APIGatewayProxyEvent, APIGatewayProxyResult } from "aws-lambda"; +import {axios} from "axios"; +import * as requestPromise from "request-promise"; + + +export const lambdaHandler = async (event: APIGatewayProxyEvent): Promise => { + const queries = JSON.stringify(event.queryStringParameters); + return { + statusCode: 200, + body: "OK" + } +} diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root/manifest/package.json b/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root/manifest/package.json new file mode 100644 index 000000000..3008b58f3 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root/manifest/package.json @@ -0,0 +1,15 @@ +{ + "name": "with-deps-esbuild-typescript", + "version": "1.0.0", + "description": "", + "keywords": [], + "author": "", + "license": "APACHE2.0", + "dependencies": { + "@types/aws-lambda": "^8.10.76", + "minimal-request-promise": "^1.5.0" + }, + "devDependencies": { + "esbuild": "^0.11.23" + } +} diff --git a/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root/src/included.ts b/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root/src/included.ts new file mode 100644 index 000000000..29a6f863b --- /dev/null +++ b/tests/integration/workflows/nodejs_npm_esbuild/testdata/esbuild-manifest-outside-root/src/included.ts @@ -0,0 +1,10 @@ +import { APIGatewayProxyEvent, APIGatewayProxyResult } from "aws-lambda"; +import * as requestPromise from 'request-promise'; + +export const lambdaHandler = async (event: APIGatewayProxyEvent): Promise => { + const queries = JSON.stringify(event.queryStringParameters); + return { + statusCode: 200, + body: "OK" + } +} diff --git a/tests/unit/test_actions.py b/tests/unit/test_actions.py index 8381d2bb8..1314c5f60 100644 --- a/tests/unit/test_actions.py +++ b/tests/unit/test_actions.py @@ -91,6 +91,35 @@ def test_must_copy( copy2_mock.assert_called_once_with("file1", "file2") makedirs_mock.assert_called_once_with("parent_dir_1", exist_ok=True) + @patch("aws_lambda_builders.actions.os.makedirs") + @patch("aws_lambda_builders.actions.os.path.dirname") + @patch("aws_lambda_builders.actions.shutil.copy2") + @patch("aws_lambda_builders.actions.copytree") + @patch("aws_lambda_builders.actions.os.path.isdir") + @patch("aws_lambda_builders.actions.os.listdir") + @patch("aws_lambda_builders.actions.os.path.join") + def test_must_copy_with_external_manifest( + self, path_mock, listdir_mock, isdir_mock, copytree_mock, copy2_mock, dirname_mock, makedirs_mock + ): + source_dir = "source" + artifact_dir = "artifact" + dest_dir = "dest" + manifest_dir = "manifest" + + listdir_mock.side_effect = [[1], [2], [1, 2, 3, 4]] + path_mock.side_effect = ["dir1", "dir2", "file1", "file2"] + isdir_mock.side_effect = [True, False] + dirname_mock.side_effect = ["parent_dir_1"] + action = CopyDependenciesAction(source_dir, artifact_dir, dest_dir, manifest_dir=manifest_dir) + action.execute() + + listdir_mock.assert_any_call(source_dir) + listdir_mock.assert_any_call(manifest_dir) + listdir_mock.assert_any_call(artifact_dir) + copytree_mock.assert_called_once_with("dir1", "dir2", maintain_symlinks=False) + copy2_mock.assert_called_once_with("file1", "file2") + makedirs_mock.assert_called_once_with("parent_dir_1", exist_ok=True) + class TestMoveDependenciesAction_execute(TestCase): @patch("aws_lambda_builders.actions.shutil.move") @@ -111,6 +140,25 @@ def test_must_copy(self, path_mock, listdir_mock, move_mock): move_mock.assert_any_call("dir1", "dir2") move_mock.assert_any_call("file1", "file2") + @patch("aws_lambda_builders.actions.shutil.move") + @patch("aws_lambda_builders.actions.os.listdir") + @patch("aws_lambda_builders.actions.os.path.join") + def test_must_copy_with_manifest(self, path_mock, listdir_mock, move_mock): + source_dir = "source" + artifact_dir = "artifact" + dest_dir = "dest" + manifest_dir = "manifest" + + listdir_mock.side_effect = [[1], [2], [1, 2, 3]] + path_mock.side_effect = ["dir1", "dir2", "file1", "file2"] + action = MoveDependenciesAction(source_dir, artifact_dir, dest_dir, manifest_dir=manifest_dir) + action.execute() + + listdir_mock.assert_any_call(source_dir) + listdir_mock.assert_any_call(manifest_dir) + listdir_mock.assert_any_call(artifact_dir) + move_mock.assert_any_call("dir1", "dir2") + class TestCleanUpAction_execute(TestCase): @patch("aws_lambda_builders.actions.os.remove") @@ -155,32 +203,55 @@ class TestDependencyManager(TestCase): ( ["app.js", "package.js", "libs", "node_modules"], ["app.js", "package.js", "libs", "node_modules"], + None, + [("artifacts/node_modules", "dest/node_modules")], + None, + None, + ), + ( + ["app.js", "libs", "node_modules"], + ["app.js", "package.js", "libs", "node_modules"], + ["package.js"], [("artifacts/node_modules", "dest/node_modules")], None, + "manifest", ), ( ["file1, file2", "dep1", "dep2"], ["file1, file2", "dep1", "dep2"], + None, [("artifacts/dep1", "dest/dep1"), ("artifacts/dep2", "dest/dep2")], ["dep1", "dep2"], + None, ), ( ["file1, file2"], ["file1, file2", "dep1", "dep2"], + None, [("artifacts/dep1", "dest/dep1"), ("artifacts/dep2", "dest/dep2")], ["dep1", "dep2"], + None, ), ] ) @patch("aws_lambda_builders.actions.os.listdir") def test_excludes_dependencies_from_source( - self, source_files, artifact_files, expected, mock_dependencies, patched_list_dir + self, + source_files, + artifact_files, + manifest_files, + expected, + mock_dependencies, + manifest_dir, + patched_list_dir, ): - dependency_manager = DependencyManager("source", "artifacts", "dest") + dependency_manager = DependencyManager("source", "artifacts", "dest", manifest_dir) dependency_manager.IGNORE_LIST = ( dependency_manager.IGNORE_LIST if mock_dependencies is None else mock_dependencies ) - patched_list_dir.side_effect = [source_files, artifact_files] + patched_list_dir.side_effect = ( + [source_files, manifest_files, artifact_files] if manifest_dir else [source_files, artifact_files] + ) source_destinations = list( TestDependencyManager._convert_strings_to_paths(list(dependency_manager.yield_source_dest())) ) diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index 2acb8da39..222cd7876 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -42,6 +42,7 @@ class TestNodejsNpmWorkflow(TestCase): def setUp(self, OSUtilMock): self.osutils = OSUtilMock.return_value self.osutils.pipe = "PIPE" + self.osutils.dirname.return_value = "source" self.popen = FakePopen() self.osutils.popen.side_effect = [self.popen] self.osutils.is_windows.side_effect = [False] @@ -52,7 +53,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende self.osutils.file_exists.side_effect = [True, False, False] - workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest", osutils=self.osutils) + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "source/manifest", osutils=self.osutils) self.assertEqual(len(workflow.actions), 6) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) @@ -62,6 +63,55 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) self.assertIsInstance(workflow.actions[5], NodejsNpmLockFileCleanUpAction) + def test_workflow_sets_up_npm_actions_with_download_dependencies_without_dependencies_dir_external_manifest(self): + self.osutils.dirname.return_value = "not_source" + self.osutils.file_exists.return_value = True + + self.osutils.file_exists.side_effect = [True, False, False] + + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "not_source/manifest", osutils=self.osutils) + + self.assertEqual(len(workflow.actions), 7) + self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction) + self.assertIsInstance(workflow.actions[2], CopySourceAction) + self.assertIsInstance(workflow.actions[3], CopySourceAction) + self.assertEqual(workflow.actions[3].source_dir, "source") + self.assertEqual(workflow.actions[3].dest_dir, "artifacts") + self.assertIsInstance(workflow.actions[4], NodejsNpmInstallAction) + self.assertEqual(workflow.actions[4].install_dir, "artifacts") + self.assertIsInstance(workflow.actions[5], NodejsNpmrcCleanUpAction) + self.assertIsInstance(workflow.actions[6], NodejsNpmLockFileCleanUpAction) + + def test_workflow_sets_up_npm_actions_with_download_dependencies_without_dependencies_dir_external_manifest_and_build_in_source( + self, + ): + self.osutils.dirname.return_value = "not_source" + self.osutils.file_exists.return_value = True + + self.osutils.file_exists.side_effect = [True, False, False] + + workflow = NodejsNpmWorkflow( + "source", "artifacts", "scratch_dir", "not_source/manifest", osutils=self.osutils, build_in_source=True + ) + + self.assertEqual(len(workflow.actions), 8) + self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) + self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction) + self.assertIsInstance(workflow.actions[2], CopySourceAction) + self.assertIsInstance(workflow.actions[3], CopySourceAction) + self.assertEqual(workflow.actions[3].source_dir, "source") + self.assertEqual(workflow.actions[3].dest_dir, "artifacts") + self.assertIsInstance(workflow.actions[4], NodejsNpmInstallAction) + self.assertEqual(workflow.actions[4].install_dir, "not_source") + self.assertIsInstance(workflow.actions[5], LinkSinglePathAction) + self.assertEqual(workflow.actions[5]._source, os.path.join("not_source", "node_modules")) + self.assertEqual(workflow.actions[5]._dest, os.path.join("source", "node_modules")) + self.assertIsInstance(workflow.actions[6], LinkSinglePathAction) + self.assertEqual(workflow.actions[6]._source, os.path.join("source", "node_modules")) + self.assertEqual(workflow.actions[6]._dest, os.path.join("artifacts", "node_modules")) + self.assertIsInstance(workflow.actions[7], NodejsNpmrcCleanUpAction) + def test_workflow_sets_up_npm_actions_without_download_dependencies_with_dependencies_dir(self): self.osutils.file_exists.return_value = True @@ -69,7 +119,7 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_with_depende "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", dependencies_dir="dep", download_dependencies=False, osutils=self.osutils, @@ -88,7 +138,7 @@ 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) + workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "source/manifest", osutils=self.osutils) self.assertEqual(len(workflow.actions), 6) @@ -106,7 +156,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_and_dependencie "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", dependencies_dir="dep", download_dependencies=True, osutils=self.osutils, @@ -129,7 +179,7 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_and_without_ "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", dependencies_dir=None, download_dependencies=False, osutils=self.osutils, @@ -150,7 +200,7 @@ def test_workflow_sets_up_npm_actions_without_combine_dependencies(self): "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", dependencies_dir="dep", download_dependencies=True, combine_dependencies=False, @@ -175,7 +225,7 @@ def test_must_validate_architecture(self): "source", "artifacts", "scratch", - "manifest", + "source/manifest", options={"artifact_executable_name": "foo"}, osutils=self.osutils, ) @@ -183,7 +233,7 @@ def test_must_validate_architecture(self): "source", "artifacts", "scratch", - "manifest", + "source/manifest", architecture=ARM64, osutils=self.osutils, ) @@ -198,7 +248,7 @@ def test_workflow_uses_npm_ci_if_shrinkwrap_exists_and_npm_ci_enabled(self): "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=self.osutils, options={"use_npm_ci": True}, ) @@ -221,7 +271,7 @@ def test_workflow_uses_npm_ci_if_lockfile_exists_and_npm_ci_enabled(self): "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=self.osutils, options={"use_npm_ci": True}, ) @@ -242,7 +292,7 @@ def test_build_in_source_without_download_dependencies_and_without_dependencies_ source_dir=source_dir, artifacts_dir=artifacts_dir, scratch_dir="scratch_dir", - manifest_path="manifest", + manifest_path="source/manifest", osutils=self.osutils, build_in_source=True, download_dependencies=False, @@ -261,7 +311,7 @@ def test_build_in_source_with_download_dependencies(self): source_dir=source_dir, artifacts_dir=artifacts_dir, scratch_dir="scratch_dir", - manifest_path="manifest", + manifest_path="source/manifest", osutils=self.osutils, build_in_source=True, ) @@ -284,7 +334,7 @@ def test_build_in_source_with_download_dependencies_and_dependencies_dir(self): source_dir=source_dir, artifacts_dir=artifacts_dir, scratch_dir="scratch_dir", - manifest_path="manifest", + manifest_path="source/manifest", osutils=self.osutils, build_in_source=True, dependencies_dir="dep", @@ -310,7 +360,7 @@ def test_build_in_source_with_dependencies_dir(self): source_dir=source_dir, artifacts_dir=artifacts_dir, scratch_dir="scratch_dir", - manifest_path="manifest", + manifest_path="source/manifest", osutils=self.osutils, build_in_source=True, dependencies_dir="dep", diff --git a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py index 6a06f3536..de4cfed15 100644 --- a/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py @@ -1,3 +1,4 @@ +import os from pathlib import Path from unittest import TestCase from unittest.mock import ANY, patch, call @@ -7,6 +8,7 @@ CleanUpAction, MoveDependenciesAction, LinkSourceAction, + LinkSinglePathAction, ) from aws_lambda_builders.architecture import ARM64 from aws_lambda_builders.workflows.nodejs_npm.actions import NodejsNpmInstallAction, NodejsNpmCIAction @@ -39,6 +41,7 @@ def setUp(self, OSUtilMock): self.osutils.pipe = "PIPE" self.popen = FakePopen() self.osutils.popen.side_effect = [self.popen] + self.osutils.dirname.return_value = "source" self.osutils.is_windows.side_effect = [False] self.osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b) @@ -49,7 +52,7 @@ def test_workflow_sets_up_npm_actions_with_bundler_if_manifest_requests_it(self) "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=self.osutils, experimental_flags=[], ) @@ -69,7 +72,7 @@ def test_sets_up_esbuild_search_path_from_npm_bin(self): "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=self.osutils, experimental_flags=[], ) @@ -87,7 +90,7 @@ def test_sets_up_esbuild_search_path_with_workflow_executable_search_paths_after "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=self.osutils, executable_search_paths=["other/bin"], experimental_flags=[], @@ -105,7 +108,7 @@ def test_workflow_uses_npm_ci_if_lockfile_exists(self): "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=self.osutils, experimental_flags=[], options={"use_npm_ci": True}, @@ -124,7 +127,7 @@ def test_workflow_uses_npm_ci_if_shrinkwrap_exists(self): "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=self.osutils, experimental_flags=[], options={"use_npm_ci": True}, @@ -145,7 +148,7 @@ def test_workflow_doesnt_use_npm_ci_no_options_config(self): "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=self.osutils, experimental_flags=[], ) @@ -166,7 +169,7 @@ def test_must_validate_architecture(self): "source", "artifacts", "scratch", - "manifest", + "source/manifest", options={"artifact_executable_name": "foo"}, osutils=self.osutils, experimental_flags=[], @@ -176,7 +179,7 @@ def test_must_validate_architecture(self): "source", "artifacts", "scratch", - "manifest", + "source/manifest", architecture=ARM64, osutils=self.osutils, experimental_flags=[], @@ -192,7 +195,7 @@ def test_workflow_sets_up_esbuild_actions_with_download_dependencies_without_dep "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=self.osutils, experimental_flags=[], ) @@ -209,7 +212,7 @@ def test_workflow_sets_up_esbuild_actions_without_download_dependencies_with_dep "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", dependencies_dir="dep", download_dependencies=False, combine_dependencies=True, @@ -229,7 +232,7 @@ def test_workflow_sets_up_esbuild_actions_without_download_dependencies_with_dep "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", dependencies_dir="dep", download_dependencies=False, combine_dependencies=False, @@ -249,7 +252,7 @@ def test_workflow_sets_up_esbuild_actions_with_download_dependencies_and_depende "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", dependencies_dir="dep", download_dependencies=True, osutils=self.osutils, @@ -269,7 +272,7 @@ def test_workflow_sets_up_esbuild_actions_with_download_dependencies_and_depende "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", dependencies_dir="dep", download_dependencies=True, combine_dependencies=False, @@ -291,7 +294,7 @@ def test_workflow_uses_production_npm_version(self, get_workflow_mock): "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", dependencies_dir=None, download_dependencies=True, combine_dependencies=False, @@ -322,7 +325,7 @@ def test_manifest_not_found(self, osutils_mock, subprocess_npm_mock, get_esbuild "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=osutils_mock, ) @@ -335,7 +338,7 @@ def test_no_download_dependencies_and_no_dependencies_dir_fails(self): "source", "artifacts", "scratch_dir", - "manifest", + "source/manifest", osutils=self.osutils, download_dependencies=False, ) @@ -346,7 +349,7 @@ def test_build_in_source(self): source_dir=source_dir, artifacts_dir="artifacts", scratch_dir="scratch_dir", - manifest_path="manifest", + manifest_path="source/manifest", osutils=self.osutils, build_in_source=True, ) @@ -357,3 +360,47 @@ def test_build_in_source(self): self.assertEqual(workflow.actions[0].install_dir, source_dir) self.assertIsInstance(workflow.actions[1], EsbuildBundleAction) self.assertEqual(workflow.actions[1]._working_directory, source_dir) + + def test_workflow_sets_up_npm_actions_with_download_dependencies_without_dependencies_dir_external_manifest(self): + self.osutils.dirname.return_value = "not_source" + + workflow = NodejsNpmEsbuildWorkflow( + source_dir="source", + artifacts_dir="artifacts", + scratch_dir="scratch_dir", + manifest_path="not_source/manifest", + osutils=self.osutils, + ) + + self.assertEqual(len(workflow.actions), 4) + + self.assertIsInstance(workflow.actions[0], CopySourceAction) + self.assertIsInstance(workflow.actions[1], CopySourceAction) + self.assertEquals(workflow.actions[1].source_dir, "not_source") + self.assertEquals(workflow.actions[1].dest_dir, "scratch_dir") + self.assertIsInstance(workflow.actions[2], NodejsNpmInstallAction) + self.assertEquals(workflow.actions[2].install_dir, "scratch_dir") + self.assertIsInstance(workflow.actions[3], EsbuildBundleAction) + + def test_workflow_sets_up_npm_actions_with_download_dependencies_without_dependencies_dir_external_manifest_and_build_in_source( + self, + ): + self.osutils.dirname.return_value = "not_source" + + workflow = NodejsNpmEsbuildWorkflow( + source_dir="source", + artifacts_dir="artifacts", + scratch_dir="scratch_dir", + manifest_path="not_source/manifest", + osutils=self.osutils, + build_in_source=True, + ) + + self.assertEqual(len(workflow.actions), 3) + + self.assertIsInstance(workflow.actions[0], NodejsNpmInstallAction) + self.assertEquals(workflow.actions[0].install_dir, "not_source") + self.assertIsInstance(workflow.actions[1], LinkSinglePathAction) + self.assertEquals(workflow.actions[1]._source, os.path.join("not_source", "node_modules")) + self.assertEquals(workflow.actions[1]._dest, os.path.join("source", "node_modules")) + self.assertIsInstance(workflow.actions[2], EsbuildBundleAction) From 2ba266fbde2c629c57c6ee7f01fea1802bc96952 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 7 Jun 2023 17:51:12 -0700 Subject: [PATCH 2/3] chore(deps-dev): bump coverage from 7.2.6 to 7.2.7 in /requirements (#512) Bumps [coverage](https://github.com/nedbat/coveragepy) from 7.2.6 to 7.2.7. - [Release notes](https://github.com/nedbat/coveragepy/releases) - [Changelog](https://github.com/nedbat/coveragepy/blob/master/CHANGES.rst) - [Commits](https://github.com/nedbat/coveragepy/compare/7.2.6...7.2.7) --- updated-dependencies: - dependency-name: coverage dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements/dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/dev.txt b/requirements/dev.txt index baa415117..34e4dbac4 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -1,4 +1,4 @@ -coverage==7.2.6 +coverage==7.2.7 flake8==3.3.0; python_version < '3.8' flake8==3.8.4; python_version >= '3.8' pytest-cov==4.1.0 From dd572fd503e4d63e4b1aed170aefbdcc47b81977 Mon Sep 17 00:00:00 2001 From: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Date: Mon, 12 Jun 2023 09:20:36 -0700 Subject: [PATCH 3/3] chore: Version bump to 1.34.0 (#514) Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> --- aws_lambda_builders/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws_lambda_builders/__init__.py b/aws_lambda_builders/__init__.py index 77421e254..8dc5ad22a 100644 --- a/aws_lambda_builders/__init__.py +++ b/aws_lambda_builders/__init__.py @@ -4,5 +4,5 @@ # Changing version will trigger a new release! # Please make the version change as the last step of your development. -__version__ = "1.33.0" +__version__ = "1.34.0" RPC_PROTOCOL_VERSION = "0.3"