From b89732cf812712fd44f1b8862e49c6baadb00bee Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Wed, 9 Dec 2020 23:00:40 -0600
Subject: [PATCH 01/24] proof of concept action updates working

---
 .../workflows/nodejs_npm/actions.py           | 103 ++++++++++++++++++
 .../workflows/nodejs_npm/utils.py             |  12 ++
 2 files changed, 115 insertions(+)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index 43259bd8c..79f679053 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -2,12 +2,15 @@
 Action to resolve NodeJS dependencies using NPM
 """
 
+import json
 import logging
+from os import sep as path_sep
 
 from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError
 from .npm import NpmExecutionError
 
 LOG = logging.getLogger(__name__)
+LOG.setLevel(logging.DEBUG)
 
 
 class NodejsNpmPackAction(BaseAction):
@@ -66,6 +69,20 @@ def execute(self):
 
             self.osutils.extract_tarfile(tarfile_path, self.artifacts_dir)
 
+            LOG.debug("NODEJS searching for local dependencies")
+
+            local_manifest_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'package.json')
+            local_dependencies = get_local_dependencies(self.manifest_path)
+            for (dep_name, dep_path) in local_dependencies.items():
+                dep_scratch_dir = self.osutils.joinpath(self.scratch_dir, str(abs(hash(dep_name))))
+                dep_artifacts_dir = self.osutils.joinpath(dep_scratch_dir, 'unpacked')
+                dependency_tarfile_path = package_local_dependency(package_path[5:], dep_path, dep_artifacts_dir, dep_scratch_dir, self.osutils, self.subprocess_npm)
+                local_packaged_dep_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'local_dep')
+                if not self.osutils.dir_exists(local_packaged_dep_path):
+                    self.osutils.mkdir(local_packaged_dep_path)
+                dependency_tarfile_path = self.osutils.copy_file(dependency_tarfile_path, self.osutils.joinpath(local_packaged_dep_path))
+                update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, self.osutils)
+
         except NpmExecutionError as ex:
             raise ActionFailedError(str(ex))
 
@@ -196,3 +213,89 @@ def execute(self):
 
         except OSError as ex:
             raise ActionFailedError(str(ex))
+
+
+def get_local_dependencies(manifest_path):
+    """
+    Helper function to extract all local dependencies in a package.json manifest
+    """
+
+    with open(manifest_path) as manifest_file:
+        manifest = json.loads(manifest_file.read())
+
+        if 'dependencies' in manifest:
+            return dict((k, v) for (k, v) in manifest['dependencies'].items() if is_local_dependency(v))
+        else:
+            return {}
+
+
+def is_local_dependency(path):
+    """
+    Helper function to check if package dependency is a local package
+    """
+
+    try:
+        return path.startswith('file:') or path.startswith('.')
+    except AttributeError:
+        return False
+
+
+def package_local_dependency(parent_package_path, rel_package_path, artifact_dir, scratch_dir, osutils, subprocess_npm):
+    if rel_package_path.startswith('file:'):
+        rel_package_path = rel_package_path[5:].strip()
+
+    if rel_package_path.startswith('.'):
+        package_path = osutils.abspath(osutils.joinpath(parent_package_path, rel_package_path))
+
+    LOG.debug("NODEJS packaging dependency %s to %s", package_path, scratch_dir)
+
+    if not osutils.dir_exists(scratch_dir):
+        osutils.mkdir(scratch_dir)
+
+    tarfile_name = subprocess_npm.run(["pack", "-q", package_path], cwd=scratch_dir).splitlines()[-1]
+
+    LOG.debug("NODEJS packed dependency to %s", tarfile_name)
+
+    tarfile_path = osutils.joinpath(scratch_dir, tarfile_name)
+
+    LOG.debug("NODEJS extracting to %s", artifact_dir)
+
+    osutils.extract_tarfile(tarfile_path, artifact_dir)
+
+    LOG.debug("NODEJS searching for subpackage local dependencies")
+
+    # manifest_path = osutils.joinpath(package_path, 'package.json')
+    # local_dependencies = get_local_dependencies(manifest_path)
+    # for (dep_name, dep_path) in local_dependencies.items():
+    #     dep_scratch_dir = osutils.joinpath(scratch_dir, '..', hash(dep_name))
+    #     dep_artifacts_dir = osutils.joinpath(dep_scratch_dir, 'unpacked')
+    #     dependency_tarfile_path = package_local_dependency(package_path[5:], dep_path, dep_artifacts_dir, dep_scratch_dir, self.osutils, self.subprocess_npm)
+    #     update_manifest(manifest_path, dep_name, dependency_tarfile_path)
+
+    localized_package_dir = osutils.joinpath(artifact_dir, 'package')
+
+    LOG.debug("NODEJS repackaging dependency %s to %s", artifact_dir, localized_package_dir)
+
+    tarfile_name = subprocess_npm.run(["pack", "-q", localized_package_dir], cwd=localized_package_dir).splitlines()[-1]
+
+    return osutils.joinpath(localized_package_dir, tarfile_name)
+
+
+def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
+    """
+    Helper function to update dependency path to localized tar
+    """
+
+    package_path = osutils.dirname(manifest_path)
+    manifest_backup = osutils.copy_file(manifest_path, f'{manifest_path}.bak')
+
+    with open(manifest_backup, 'r') as manifest_backup_file:
+        manifest = json.loads(manifest_backup_file.read())
+
+        if 'dependencies' in manifest and dep_name in manifest['dependencies']:
+            dep_rel_path = osutils.relative_path(dependency_tarfile_path, start=package_path)
+            dep_rel_path = osutils.joinpath('.', dep_rel_path).replace(path_sep, "/")
+            manifest['dependencies'][dep_name] = f'file:{dep_rel_path}'
+
+            with open(manifest_path, 'w') as manifest_write_file:
+                manifest_write_file.write(json.dumps(manifest, indent=4))
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index ad92cfd23..bd634dc8f 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -19,6 +19,9 @@ class OSUtils(object):
     def copy_file(self, file_path, destination_path):
         return shutil.copy2(file_path, destination_path)
 
+    def dir_exists(self, directory):
+        return os.path.isdir(directory)
+
     def extract_tarfile(self, tarfile_path, unpack_dir):
         with tarfile.open(tarfile_path, "r:*") as tar:
             tar.extractall(unpack_dir)
@@ -26,9 +29,15 @@ def extract_tarfile(self, tarfile_path, unpack_dir):
     def file_exists(self, filename):
         return os.path.isfile(filename)
 
+    def filename(self, filepath):
+        return os.path.basename(filepath)
+
     def joinpath(self, *args):
         return os.path.join(*args)
 
+    def mkdir(self, path, mode=0o777, *args, dir_fd=None):
+        return os.mkdir(path, mode, *args, dir_fd=dir_fd)
+
     def popen(self, command, stdout=None, stderr=None, env=None, cwd=None):
         p = subprocess.Popen(command, stdout=stdout, stderr=stderr, env=env, cwd=cwd)
         return p
@@ -40,6 +49,9 @@ def pipe(self):
     def dirname(self, path):
         return os.path.dirname(path)
 
+    def relative_path(self, path, start):
+        return os.path.relpath(path, start=start)
+
     def remove_file(self, filename):
         return os.remove(filename)
 

From 8e5c836ab6d315aace0d2b98c0b1523a9edc8d3e Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Wed, 9 Dec 2020 23:14:54 -0600
Subject: [PATCH 02/24] moved dependency helpers to utils.py

---
 .../workflows/nodejs_npm/actions.py           | 95 +------------------
 .../workflows/nodejs_npm/utils.py             | 94 ++++++++++++++++++
 2 files changed, 98 insertions(+), 91 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index 79f679053..d52a32823 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -2,12 +2,11 @@
 Action to resolve NodeJS dependencies using NPM
 """
 
-import json
 import logging
-from os import sep as path_sep
 
 from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError
 from .npm import NpmExecutionError
+from .utils import DependencyUtils
 
 LOG = logging.getLogger(__name__)
 LOG.setLevel(logging.DEBUG)
@@ -72,16 +71,16 @@ def execute(self):
             LOG.debug("NODEJS searching for local dependencies")
 
             local_manifest_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'package.json')
-            local_dependencies = get_local_dependencies(self.manifest_path)
+            local_dependencies = DependencyUtils.get_local_dependencies(self.manifest_path)
             for (dep_name, dep_path) in local_dependencies.items():
                 dep_scratch_dir = self.osutils.joinpath(self.scratch_dir, str(abs(hash(dep_name))))
                 dep_artifacts_dir = self.osutils.joinpath(dep_scratch_dir, 'unpacked')
-                dependency_tarfile_path = package_local_dependency(package_path[5:], dep_path, dep_artifacts_dir, dep_scratch_dir, self.osutils, self.subprocess_npm)
+                dependency_tarfile_path = DependencyUtils.package_local_dependency(package_path[5:], dep_path, dep_artifacts_dir, dep_scratch_dir, self.osutils, self.subprocess_npm)
                 local_packaged_dep_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'local_dep')
                 if not self.osutils.dir_exists(local_packaged_dep_path):
                     self.osutils.mkdir(local_packaged_dep_path)
                 dependency_tarfile_path = self.osutils.copy_file(dependency_tarfile_path, self.osutils.joinpath(local_packaged_dep_path))
-                update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, self.osutils)
+                DependencyUtils.update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, self.osutils)
 
         except NpmExecutionError as ex:
             raise ActionFailedError(str(ex))
@@ -213,89 +212,3 @@ def execute(self):
 
         except OSError as ex:
             raise ActionFailedError(str(ex))
-
-
-def get_local_dependencies(manifest_path):
-    """
-    Helper function to extract all local dependencies in a package.json manifest
-    """
-
-    with open(manifest_path) as manifest_file:
-        manifest = json.loads(manifest_file.read())
-
-        if 'dependencies' in manifest:
-            return dict((k, v) for (k, v) in manifest['dependencies'].items() if is_local_dependency(v))
-        else:
-            return {}
-
-
-def is_local_dependency(path):
-    """
-    Helper function to check if package dependency is a local package
-    """
-
-    try:
-        return path.startswith('file:') or path.startswith('.')
-    except AttributeError:
-        return False
-
-
-def package_local_dependency(parent_package_path, rel_package_path, artifact_dir, scratch_dir, osutils, subprocess_npm):
-    if rel_package_path.startswith('file:'):
-        rel_package_path = rel_package_path[5:].strip()
-
-    if rel_package_path.startswith('.'):
-        package_path = osutils.abspath(osutils.joinpath(parent_package_path, rel_package_path))
-
-    LOG.debug("NODEJS packaging dependency %s to %s", package_path, scratch_dir)
-
-    if not osutils.dir_exists(scratch_dir):
-        osutils.mkdir(scratch_dir)
-
-    tarfile_name = subprocess_npm.run(["pack", "-q", package_path], cwd=scratch_dir).splitlines()[-1]
-
-    LOG.debug("NODEJS packed dependency to %s", tarfile_name)
-
-    tarfile_path = osutils.joinpath(scratch_dir, tarfile_name)
-
-    LOG.debug("NODEJS extracting to %s", artifact_dir)
-
-    osutils.extract_tarfile(tarfile_path, artifact_dir)
-
-    LOG.debug("NODEJS searching for subpackage local dependencies")
-
-    # manifest_path = osutils.joinpath(package_path, 'package.json')
-    # local_dependencies = get_local_dependencies(manifest_path)
-    # for (dep_name, dep_path) in local_dependencies.items():
-    #     dep_scratch_dir = osutils.joinpath(scratch_dir, '..', hash(dep_name))
-    #     dep_artifacts_dir = osutils.joinpath(dep_scratch_dir, 'unpacked')
-    #     dependency_tarfile_path = package_local_dependency(package_path[5:], dep_path, dep_artifacts_dir, dep_scratch_dir, self.osutils, self.subprocess_npm)
-    #     update_manifest(manifest_path, dep_name, dependency_tarfile_path)
-
-    localized_package_dir = osutils.joinpath(artifact_dir, 'package')
-
-    LOG.debug("NODEJS repackaging dependency %s to %s", artifact_dir, localized_package_dir)
-
-    tarfile_name = subprocess_npm.run(["pack", "-q", localized_package_dir], cwd=localized_package_dir).splitlines()[-1]
-
-    return osutils.joinpath(localized_package_dir, tarfile_name)
-
-
-def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
-    """
-    Helper function to update dependency path to localized tar
-    """
-
-    package_path = osutils.dirname(manifest_path)
-    manifest_backup = osutils.copy_file(manifest_path, f'{manifest_path}.bak')
-
-    with open(manifest_backup, 'r') as manifest_backup_file:
-        manifest = json.loads(manifest_backup_file.read())
-
-        if 'dependencies' in manifest and dep_name in manifest['dependencies']:
-            dep_rel_path = osutils.relative_path(dependency_tarfile_path, start=package_path)
-            dep_rel_path = osutils.joinpath('.', dep_rel_path).replace(path_sep, "/")
-            manifest['dependencies'][dep_name] = f'file:{dep_rel_path}'
-
-            with open(manifest_path, 'w') as manifest_write_file:
-                manifest_write_file.write(json.dumps(manifest, indent=4))
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index bd634dc8f..3ce6340f0 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -2,6 +2,7 @@
 Commonly used utilities
 """
 
+import json
 import os
 import platform
 import tarfile
@@ -60,3 +61,96 @@ def abspath(self, path):
 
     def is_windows(self):
         return platform.system().lower() == "windows"
+
+
+class DependencyUtils(object):
+
+    """
+    Collection of helper functions for managing local NPM dependencies
+    """
+
+    def get_local_dependencies(manifest_path):
+        """
+        Helper function to extract all local dependencies in a package.json manifest
+        """
+
+        with open(manifest_path) as manifest_file:
+            manifest = json.loads(manifest_file.read())
+
+            if 'dependencies' in manifest:
+                return dict((k, v) for (k, v) in manifest['dependencies'].items() if DependencyUtils.is_local_dependency(v))
+            else:
+                return {}
+
+    def is_local_dependency(path):
+        """
+        Helper function to check if package dependency is a local package
+        """
+
+        try:
+            return path.startswith('file:') or path.startswith('.')
+        except AttributeError:
+            return False
+
+    def package_local_dependency(parent_package_path, rel_package_path, artifact_dir, scratch_dir, osutils, subprocess_npm):
+        if rel_package_path.startswith('file:'):
+            rel_package_path = rel_package_path[5:].strip()
+
+        if rel_package_path.startswith('.'):
+            package_path = osutils.abspath(osutils.joinpath(parent_package_path, rel_package_path))
+
+        # LOG.debug("NODEJS packaging dependency %s to %s", package_path, scratch_dir)
+
+        if not osutils.dir_exists(scratch_dir):
+            osutils.mkdir(scratch_dir)
+
+        tarfile_name = subprocess_npm.run(["pack", "-q", package_path], cwd=scratch_dir).splitlines()[-1]
+
+        # LOG.debug("NODEJS packed dependency to %s", tarfile_name)
+
+        tarfile_path = osutils.joinpath(scratch_dir, tarfile_name)
+
+        # LOG.debug("NODEJS extracting to %s", artifact_dir)
+
+        osutils.extract_tarfile(tarfile_path, artifact_dir)
+
+        # LOG.debug("NODEJS searching for subpackage local dependencies")
+
+        # local_manifest_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'package.json')
+        # local_dependencies = DependencyUtils.get_local_dependencies(self.manifest_path)
+        # for (dep_name, dep_path) in local_dependencies.items():
+        #     dep_scratch_dir = self.osutils.joinpath(self.scratch_dir, str(abs(hash(dep_name))))
+        #     dep_artifacts_dir = self.osutils.joinpath(dep_scratch_dir, 'unpacked')
+        #     dependency_tarfile_path = DependencyUtils.package_local_dependency(package_path[5:], dep_path, dep_artifacts_dir, dep_scratch_dir, self.osutils, self.subprocess_npm)
+        #     local_packaged_dep_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'local_dep')
+        #     if not self.osutils.dir_exists(local_packaged_dep_path):
+        #         self.osutils.mkdir(local_packaged_dep_path)
+        #     dependency_tarfile_path = self.osutils.copy_file(dependency_tarfile_path, self.osutils.joinpath(local_packaged_dep_path))
+        #     DependencyUtils.update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, self.osutils)
+
+        localized_package_dir = osutils.joinpath(artifact_dir, 'package')
+
+        # LOG.debug("NODEJS repackaging dependency %s to %s", artifact_dir, localized_package_dir)
+
+        tarfile_name = subprocess_npm.run(["pack", "-q", localized_package_dir], cwd=localized_package_dir).splitlines()[-1]
+
+        return osutils.joinpath(localized_package_dir, tarfile_name)
+
+    def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
+        """
+        Helper function to update dependency path to localized tar
+        """
+
+        package_path = osutils.dirname(manifest_path)
+        manifest_backup = osutils.copy_file(manifest_path, f'{manifest_path}.bak')
+
+        with open(manifest_backup, 'r') as manifest_backup_file:
+            manifest = json.loads(manifest_backup_file.read())
+
+            if 'dependencies' in manifest and dep_name in manifest['dependencies']:
+                dep_rel_path = osutils.relative_path(dependency_tarfile_path, start=package_path)
+                dep_rel_path = osutils.joinpath('.', dep_rel_path).replace(os.sep, "/")
+                manifest['dependencies'][dep_name] = f'file:{dep_rel_path}'
+
+                with open(manifest_path, 'w') as manifest_write_file:
+                    manifest_write_file.write(json.dumps(manifest, indent=4))

From e16d565039efb45977bdd06f0ce7f1e50e061c09 Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Thu, 10 Dec 2020 07:44:54 -0600
Subject: [PATCH 03/24] Removed unused args in utils.mkdir

---
 aws_lambda_builders/workflows/nodejs_npm/utils.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 3ce6340f0..e576682c3 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -36,8 +36,8 @@ def filename(self, filepath):
     def joinpath(self, *args):
         return os.path.join(*args)
 
-    def mkdir(self, path, mode=0o777, *args, dir_fd=None):
-        return os.mkdir(path, mode, *args, dir_fd=dir_fd)
+    def mkdir(self, path):
+        return os.mkdir(path)
 
     def popen(self, command, stdout=None, stderr=None, env=None, cwd=None):
         p = subprocess.Popen(command, stdout=stdout, stderr=stderr, env=env, cwd=cwd)

From d84e5eac8660d9a84d7bdc882f0735da458ad0e2 Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Thu, 10 Dec 2020 08:51:41 -0600
Subject: [PATCH 04/24] Expanded to recursive local modules

---
 .../workflows/nodejs_npm/actions.py           | 10 +++---
 .../workflows/nodejs_npm/utils.py             | 35 +++++++++----------
 2 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index d52a32823..38142ddca 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -75,11 +75,11 @@ def execute(self):
             for (dep_name, dep_path) in local_dependencies.items():
                 dep_scratch_dir = self.osutils.joinpath(self.scratch_dir, str(abs(hash(dep_name))))
                 dep_artifacts_dir = self.osutils.joinpath(dep_scratch_dir, 'unpacked')
-                dependency_tarfile_path = DependencyUtils.package_local_dependency(package_path[5:], dep_path, dep_artifacts_dir, dep_scratch_dir, self.osutils, self.subprocess_npm)
-                local_packaged_dep_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'local_dep')
-                if not self.osutils.dir_exists(local_packaged_dep_path):
-                    self.osutils.mkdir(local_packaged_dep_path)
-                dependency_tarfile_path = self.osutils.copy_file(dependency_tarfile_path, self.osutils.joinpath(local_packaged_dep_path))
+                local_packaged_output_dir = self.osutils.joinpath(self.artifacts_dir, 'package', 'local_dep')
+                if not self.osutils.dir_exists(local_packaged_output_dir):
+                    self.osutils.mkdir(local_packaged_output_dir)
+                dependency_tarfile_path = DependencyUtils.package_local_dependency(package_path[5:], dep_path, dep_artifacts_dir, dep_scratch_dir, local_packaged_output_dir, self.osutils, self.subprocess_npm)
+                dependency_tarfile_path = self.osutils.copy_file(dependency_tarfile_path, local_packaged_output_dir)
                 DependencyUtils.update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, self.osutils)
 
         except NpmExecutionError as ex:
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index e576682c3..ce9da8ca1 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -92,7 +92,7 @@ def is_local_dependency(path):
         except AttributeError:
             return False
 
-    def package_local_dependency(parent_package_path, rel_package_path, artifact_dir, scratch_dir, osutils, subprocess_npm):
+    def package_local_dependency(parent_package_path, rel_package_path, artifacts_dir, scratch_dir, output_dir, osutils, subprocess_npm):
         if rel_package_path.startswith('file:'):
             rel_package_path = rel_package_path[5:].strip()
 
@@ -110,27 +110,24 @@ def package_local_dependency(parent_package_path, rel_package_path, artifact_dir
 
         tarfile_path = osutils.joinpath(scratch_dir, tarfile_name)
 
-        # LOG.debug("NODEJS extracting to %s", artifact_dir)
+        # LOG.debug("NODEJS extracting to %s", artifacts_dir)
 
-        osutils.extract_tarfile(tarfile_path, artifact_dir)
+        osutils.extract_tarfile(tarfile_path, artifacts_dir)
 
         # LOG.debug("NODEJS searching for subpackage local dependencies")
 
-        # local_manifest_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'package.json')
-        # local_dependencies = DependencyUtils.get_local_dependencies(self.manifest_path)
-        # for (dep_name, dep_path) in local_dependencies.items():
-        #     dep_scratch_dir = self.osutils.joinpath(self.scratch_dir, str(abs(hash(dep_name))))
-        #     dep_artifacts_dir = self.osutils.joinpath(dep_scratch_dir, 'unpacked')
-        #     dependency_tarfile_path = DependencyUtils.package_local_dependency(package_path[5:], dep_path, dep_artifacts_dir, dep_scratch_dir, self.osutils, self.subprocess_npm)
-        #     local_packaged_dep_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'local_dep')
-        #     if not self.osutils.dir_exists(local_packaged_dep_path):
-        #         self.osutils.mkdir(local_packaged_dep_path)
-        #     dependency_tarfile_path = self.osutils.copy_file(dependency_tarfile_path, self.osutils.joinpath(local_packaged_dep_path))
-        #     DependencyUtils.update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, self.osutils)
+        local_manifest_path = osutils.joinpath(artifacts_dir, 'package', 'package.json')
+        local_dependencies = DependencyUtils.get_local_dependencies(local_manifest_path)
+        for (dep_name, dep_path) in local_dependencies.items():
+            dep_scratch_dir = osutils.joinpath(scratch_dir, str(abs(hash(dep_name))))
+            dep_artifacts_dir = osutils.joinpath(dep_scratch_dir, 'unpacked')
+            dependency_tarfile_path = DependencyUtils.package_local_dependency(package_path, dep_path, dep_artifacts_dir, dep_scratch_dir, output_dir, osutils, subprocess_npm)
+            dependency_tarfile_path = osutils.copy_file(dependency_tarfile_path, output_dir)
+            DependencyUtils.update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, osutils)
 
-        localized_package_dir = osutils.joinpath(artifact_dir, 'package')
+        localized_package_dir = osutils.joinpath(artifacts_dir, 'package')
 
-        # LOG.debug("NODEJS repackaging dependency %s to %s", artifact_dir, localized_package_dir)
+        # LOG.debug("NODEJS repackaging dependency %s to %s", artifacts_dir, localized_package_dir)
 
         tarfile_name = subprocess_npm.run(["pack", "-q", localized_package_dir], cwd=localized_package_dir).splitlines()[-1]
 
@@ -148,9 +145,9 @@ def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
             manifest = json.loads(manifest_backup_file.read())
 
             if 'dependencies' in manifest and dep_name in manifest['dependencies']:
-                dep_rel_path = osutils.relative_path(dependency_tarfile_path, start=package_path)
-                dep_rel_path = osutils.joinpath('.', dep_rel_path).replace(os.sep, "/")
-                manifest['dependencies'][dep_name] = f'file:{dep_rel_path}'
+                # dep_rel_path = osutils.relative_path(dependency_tarfile_path, start=package_path)
+                # dep_rel_path = osutils.joinpath('.', dep_rel_path).replace(os.sep, "/")
+                manifest['dependencies'][dep_name] = f'file:{dependency_tarfile_path}'
 
                 with open(manifest_path, 'w') as manifest_write_file:
                     manifest_write_file.write(json.dumps(manifest, indent=4))

From 221135a678817870c7519279903478fd0d6c9534 Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Thu, 10 Dec 2020 10:02:02 -0600
Subject: [PATCH 05/24] Updated baseline tests

---
 .../workflows/nodejs_npm/actions.py           |  2 +-
 .../workflows/nodejs_npm/utils.py             | 18 +++++------
 .../unit/workflows/nodejs_npm/test_actions.py | 30 ++++++++++++++++++-
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index 38142ddca..6bb70a464 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -71,7 +71,7 @@ def execute(self):
             LOG.debug("NODEJS searching for local dependencies")
 
             local_manifest_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'package.json')
-            local_dependencies = DependencyUtils.get_local_dependencies(self.manifest_path)
+            local_dependencies = DependencyUtils.get_local_dependencies(self.manifest_path, self.osutils)
             for (dep_name, dep_path) in local_dependencies.items():
                 dep_scratch_dir = self.osutils.joinpath(self.scratch_dir, str(abs(hash(dep_name))))
                 dep_artifacts_dir = self.osutils.joinpath(dep_scratch_dir, 'unpacked')
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index ce9da8ca1..0c06153b5 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -39,6 +39,9 @@ def joinpath(self, *args):
     def mkdir(self, path):
         return os.mkdir(path)
 
+    def open_file(self, filename, mode='r'):
+        return open(filename, mode)
+
     def popen(self, command, stdout=None, stderr=None, env=None, cwd=None):
         p = subprocess.Popen(command, stdout=stdout, stderr=stderr, env=env, cwd=cwd)
         return p
@@ -69,12 +72,12 @@ class DependencyUtils(object):
     Collection of helper functions for managing local NPM dependencies
     """
 
-    def get_local_dependencies(manifest_path):
+    def get_local_dependencies(manifest_path, osutils):
         """
         Helper function to extract all local dependencies in a package.json manifest
         """
 
-        with open(manifest_path) as manifest_file:
+        with osutils.open_file(manifest_path) as manifest_file:
             manifest = json.loads(manifest_file.read())
 
             if 'dependencies' in manifest:
@@ -117,7 +120,7 @@ def package_local_dependency(parent_package_path, rel_package_path, artifacts_di
         # LOG.debug("NODEJS searching for subpackage local dependencies")
 
         local_manifest_path = osutils.joinpath(artifacts_dir, 'package', 'package.json')
-        local_dependencies = DependencyUtils.get_local_dependencies(local_manifest_path)
+        local_dependencies = DependencyUtils.get_local_dependencies(local_manifest_path, osutils)
         for (dep_name, dep_path) in local_dependencies.items():
             dep_scratch_dir = osutils.joinpath(scratch_dir, str(abs(hash(dep_name))))
             dep_artifacts_dir = osutils.joinpath(dep_scratch_dir, 'unpacked')
@@ -138,16 +141,13 @@ def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
         Helper function to update dependency path to localized tar
         """
 
-        package_path = osutils.dirname(manifest_path)
-        manifest_backup = osutils.copy_file(manifest_path, f'{manifest_path}.bak')
+        manifest_backup = osutils.copy_file(manifest_path, '{}.bak'.format(manifest_path))
 
-        with open(manifest_backup, 'r') as manifest_backup_file:
+        with osutils.open_file(manifest_backup, 'r') as manifest_backup_file:
             manifest = json.loads(manifest_backup_file.read())
 
             if 'dependencies' in manifest and dep_name in manifest['dependencies']:
-                # dep_rel_path = osutils.relative_path(dependency_tarfile_path, start=package_path)
-                # dep_rel_path = osutils.joinpath('.', dep_rel_path).replace(os.sep, "/")
                 manifest['dependencies'][dep_name] = f'file:{dependency_tarfile_path}'
 
-                with open(manifest_path, 'w') as manifest_write_file:
+                with osutils.open_file(manifest_path, 'w') as manifest_write_file:
                     manifest_write_file.write(json.dumps(manifest, indent=4))
diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py
index 39007a59c..975d1cac3 100644
--- a/tests/unit/workflows/nodejs_npm/test_actions.py
+++ b/tests/unit/workflows/nodejs_npm/test_actions.py
@@ -11,6 +11,33 @@
 from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError
 
 
+class FakeFileObject(object):
+    def __init__(self, filename, mode='r'):
+        self.filename = filename
+        self.mode = mode
+        self.contents = "{}"
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, type, value, traceback):
+        pass
+
+    def read(self):
+        if self.mode.startswith('r'):
+            return self.contents
+        elif (self.mode.startswith == 'w' or self.mode.startswith == 'a') and self.mode.endswith('+'):
+            return ''
+        else:
+            raise IOError('file not open for reading')
+
+    def write(self, data):
+        if self.mode.startswith('w') or self.mode.startswith('a') or self.mode.endswith('+'):
+            return self.contents
+        else:
+            raise IOError('file not open for writing')
+
+
 class TestNodejsNpmPackAction(TestCase):
     @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils")
     @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm")
@@ -24,7 +51,8 @@ def test_tars_and_unpacks_npm_project(self, OSUtilMock, SubprocessNpmMock):
 
         osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
         osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
-        osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b)
+        osutils.joinpath.side_effect = lambda *args: '/'.join(args)
+        osutils.open_file.side_effect = lambda filename, mode='r': FakeFileObject(filename, mode)
 
         subprocess_npm.run.return_value = "package.tar"
 

From 988a6c47e01f6b519b359f26d48a48608515d367 Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Thu, 10 Dec 2020 10:15:32 -0600
Subject: [PATCH 06/24] black formatting

---
 .../workflows/nodejs_npm/actions.py           | 16 +++++--
 .../workflows/nodejs_npm/utils.py             | 42 +++++++++++--------
 .../unit/workflows/nodejs_npm/test_actions.py | 18 ++++----
 3 files changed, 46 insertions(+), 30 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index 6bb70a464..34d351819 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -70,15 +70,23 @@ def execute(self):
 
             LOG.debug("NODEJS searching for local dependencies")
 
-            local_manifest_path = self.osutils.joinpath(self.artifacts_dir, 'package', 'package.json')
+            local_manifest_path = self.osutils.joinpath(self.artifacts_dir, "package", "package.json")
             local_dependencies = DependencyUtils.get_local_dependencies(self.manifest_path, self.osutils)
             for (dep_name, dep_path) in local_dependencies.items():
                 dep_scratch_dir = self.osutils.joinpath(self.scratch_dir, str(abs(hash(dep_name))))
-                dep_artifacts_dir = self.osutils.joinpath(dep_scratch_dir, 'unpacked')
-                local_packaged_output_dir = self.osutils.joinpath(self.artifacts_dir, 'package', 'local_dep')
+                dep_artifacts_dir = self.osutils.joinpath(dep_scratch_dir, "unpacked")
+                local_packaged_output_dir = self.osutils.joinpath(self.artifacts_dir, "package", "local_dep")
                 if not self.osutils.dir_exists(local_packaged_output_dir):
                     self.osutils.mkdir(local_packaged_output_dir)
-                dependency_tarfile_path = DependencyUtils.package_local_dependency(package_path[5:], dep_path, dep_artifacts_dir, dep_scratch_dir, local_packaged_output_dir, self.osutils, self.subprocess_npm)
+                dependency_tarfile_path = DependencyUtils.package_local_dependency(
+                    package_path[5:],
+                    dep_path,
+                    dep_artifacts_dir,
+                    dep_scratch_dir,
+                    local_packaged_output_dir,
+                    self.osutils,
+                    self.subprocess_npm,
+                )
                 dependency_tarfile_path = self.osutils.copy_file(dependency_tarfile_path, local_packaged_output_dir)
                 DependencyUtils.update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, self.osutils)
 
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 0c06153b5..1211626db 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -39,7 +39,7 @@ def joinpath(self, *args):
     def mkdir(self, path):
         return os.mkdir(path)
 
-    def open_file(self, filename, mode='r'):
+    def open_file(self, filename, mode="r"):
         return open(filename, mode)
 
     def popen(self, command, stdout=None, stderr=None, env=None, cwd=None):
@@ -80,8 +80,10 @@ def get_local_dependencies(manifest_path, osutils):
         with osutils.open_file(manifest_path) as manifest_file:
             manifest = json.loads(manifest_file.read())
 
-            if 'dependencies' in manifest:
-                return dict((k, v) for (k, v) in manifest['dependencies'].items() if DependencyUtils.is_local_dependency(v))
+            if "dependencies" in manifest:
+                return dict(
+                    (k, v) for (k, v) in manifest["dependencies"].items() if DependencyUtils.is_local_dependency(v)
+                )
             else:
                 return {}
 
@@ -91,15 +93,17 @@ def is_local_dependency(path):
         """
 
         try:
-            return path.startswith('file:') or path.startswith('.')
+            return path.startswith("file:") or path.startswith(".")
         except AttributeError:
             return False
 
-    def package_local_dependency(parent_package_path, rel_package_path, artifacts_dir, scratch_dir, output_dir, osutils, subprocess_npm):
-        if rel_package_path.startswith('file:'):
+    def package_local_dependency(
+        parent_package_path, rel_package_path, artifacts_dir, scratch_dir, output_dir, osutils, subprocess_npm
+    ):
+        if rel_package_path.startswith("file:"):
             rel_package_path = rel_package_path[5:].strip()
 
-        if rel_package_path.startswith('.'):
+        if rel_package_path.startswith("."):
             package_path = osutils.abspath(osutils.joinpath(parent_package_path, rel_package_path))
 
         # LOG.debug("NODEJS packaging dependency %s to %s", package_path, scratch_dir)
@@ -119,20 +123,24 @@ def package_local_dependency(parent_package_path, rel_package_path, artifacts_di
 
         # LOG.debug("NODEJS searching for subpackage local dependencies")
 
-        local_manifest_path = osutils.joinpath(artifacts_dir, 'package', 'package.json')
+        local_manifest_path = osutils.joinpath(artifacts_dir, "package", "package.json")
         local_dependencies = DependencyUtils.get_local_dependencies(local_manifest_path, osutils)
         for (dep_name, dep_path) in local_dependencies.items():
             dep_scratch_dir = osutils.joinpath(scratch_dir, str(abs(hash(dep_name))))
-            dep_artifacts_dir = osutils.joinpath(dep_scratch_dir, 'unpacked')
-            dependency_tarfile_path = DependencyUtils.package_local_dependency(package_path, dep_path, dep_artifacts_dir, dep_scratch_dir, output_dir, osutils, subprocess_npm)
+            dep_artifacts_dir = osutils.joinpath(dep_scratch_dir, "unpacked")
+            dependency_tarfile_path = DependencyUtils.package_local_dependency(
+                package_path, dep_path, dep_artifacts_dir, dep_scratch_dir, output_dir, osutils, subprocess_npm
+            )
             dependency_tarfile_path = osutils.copy_file(dependency_tarfile_path, output_dir)
             DependencyUtils.update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, osutils)
 
-        localized_package_dir = osutils.joinpath(artifacts_dir, 'package')
+        localized_package_dir = osutils.joinpath(artifacts_dir, "package")
 
         # LOG.debug("NODEJS repackaging dependency %s to %s", artifacts_dir, localized_package_dir)
 
-        tarfile_name = subprocess_npm.run(["pack", "-q", localized_package_dir], cwd=localized_package_dir).splitlines()[-1]
+        tarfile_name = subprocess_npm.run(
+            ["pack", "-q", localized_package_dir], cwd=localized_package_dir
+        ).splitlines()[-1]
 
         return osutils.joinpath(localized_package_dir, tarfile_name)
 
@@ -141,13 +149,13 @@ def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
         Helper function to update dependency path to localized tar
         """
 
-        manifest_backup = osutils.copy_file(manifest_path, '{}.bak'.format(manifest_path))
+        manifest_backup = osutils.copy_file(manifest_path, "{}.bak".format(manifest_path))
 
-        with osutils.open_file(manifest_backup, 'r') as manifest_backup_file:
+        with osutils.open_file(manifest_backup, "r") as manifest_backup_file:
             manifest = json.loads(manifest_backup_file.read())
 
-            if 'dependencies' in manifest and dep_name in manifest['dependencies']:
-                manifest['dependencies'][dep_name] = f'file:{dependency_tarfile_path}'
+            if "dependencies" in manifest and dep_name in manifest["dependencies"]:
+                manifest["dependencies"][dep_name] = f"file:{dependency_tarfile_path}"
 
-                with osutils.open_file(manifest_path, 'w') as manifest_write_file:
+                with osutils.open_file(manifest_path, "w") as manifest_write_file:
                     manifest_write_file.write(json.dumps(manifest, indent=4))
diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py
index 975d1cac3..baa6adcb3 100644
--- a/tests/unit/workflows/nodejs_npm/test_actions.py
+++ b/tests/unit/workflows/nodejs_npm/test_actions.py
@@ -12,7 +12,7 @@
 
 
 class FakeFileObject(object):
-    def __init__(self, filename, mode='r'):
+    def __init__(self, filename, mode="r"):
         self.filename = filename
         self.mode = mode
         self.contents = "{}"
@@ -24,18 +24,18 @@ def __exit__(self, type, value, traceback):
         pass
 
     def read(self):
-        if self.mode.startswith('r'):
+        if self.mode.startswith("r"):
             return self.contents
-        elif (self.mode.startswith == 'w' or self.mode.startswith == 'a') and self.mode.endswith('+'):
-            return ''
+        elif (self.mode.startswith == "w" or self.mode.startswith == "a") and self.mode.endswith("+"):
+            return ""
         else:
-            raise IOError('file not open for reading')
+            raise IOError("file not open for reading")
 
     def write(self, data):
-        if self.mode.startswith('w') or self.mode.startswith('a') or self.mode.endswith('+'):
+        if self.mode.startswith("w") or self.mode.startswith("a") or self.mode.endswith("+"):
             return self.contents
         else:
-            raise IOError('file not open for writing')
+            raise IOError("file not open for writing")
 
 
 class TestNodejsNpmPackAction(TestCase):
@@ -51,8 +51,8 @@ def test_tars_and_unpacks_npm_project(self, OSUtilMock, SubprocessNpmMock):
 
         osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
         osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
-        osutils.joinpath.side_effect = lambda *args: '/'.join(args)
-        osutils.open_file.side_effect = lambda filename, mode='r': FakeFileObject(filename, mode)
+        osutils.joinpath.side_effect = lambda *args: "/".join(args)
+        osutils.open_file.side_effect = lambda filename, mode="r": FakeFileObject(filename, mode)
 
         subprocess_npm.run.return_value = "package.tar"
 

From 7697c3bd4ebea9788dbf1476afc73a9bc4319872 Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Thu, 10 Dec 2020 11:14:36 -0600
Subject: [PATCH 07/24] Added basic integration test for local deps

---
 .../workflows/nodejs_npm/actions.py           |  2 +-
 .../workflows/nodejs_npm/utils.py             | 10 ++++----
 .../workflows/nodejs_npm/test_nodejs_npm.py   | 23 +++++++++++++++++++
 .../testdata/local-deps/excluded.js           |  2 ++
 .../testdata/local-deps/included.js           |  6 +++++
 .../testdata/local-deps/package.json          | 12 ++++++++++
 .../testdata/modules/module_a/index.js        |  5 ++++
 .../testdata/modules/module_a/package.json    | 12 ++++++++++
 .../testdata/modules/module_b/index.js        |  5 ++++
 .../testdata/modules/module_b/package.json    | 12 ++++++++++
 .../testdata/modules/module_c/index.js        |  3 +++
 .../testdata/modules/module_c/package.json    |  9 ++++++++
 12 files changed, 96 insertions(+), 5 deletions(-)
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/local-deps/excluded.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/local-deps/included.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/local-deps/package.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/modules/module_a/index.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/modules/module_a/package.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/modules/module_b/index.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/modules/module_b/package.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/modules/module_c/index.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/modules/module_c/package.json

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index 34d351819..dbc5c6a70 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -75,7 +75,7 @@ def execute(self):
             for (dep_name, dep_path) in local_dependencies.items():
                 dep_scratch_dir = self.osutils.joinpath(self.scratch_dir, str(abs(hash(dep_name))))
                 dep_artifacts_dir = self.osutils.joinpath(dep_scratch_dir, "unpacked")
-                local_packaged_output_dir = self.osutils.joinpath(self.artifacts_dir, "package", "local_dep")
+                local_packaged_output_dir = self.osutils.joinpath(self.artifacts_dir, "@aws_lambda_builders_local_dep")
                 if not self.osutils.dir_exists(local_packaged_output_dir):
                     self.osutils.mkdir(local_packaged_output_dir)
                 dependency_tarfile_path = DependencyUtils.package_local_dependency(
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 1211626db..8267acdc8 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -30,8 +30,8 @@ def extract_tarfile(self, tarfile_path, unpack_dir):
     def file_exists(self, filename):
         return os.path.isfile(filename)
 
-    def filename(self, filepath):
-        return os.path.basename(filepath)
+    # def filename(self, filepath):
+    #     return os.path.basename(filepath)
 
     def joinpath(self, *args):
         return os.path.join(*args)
@@ -53,8 +53,8 @@ def pipe(self):
     def dirname(self, path):
         return os.path.dirname(path)
 
-    def relative_path(self, path, start):
-        return os.path.relpath(path, start=start)
+    # def relative_path(self, path, start):
+    #     return os.path.relpath(path, start=start)
 
     def remove_file(self, filename):
         return os.remove(filename)
@@ -159,3 +159,5 @@ def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
 
                 with osutils.open_file(manifest_path, "w") as manifest_write_file:
                     manifest_write_file.write(json.dumps(manifest, indent=4))
+
+        osutils.remove_file(manifest_backup)
diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
index 83a8de6e0..42d792f44 100644
--- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
+++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
@@ -77,6 +77,29 @@ def test_builds_project_with_remote_dependencies(self):
         output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules")))
         self.assertEqual(expected_modules, output_modules)
 
+    def test_builds_project_with_local_dependencies(self):
+        source_dir = os.path.join(self.TEST_DATA_FOLDER, "local-deps")
+
+        self.builder.build(
+            source_dir,
+            self.artifacts_dir,
+            self.scratch_dir,
+            os.path.join(source_dir, "package.json"),
+            runtime=self.runtime,
+        )
+
+        expected_files = {"package.json", "included.js", "node_modules"}
+        output_files = set(os.listdir(self.artifacts_dir))
+        self.assertEqual(expected_files, output_files)
+
+        expected_modules = {"@mockcompany"}
+        output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules")))
+        self.assertEqual(expected_modules, output_modules)
+
+        expected_sub_modules = {"module-a", "module-b", "module-c"}
+        output_sub_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules", "@mockcompany")))
+        self.assertEqual(expected_sub_modules, output_sub_modules)
+
     def test_builds_project_with_npmrc(self):
         source_dir = os.path.join(self.TEST_DATA_FOLDER, "npmrc")
 
diff --git a/tests/integration/workflows/nodejs_npm/testdata/local-deps/excluded.js b/tests/integration/workflows/nodejs_npm/testdata/local-deps/excluded.js
new file mode 100644
index 000000000..8bf8be437
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/local-deps/excluded.js
@@ -0,0 +1,2 @@
+//excluded
+const x = 1;
diff --git a/tests/integration/workflows/nodejs_npm/testdata/local-deps/included.js b/tests/integration/workflows/nodejs_npm/testdata/local-deps/included.js
new file mode 100644
index 000000000..872ecacbb
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/local-deps/included.js
@@ -0,0 +1,6 @@
+const module_a = require('@mockcompany/module-a')
+
+//included
+const x = 1;
+
+const greetings = module_a.sayHello();
diff --git a/tests/integration/workflows/nodejs_npm/testdata/local-deps/package.json b/tests/integration/workflows/nodejs_npm/testdata/local-deps/package.json
new file mode 100644
index 000000000..47df45240
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/local-deps/package.json
@@ -0,0 +1,12 @@
+{
+  "name": "localdeps",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["included.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0",
+  "dependencies": {
+    "@mockcompany/module-a": "file:../modules/module_a" 
+  }
+}
diff --git a/tests/integration/workflows/nodejs_npm/testdata/modules/module_a/index.js b/tests/integration/workflows/nodejs_npm/testdata/modules/module_a/index.js
new file mode 100644
index 000000000..5594a5346
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/modules/module_a/index.js
@@ -0,0 +1,5 @@
+const module_b = require('@mockcompany/module-b');
+
+exports.sayHello = function() {
+  return 'hello from module a! module b says: ' + module_b.sayHello();
+}
\ No newline at end of file
diff --git a/tests/integration/workflows/nodejs_npm/testdata/modules/module_a/package.json b/tests/integration/workflows/nodejs_npm/testdata/modules/module_a/package.json
new file mode 100644
index 000000000..84952331b
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/modules/module_a/package.json
@@ -0,0 +1,12 @@
+{
+  "name": "@mockcompany/module-a",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["index.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0",
+  "dependencies": {
+    "@mockcompany/module-b": "file:../module_b" 
+  }
+}
\ No newline at end of file
diff --git a/tests/integration/workflows/nodejs_npm/testdata/modules/module_b/index.js b/tests/integration/workflows/nodejs_npm/testdata/modules/module_b/index.js
new file mode 100644
index 000000000..e99abb1ac
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/modules/module_b/index.js
@@ -0,0 +1,5 @@
+const module_c = require('@mockcompany/module-c');
+
+exports.sayHello = function() {
+  return 'hello from module b! module-c says: ' + module_c.sayHello();
+}
\ No newline at end of file
diff --git a/tests/integration/workflows/nodejs_npm/testdata/modules/module_b/package.json b/tests/integration/workflows/nodejs_npm/testdata/modules/module_b/package.json
new file mode 100644
index 000000000..1868f25c4
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/modules/module_b/package.json
@@ -0,0 +1,12 @@
+{
+  "name": "@mockcompany/module-b",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["index.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0",
+  "dependencies": {
+    "@mockcompany/module-c": "file:../module_c" 
+  }
+}
\ No newline at end of file
diff --git a/tests/integration/workflows/nodejs_npm/testdata/modules/module_c/index.js b/tests/integration/workflows/nodejs_npm/testdata/modules/module_c/index.js
new file mode 100644
index 000000000..9162b06fb
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/modules/module_c/index.js
@@ -0,0 +1,3 @@
+exports.sayHello = function() {
+  return 'hello from module c! I have no news... :(';
+}
\ No newline at end of file
diff --git a/tests/integration/workflows/nodejs_npm/testdata/modules/module_c/package.json b/tests/integration/workflows/nodejs_npm/testdata/modules/module_c/package.json
new file mode 100644
index 000000000..d20ca19ec
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/modules/module_c/package.json
@@ -0,0 +1,9 @@
+{
+  "name": "@mockcompany/module-c",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["index.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0"
+}
\ No newline at end of file

From 49374dd44bec5e01029733d799ed623b2d28949b Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Thu, 10 Dec 2020 12:24:10 -0600
Subject: [PATCH 08/24] Refactored common to single function

---
 .../workflows/nodejs_npm/actions.py           | 42 +++++++-------
 .../workflows/nodejs_npm/utils.py             | 55 +++++++++++++------
 .../workflows/nodejs_npm/workflow.py          |  7 ++-
 3 files changed, 66 insertions(+), 38 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index dbc5c6a70..e4175ff3c 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -9,7 +9,6 @@
 from .utils import DependencyUtils
 
 LOG = logging.getLogger(__name__)
-LOG.setLevel(logging.DEBUG)
 
 
 class NodejsNpmPackAction(BaseAction):
@@ -68,29 +67,30 @@ def execute(self):
 
             self.osutils.extract_tarfile(tarfile_path, self.artifacts_dir)
 
+            self._package_local_dependencies()
+
+        except NpmExecutionError as ex:
+            raise ActionFailedError(str(ex))
+
+    def _package_local_dependencies(self):
+        """
+        Iterates (recursively) through any local dependencies defined in package.json.
+
+        :raises lambda_builders.actions.ActionFailedError: when any file system operations fail
+        """
+
+        try:
             LOG.debug("NODEJS searching for local dependencies")
 
-            local_manifest_path = self.osutils.joinpath(self.artifacts_dir, "package", "package.json")
-            local_dependencies = DependencyUtils.get_local_dependencies(self.manifest_path, self.osutils)
-            for (dep_name, dep_path) in local_dependencies.items():
-                dep_scratch_dir = self.osutils.joinpath(self.scratch_dir, str(abs(hash(dep_name))))
-                dep_artifacts_dir = self.osutils.joinpath(dep_scratch_dir, "unpacked")
-                local_packaged_output_dir = self.osutils.joinpath(self.artifacts_dir, "@aws_lambda_builders_local_dep")
-                if not self.osutils.dir_exists(local_packaged_output_dir):
-                    self.osutils.mkdir(local_packaged_output_dir)
-                dependency_tarfile_path = DependencyUtils.package_local_dependency(
-                    package_path[5:],
-                    dep_path,
-                    dep_artifacts_dir,
-                    dep_scratch_dir,
-                    local_packaged_output_dir,
-                    self.osutils,
-                    self.subprocess_npm,
-                )
-                dependency_tarfile_path = self.osutils.copy_file(dependency_tarfile_path, local_packaged_output_dir)
-                DependencyUtils.update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, self.osutils)
+            parent_package_path = self.osutils.abspath(self.osutils.dirname(self.manifest_path))
 
-        except NpmExecutionError as ex:
+            DependencyUtils.package_local_dependency(
+                parent_package_path, ".", self.artifacts_dir, self.scratch_dir, None, self.osutils, self.subprocess_npm
+            )
+
+            LOG.debug("NODEJS finished processing local dependencies")
+
+        except OSError as ex:
             raise ActionFailedError(str(ex))
 
 
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 8267acdc8..0b8390925 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -3,12 +3,15 @@
 """
 
 import json
+import logging
 import os
 import platform
 import tarfile
 import subprocess
 import shutil
 
+LOG = logging.getLogger(__name__)
+
 
 class OSUtils(object):
 
@@ -100,49 +103,69 @@ def is_local_dependency(path):
     def package_local_dependency(
         parent_package_path, rel_package_path, artifacts_dir, scratch_dir, output_dir, osutils, subprocess_npm
     ):
+        """
+        Helper function to recurse local dependencies and package them to a common directory
+        """
+
         if rel_package_path.startswith("file:"):
             rel_package_path = rel_package_path[5:].strip()
 
         if rel_package_path.startswith("."):
             package_path = osutils.abspath(osutils.joinpath(parent_package_path, rel_package_path))
-
-        # LOG.debug("NODEJS packaging dependency %s to %s", package_path, scratch_dir)
+        else:
+            package_path = rel_package_path
 
         if not osutils.dir_exists(scratch_dir):
             osutils.mkdir(scratch_dir)
 
-        tarfile_name = subprocess_npm.run(["pack", "-q", package_path], cwd=scratch_dir).splitlines()[-1]
-
-        # LOG.debug("NODEJS packed dependency to %s", tarfile_name)
-
-        tarfile_path = osutils.joinpath(scratch_dir, tarfile_name)
+        if output_dir is None:
+            # TODO: get a higher level output_dir to keep process locals between jobs
+            output_dir = osutils.joinpath(artifacts_dir, "@aws_lambda_builders_local_dep")
+            if not osutils.dir_exists(output_dir):
+                osutils.mkdir(output_dir)
+            top_level = True
+        else:
+            tarfile_name = subprocess_npm.run(["pack", "-q", package_path], cwd=scratch_dir).splitlines()[-1]
+            tarfile_path = osutils.joinpath(scratch_dir, tarfile_name)
 
-        # LOG.debug("NODEJS extracting to %s", artifacts_dir)
+            LOG.debug("NODEJS extracting child dependency for recursive dependency check")
 
-        osutils.extract_tarfile(tarfile_path, artifacts_dir)
+            osutils.extract_tarfile(tarfile_path, artifacts_dir)
 
-        # LOG.debug("NODEJS searching for subpackage local dependencies")
+            top_level = False
 
         local_manifest_path = osutils.joinpath(artifacts_dir, "package", "package.json")
         local_dependencies = DependencyUtils.get_local_dependencies(local_manifest_path, osutils)
         for (dep_name, dep_path) in local_dependencies.items():
             dep_scratch_dir = osutils.joinpath(scratch_dir, str(abs(hash(dep_name))))
+
+            # TODO: if dep_scratch_dir exists, it means we've already processed it this round, skip
+
             dep_artifacts_dir = osutils.joinpath(dep_scratch_dir, "unpacked")
+
+            LOG.debug("NODEJS packaging dependency, %s, from %s to %s", dep_name, parent_package_path, output_dir)
+
             dependency_tarfile_path = DependencyUtils.package_local_dependency(
                 package_path, dep_path, dep_artifacts_dir, dep_scratch_dir, output_dir, osutils, subprocess_npm
             )
             dependency_tarfile_path = osutils.copy_file(dependency_tarfile_path, output_dir)
+
+            LOG.debug("NODEJS packed localized child dependency to %s", dependency_tarfile_path)
+
+            LOG.debug("NODEJS updating package.json %s", local_manifest_path)
+
             DependencyUtils.update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, osutils)
 
-        localized_package_dir = osutils.joinpath(artifacts_dir, "package")
+        if not top_level:
+            localized_package_dir = osutils.joinpath(artifacts_dir, "package")
 
-        # LOG.debug("NODEJS repackaging dependency %s to %s", artifacts_dir, localized_package_dir)
+            LOG.debug("NODEJS repackaging child dependency")
 
-        tarfile_name = subprocess_npm.run(
-            ["pack", "-q", localized_package_dir], cwd=localized_package_dir
-        ).splitlines()[-1]
+            tarfile_name = subprocess_npm.run(
+                ["pack", "-q", localized_package_dir], cwd=localized_package_dir
+            ).splitlines()[-1]
 
-        return osutils.joinpath(localized_package_dir, tarfile_name)
+            return osutils.joinpath(localized_package_dir, tarfile_name)
 
     def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
         """
diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py
index b48ea4430..b66cf5162 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py
@@ -4,7 +4,12 @@
 from aws_lambda_builders.path_resolver import PathResolver
 from aws_lambda_builders.workflow import BaseWorkflow, Capability
 from aws_lambda_builders.actions import CopySourceAction
-from .actions import NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction
+from .actions import (
+    NodejsNpmPackAction,
+    NodejsNpmInstallAction,
+    NodejsNpmrcCopyAction,
+    NodejsNpmrcCleanUpAction,
+)
 from .utils import OSUtils
 from .npm import SubprocessNpm
 

From e378384cd45b10943b53ea8068b08a8524faab9a Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Thu, 10 Dec 2020 12:34:45 -0600
Subject: [PATCH 09/24] Python2: updated f"{sfuff}" to "{}".format(stuff)

---
 aws_lambda_builders/workflows/nodejs_npm/utils.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 0b8390925..0843a2ed6 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -178,7 +178,7 @@ def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
             manifest = json.loads(manifest_backup_file.read())
 
             if "dependencies" in manifest and dep_name in manifest["dependencies"]:
-                manifest["dependencies"][dep_name] = f"file:{dependency_tarfile_path}"
+                manifest["dependencies"][dep_name] = "file:{}".format(dependency_tarfile_path)
 
                 with osutils.open_file(manifest_path, "w") as manifest_write_file:
                     manifest_write_file.write(json.dumps(manifest, indent=4))

From b98b533231b4115e94cc4cafd9394e3531bf7a7b Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Thu, 10 Dec 2020 14:22:35 -0600
Subject: [PATCH 10/24] Updated tests

---
 .../workflows/nodejs_npm/utils.py             | 17 ++----
 .../workflows/nodejs_npm/test_utils.py        | 54 +++++++++++++++++
 .../unit/workflows/nodejs_npm/test_actions.py | 59 +++++++++++++++++--
 3 files changed, 114 insertions(+), 16 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 0843a2ed6..768b5e4bb 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -33,9 +33,6 @@ def extract_tarfile(self, tarfile_path, unpack_dir):
     def file_exists(self, filename):
         return os.path.isfile(filename)
 
-    # def filename(self, filepath):
-    #     return os.path.basename(filepath)
-
     def joinpath(self, *args):
         return os.path.join(*args)
 
@@ -56,9 +53,6 @@ def pipe(self):
     def dirname(self, path):
         return os.path.dirname(path)
 
-    # def relative_path(self, path, start):
-    #     return os.path.relpath(path, start=start)
-
     def remove_file(self, filename):
         return os.remove(filename)
 
@@ -75,6 +69,7 @@ class DependencyUtils(object):
     Collection of helper functions for managing local NPM dependencies
     """
 
+    @staticmethod
     def get_local_dependencies(manifest_path, osutils):
         """
         Helper function to extract all local dependencies in a package.json manifest
@@ -90,6 +85,7 @@ def get_local_dependencies(manifest_path, osutils):
             else:
                 return {}
 
+    @staticmethod
     def is_local_dependency(path):
         """
         Helper function to check if package dependency is a local package
@@ -100,6 +96,7 @@ def is_local_dependency(path):
         except AttributeError:
             return False
 
+    @staticmethod
     def package_local_dependency(
         parent_package_path, rel_package_path, artifacts_dir, scratch_dir, output_dir, osutils, subprocess_npm
     ):
@@ -110,10 +107,7 @@ def package_local_dependency(
         if rel_package_path.startswith("file:"):
             rel_package_path = rel_package_path[5:].strip()
 
-        if rel_package_path.startswith("."):
-            package_path = osutils.abspath(osutils.joinpath(parent_package_path, rel_package_path))
-        else:
-            package_path = rel_package_path
+        package_path = osutils.abspath(osutils.joinpath(parent_package_path, rel_package_path))
 
         if not osutils.dir_exists(scratch_dir):
             osutils.mkdir(scratch_dir)
@@ -139,7 +133,7 @@ def package_local_dependency(
         for (dep_name, dep_path) in local_dependencies.items():
             dep_scratch_dir = osutils.joinpath(scratch_dir, str(abs(hash(dep_name))))
 
-            # TODO: if dep_scratch_dir exists, it means we've already processed it this round, skip
+            # TODO: if dep_scratch_dir exists (anywhere up path), it means we've already processed it this round, skip
 
             dep_artifacts_dir = osutils.joinpath(dep_scratch_dir, "unpacked")
 
@@ -167,6 +161,7 @@ def package_local_dependency(
 
             return osutils.joinpath(localized_package_dir, tarfile_name)
 
+    @staticmethod
     def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
         """
         Helper function to update dependency path to localized tar
diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py
index bd39e0ff3..7eaeae811 100644
--- a/tests/functional/workflows/nodejs_npm/test_utils.py
+++ b/tests/functional/workflows/nodejs_npm/test_utils.py
@@ -123,3 +123,57 @@ def test_popen_can_accept_cwd(self):
         self.assertEqual(p.returncode, 0)
 
         self.assertEqual(out.decode("utf8").strip(), os.path.abspath(testdata_dir))
+
+    def test_dir_exists(self):
+        self.assertFalse(self.osutils.dir_exists("20201210_some_directory_that_should_not_exist"))
+
+        with tempfile.TemporaryDirectory() as temp_dir:
+            self.assertTrue(self.osutils.dir_exists(temp_dir))
+
+    def test_mkdir_makes_directory(self):
+        dir_to_create = os.path.join(
+            tempfile._get_default_tempdir(), next(tempfile._get_candidate_names())
+        )  # pylint: disable=protected-access
+        self.assertFalse(os.path.isdir(dir_to_create))
+
+        self.osutils.mkdir(dir_to_create)
+
+        self.assertTrue(os.path.isdir(dir_to_create))
+
+    def test_open_file_opens_file_for_reading(self):
+        with tempfile.TemporaryDirectory() as temp_dir:
+            file_to_open = os.path.join(temp_dir, "test_open.txt")
+
+            with open(file_to_open, "w") as fid:
+                fid.write("this is text")
+
+            with self.osutils.open_file(file_to_open) as fid:
+                content = fid.read()
+
+            self.assertEqual("this is text", content)
+
+    def test_open_file_opens_file_for_writing(self):
+        with tempfile.TemporaryDirectory() as temp_dir:
+            file_to_open = os.path.join(temp_dir, "test_open.txt")
+
+            with self.osutils.open_file(file_to_open, "w") as fid:
+                fid.write("this is some other text")
+
+            with self.osutils.open_file(file_to_open) as fid:
+                content = fid.read()
+
+            self.assertEqual("this is some other text", content)
+
+
+class TestDependencyUtils(TestCase):
+    def test_is_local_dependency_file_prefix(self):
+        self.assertTrue(utils.DependencyUtils.is_local_dependency("file:./local/dep"))
+
+    def test_is_local_dependency_dot_prefix(self):
+        self.assertTrue(utils.DependencyUtils.is_local_dependency("./local/dep"))
+
+    def test_is_local_dependency_package_name(self):
+        self.assertFalse(utils.DependencyUtils.is_local_dependency("typescript"))
+
+    def test_is_local_dependency_invalid(self):
+        self.assertFalse(utils.DependencyUtils.is_local_dependency(None))
diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py
index baa6adcb3..879b6f66d 100644
--- a/tests/unit/workflows/nodejs_npm/test_actions.py
+++ b/tests/unit/workflows/nodejs_npm/test_actions.py
@@ -1,3 +1,4 @@
+from json import dumps
 from unittest import TestCase
 from mock import patch
 
@@ -11,11 +12,30 @@
 from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError
 
 
+class MockOpener:
+    calls = -1
+
+    @property
+    def content(self):
+        return self._contents[min(self.calls, len(self._contents) - 1)]
+
+    def __init__(self, contents=None):
+        if contents is None:
+            self._contents = ["{}"]
+        else:
+            self._contents = contents
+
+    def open(self, filename, mode="r"):
+        self.calls += 1
+        return FakeFileObject(filename, mode, self.content)
+
+
 class FakeFileObject(object):
-    def __init__(self, filename, mode="r"):
+    def __init__(self, filename, mode="r", content="{}"):
         self.filename = filename
         self.mode = mode
-        self.contents = "{}"
+
+        self.content = content
 
     def __enter__(self):
         return self
@@ -25,7 +45,7 @@ def __exit__(self, type, value, traceback):
 
     def read(self):
         if self.mode.startswith("r"):
-            return self.contents
+            return self.content
         elif (self.mode.startswith == "w" or self.mode.startswith == "a") and self.mode.endswith("+"):
             return ""
         else:
@@ -33,7 +53,7 @@ def read(self):
 
     def write(self, data):
         if self.mode.startswith("w") or self.mode.startswith("a") or self.mode.endswith("+"):
-            return self.contents
+            return self.content
         else:
             raise IOError("file not open for writing")
 
@@ -52,7 +72,7 @@ def test_tars_and_unpacks_npm_project(self, OSUtilMock, SubprocessNpmMock):
         osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
         osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
         osutils.joinpath.side_effect = lambda *args: "/".join(args)
-        osutils.open_file.side_effect = lambda filename, mode="r": FakeFileObject(filename, mode)
+        osutils.open_file.side_effect = MockOpener().open
 
         subprocess_npm.run.return_value = "package.tar"
 
@@ -79,6 +99,35 @@ def test_raises_action_failed_when_npm_fails(self, OSUtilMock, SubprocessNpmMock
 
         self.assertEqual(raised.exception.args[0], "NPM Failed: boom!")
 
+    @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils")
+    @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm")
+    def test_tars_and_unpacks_local_dependencies(self, OSUtilMock, SubprocessNpmMock):
+        osutils = OSUtilMock.return_value
+        subprocess_npm = SubprocessNpmMock.return_value
+
+        action = NodejsNpmPackAction(
+            "artifacts", "scratch_dir", "manifest", osutils=osutils, subprocess_npm=subprocess_npm
+        )
+
+        file_open_responses = [
+            dumps({"dependencies": {"dep_1": "file:./local/path"}}),
+            dumps({"dependencies": {"dep_2": "file:local/path"}}),
+            "{}",
+        ]
+
+        osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
+        osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
+        osutils.joinpath.side_effect = lambda *args: "/".join(args)
+        osutils.open_file.side_effect = MockOpener(file_open_responses).open
+
+        subprocess_npm.run.return_value = "package.tar"
+
+        action.execute()
+
+        # pack is called once for top level package, then twice for each local dependency
+        self.assertEqual(subprocess_npm.run.call_count, 1 + 2 * 2)
+        subprocess_npm.run.assert_any_call(["pack", "-q", "file:/abs:/dir:manifest"], cwd="scratch_dir")
+
 
 class TestNodejsNpmInstallAction(TestCase):
     @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm")

From f5d1a2fc3c2f67ae45c743ca0ee6479a8416ca98 Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Thu, 10 Dec 2020 14:25:18 -0600
Subject: [PATCH 11/24] updated npmjs DESIGN.md

---
 aws_lambda_builders/workflows/nodejs_npm/DESIGN.md | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md
index 803052ed7..db3b6f0e6 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md
+++ b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md
@@ -92,8 +92,6 @@ files.
 
 #### Step 2: Rewrite local dependencies
 
-_(out of scope for the current version)_
-
 To optimise disk space and avoid including development dependencies from other
 locally linked packages, inspect the `package.json` manifest looking for dependencies
 referring to local file paths (can be identified as they start with `.` or `file:`),
@@ -102,8 +100,13 @@ then for each dependency recursively execute the packaging process
 Local dependencies may include other local dependencies themselves, this is a very 
 common way of sharing configuration or development utilities such as linting or testing 
 tools. This means that for each packaged local dependency this packager needs to
-recursively apply the packaging process. It also means that the packager needs to 
-track local paths and avoid re-packaging directories it already visited.
+recursively apply the packaging process.
+
+_(out of scope for the current version)_
+
+It also means that the packager needs to track local paths and avoid re-packaging directories it already visited.
+
+_(out of scope for the current version)_
 
 NPM produces a `tar` archive while packaging that can be directly included as a
 dependency.  This will make NPM unpack and install a copy correctly. Once the
@@ -113,7 +116,6 @@ the manifest to point to `tar` files instead of the original location.
 If the project contains a package lock file, this will cause NPM to ignore changes
 to the package.json manifest. In this case, the packager will need to remove 
 `package-lock.json` so that dependency rewrites take effect. 
-_(out of scope for the current version)_
 
 #### Step 3: Install dependencies
 

From 48ff701098f97e5c4d0985347dc2f53bed4f00bd Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Thu, 10 Dec 2020 16:02:01 -0600
Subject: [PATCH 12/24] Python2 regression.

---
 .../workflows/nodejs_npm/utils.py             | 14 ++++--
 .../workflows/nodejs_npm/test_utils.py        | 47 +++++++++++--------
 .../unit/workflows/nodejs_npm/test_actions.py |  3 ++
 3 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 768b5e4bb..fe6ca1d4b 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -33,6 +33,9 @@ def extract_tarfile(self, tarfile_path, unpack_dir):
     def file_exists(self, filename):
         return os.path.isfile(filename)
 
+    def filename(self, filename):
+        return os.path.basename(filename)
+
     def joinpath(self, *args):
         return os.path.join(*args)
 
@@ -142,13 +145,15 @@ def package_local_dependency(
             dependency_tarfile_path = DependencyUtils.package_local_dependency(
                 package_path, dep_path, dep_artifacts_dir, dep_scratch_dir, output_dir, osutils, subprocess_npm
             )
-            dependency_tarfile_path = osutils.copy_file(dependency_tarfile_path, output_dir)
 
-            LOG.debug("NODEJS packed localized child dependency to %s", dependency_tarfile_path)
+            packaged_dependency_tarfile_path = osutils.joinpath(output_dir, osutils.filename(dependency_tarfile_path))
+            osutils.copy_file(dependency_tarfile_path, output_dir)
+
+            LOG.debug("NODEJS packed localized child dependency to %s", packaged_dependency_tarfile_path)
 
             LOG.debug("NODEJS updating package.json %s", local_manifest_path)
 
-            DependencyUtils.update_manifest(local_manifest_path, dep_name, dependency_tarfile_path, osutils)
+            DependencyUtils.update_manifest(local_manifest_path, dep_name, packaged_dependency_tarfile_path, osutils)
 
         if not top_level:
             localized_package_dir = osutils.joinpath(artifacts_dir, "package")
@@ -167,7 +172,8 @@ def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
         Helper function to update dependency path to localized tar
         """
 
-        manifest_backup = osutils.copy_file(manifest_path, "{}.bak".format(manifest_path))
+        manifest_backup = "{}.bak".format(manifest_path)
+        osutils.copy_file(manifest_path, manifest_backup)
 
         with osutils.open_file(manifest_backup, "r") as manifest_backup_file:
             manifest = json.loads(manifest_backup_file.read())
diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py
index 7eaeae811..ae6ee6524 100644
--- a/tests/functional/workflows/nodejs_npm/test_utils.py
+++ b/tests/functional/workflows/nodejs_npm/test_utils.py
@@ -127,42 +127,51 @@ def test_popen_can_accept_cwd(self):
     def test_dir_exists(self):
         self.assertFalse(self.osutils.dir_exists("20201210_some_directory_that_should_not_exist"))
 
-        with tempfile.TemporaryDirectory() as temp_dir:
-            self.assertTrue(self.osutils.dir_exists(temp_dir))
+        temp_dir = tempfile.mkdtemp()
+
+        self.assertTrue(self.osutils.dir_exists(temp_dir))
+
+        shutil.rmtree(temp_dir)
 
     def test_mkdir_makes_directory(self):
-        dir_to_create = os.path.join(
-            tempfile._get_default_tempdir(), next(tempfile._get_candidate_names())
-        )  # pylint: disable=protected-access
+        dir_to_create = os.path.join(tempfile.gettempdir(), "20201210_some_directory_that_should_not_exist")
         self.assertFalse(os.path.isdir(dir_to_create))
 
         self.osutils.mkdir(dir_to_create)
 
         self.assertTrue(os.path.isdir(dir_to_create))
 
+        shutil.rmtree(dir_to_create)
+
     def test_open_file_opens_file_for_reading(self):
-        with tempfile.TemporaryDirectory() as temp_dir:
-            file_to_open = os.path.join(temp_dir, "test_open.txt")
+        temp_dir = tempfile.mkdtemp()
+
+        file_to_open = os.path.join(temp_dir, "test_open.txt")
 
-            with open(file_to_open, "w") as fid:
-                fid.write("this is text")
+        with open(file_to_open, "w") as fid:
+            fid.write("this is text")
 
-            with self.osutils.open_file(file_to_open) as fid:
-                content = fid.read()
+        with self.osutils.open_file(file_to_open) as fid:
+            content = fid.read()
 
-            self.assertEqual("this is text", content)
+        self.assertEqual("this is text", content)
+
+        shutil.rmtree(temp_dir)
 
     def test_open_file_opens_file_for_writing(self):
-        with tempfile.TemporaryDirectory() as temp_dir:
-            file_to_open = os.path.join(temp_dir, "test_open.txt")
+        temp_dir = tempfile.mkdtemp()
+
+        file_to_open = os.path.join(temp_dir, "test_open.txt")
+
+        with self.osutils.open_file(file_to_open, "w") as fid:
+            fid.write("this is some other text")
 
-            with self.osutils.open_file(file_to_open, "w") as fid:
-                fid.write("this is some other text")
+        with self.osutils.open_file(file_to_open) as fid:
+            content = fid.read()
 
-            with self.osutils.open_file(file_to_open) as fid:
-                content = fid.read()
+        self.assertEqual("this is some other text", content)
 
-            self.assertEqual("this is some other text", content)
+        shutil.rmtree(temp_dir)
 
 
 class TestDependencyUtils(TestCase):
diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py
index 879b6f66d..58c487ec9 100644
--- a/tests/unit/workflows/nodejs_npm/test_actions.py
+++ b/tests/unit/workflows/nodejs_npm/test_actions.py
@@ -1,4 +1,5 @@
 from json import dumps
+from os.path import basename
 from unittest import TestCase
 from mock import patch
 
@@ -72,6 +73,7 @@ def test_tars_and_unpacks_npm_project(self, OSUtilMock, SubprocessNpmMock):
         osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
         osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
         osutils.joinpath.side_effect = lambda *args: "/".join(args)
+        osutils.filename.side_effect = lambda path: basename(path)
         osutils.open_file.side_effect = MockOpener().open
 
         subprocess_npm.run.return_value = "package.tar"
@@ -118,6 +120,7 @@ def test_tars_and_unpacks_local_dependencies(self, OSUtilMock, SubprocessNpmMock
         osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
         osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
         osutils.joinpath.side_effect = lambda *args: "/".join(args)
+        osutils.filename.side_effect = lambda path: basename(path)
         osutils.open_file.side_effect = MockOpener(file_open_responses).open
 
         subprocess_npm.run.return_value = "package.tar"

From 62b4f20ac1bf5b3ee117f6a701579d558437e05d Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Tue, 19 Jan 2021 08:26:14 -0600
Subject: [PATCH 13/24] removed manifest backup during write

---
 .../workflows/nodejs_npm/utils.py               | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index fe6ca1d4b..af882f625 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -172,16 +172,11 @@ def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
         Helper function to update dependency path to localized tar
         """
 
-        manifest_backup = "{}.bak".format(manifest_path)
-        osutils.copy_file(manifest_path, manifest_backup)
+        with osutils.open_file(manifest_path, "r") as manifest_read_file:
+            manifest = json.loads(manifest_read_file.read())
 
-        with osutils.open_file(manifest_backup, "r") as manifest_backup_file:
-            manifest = json.loads(manifest_backup_file.read())
+        if "dependencies" in manifest and dep_name in manifest["dependencies"]:
+            manifest["dependencies"][dep_name] = "file:{}".format(dependency_tarfile_path)
 
-            if "dependencies" in manifest and dep_name in manifest["dependencies"]:
-                manifest["dependencies"][dep_name] = "file:{}".format(dependency_tarfile_path)
-
-                with osutils.open_file(manifest_path, "w") as manifest_write_file:
-                    manifest_write_file.write(json.dumps(manifest, indent=4))
-
-        osutils.remove_file(manifest_backup)
+            with osutils.open_file(manifest_path, "w") as manifest_write_file:
+                manifest_write_file.write(json.dumps(manifest, indent=4))

From 3fb1ea19a1f878bd33a26153086f53241ef334fe Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Tue, 19 Jan 2021 10:15:50 -0600
Subject: [PATCH 14/24] refactored npm-pack + extract into utility

---
 .../workflows/nodejs_npm/actions.py           | 12 +++-------
 .../workflows/nodejs_npm/utils.py             | 22 ++++++++++++++-----
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index e4175ff3c..ca18bf273 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -57,15 +57,9 @@ def execute(self):
 
             LOG.debug("NODEJS packaging %s to %s", package_path, self.scratch_dir)
 
-            tarfile_name = self.subprocess_npm.run(["pack", "-q", package_path], cwd=self.scratch_dir).splitlines()[-1]
-
-            LOG.debug("NODEJS packed to %s", tarfile_name)
-
-            tarfile_path = self.osutils.joinpath(self.scratch_dir, tarfile_name)
-
-            LOG.debug("NODEJS extracting to %s", self.artifacts_dir)
-
-            self.osutils.extract_tarfile(tarfile_path, self.artifacts_dir)
+            DependencyUtils.ship_module(
+                package_path, self.scratch_dir, self.artifacts_dir, self.osutils, self.subprocess_npm
+            )
 
             self._package_local_dependencies()
 
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index af882f625..ad5a53a07 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -72,6 +72,21 @@ class DependencyUtils(object):
     Collection of helper functions for managing local NPM dependencies
     """
 
+    @staticmethod
+    def ship_module(package_path, from_dir, to_dir, osutils, subprocess_npm):
+        """
+        Helper function to package a module and extract it to a new directory
+        """
+        tarfile_name = subprocess_npm.run(["pack", "-q", package_path], cwd=from_dir).splitlines()[-1]
+
+        LOG.debug("NODEJS packed to %s", tarfile_name)
+
+        tarfile_path = osutils.joinpath(from_dir, tarfile_name)
+
+        LOG.debug("NODEJS extracting to %s", to_dir)
+
+        osutils.extract_tarfile(tarfile_path, to_dir)
+
     @staticmethod
     def get_local_dependencies(manifest_path, osutils):
         """
@@ -122,13 +137,8 @@ def package_local_dependency(
                 osutils.mkdir(output_dir)
             top_level = True
         else:
-            tarfile_name = subprocess_npm.run(["pack", "-q", package_path], cwd=scratch_dir).splitlines()[-1]
-            tarfile_path = osutils.joinpath(scratch_dir, tarfile_name)
-
             LOG.debug("NODEJS extracting child dependency for recursive dependency check")
-
-            osutils.extract_tarfile(tarfile_path, artifacts_dir)
-
+            DependencyUtils.ship_module(package_path, scratch_dir, artifacts_dir, osutils, subprocess_npm)
             top_level = False
 
         local_manifest_path = osutils.joinpath(artifacts_dir, "package", "package.json")

From a7c25b469a783e4ae70349a191d4eac560f97498 Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Tue, 19 Jan 2021 10:17:21 -0600
Subject: [PATCH 15/24] remove unecessary else in nodejs_npm utils

---
 aws_lambda_builders/workflows/nodejs_npm/utils.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index ad5a53a07..79e66b330 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -100,8 +100,8 @@ def get_local_dependencies(manifest_path, osutils):
                 return dict(
                     (k, v) for (k, v) in manifest["dependencies"].items() if DependencyUtils.is_local_dependency(v)
                 )
-            else:
-                return {}
+
+            return {}
 
     @staticmethod
     def is_local_dependency(path):

From 84cf9b5643e4ca0736300b5d89c88f4427025978 Mon Sep 17 00:00:00 2001
From: Mathieu Grandis <grmathie@amazon.com>
Date: Tue, 16 Feb 2021 18:14:37 -0800
Subject: [PATCH 16/24] refactor: Light refactor to do everything in one pass,
 restore the final package.json from their backups

---
 .../workflows/nodejs_npm/actions.py           |  89 +++++++--------
 .../workflows/nodejs_npm/utils.py             | 104 +++++++++---------
 .../workflows/nodejs_npm/workflow.py          |   4 +-
 3 files changed, 100 insertions(+), 97 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index ca18bf273..6d900e5de 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -48,45 +48,24 @@ def __init__(self, artifacts_dir, scratch_dir, manifest_path, osutils, subproces
 
     def execute(self):
         """
-        Runs the action.
+        Runs the action
 
-        :raises lambda_builders.actions.ActionFailedError: when NPM packaging fails
+        Raises
+        ------
+        ActionFailedError
+            When NPM packaging fails
         """
         try:
-            package_path = "file:{}".format(self.osutils.abspath(self.osutils.dirname(self.manifest_path)))
-
-            LOG.debug("NODEJS packaging %s to %s", package_path, self.scratch_dir)
-
-            DependencyUtils.ship_module(
-                package_path, self.scratch_dir, self.artifacts_dir, self.osutils, self.subprocess_npm
+            tar_path = DependencyUtils.package_dependencies(
+                self.manifest_path, self.scratch_dir, {}, self.osutils, self.subprocess_npm
             )
 
-            self._package_local_dependencies()
+            LOG.debug("NODEJS extracting final %s to artifacts dir %s", tar_path, self.artifacts_dir)
 
+            self.osutils.extract_tarfile(tar_path, self.artifacts_dir)
         except NpmExecutionError as ex:
             raise ActionFailedError(str(ex))
 
-    def _package_local_dependencies(self):
-        """
-        Iterates (recursively) through any local dependencies defined in package.json.
-
-        :raises lambda_builders.actions.ActionFailedError: when any file system operations fail
-        """
-
-        try:
-            LOG.debug("NODEJS searching for local dependencies")
-
-            parent_package_path = self.osutils.abspath(self.osutils.dirname(self.manifest_path))
-
-            DependencyUtils.package_local_dependency(
-                parent_package_path, ".", self.artifacts_dir, self.scratch_dir, None, self.osutils, self.subprocess_npm
-            )
-
-            LOG.debug("NODEJS finished processing local dependencies")
-
-        except OSError as ex:
-            raise ActionFailedError(str(ex))
-
 
 class NodejsNpmInstallAction(BaseAction):
 
@@ -133,7 +112,7 @@ def execute(self):
 class NodejsNpmrcCopyAction(BaseAction):
 
     """
-    A Lambda Builder Action that copies NPM config file .npmrc
+    Lambda Builder Action that copies NPM config file .npmrc
     """
 
     NAME = "CopyNpmrc"
@@ -175,13 +154,14 @@ def execute(self):
             raise ActionFailedError(str(ex))
 
 
-class NodejsNpmrcCleanUpAction(BaseAction):
-
+class NodejsCleanUpAction(BaseAction):
     """
-    A Lambda Builder Action that cleans NPM config file .npmrc
+    Lambda Builder Action that cleans up after install:
+    - Removes NPM config file .npmrc
+    - Restores modified package.json files from their backup
     """
 
-    NAME = "CleanUpNpmrc"
+    NAME = "CleanUp"
     DESCRIPTION = "Cleans artifacts dir"
     PURPOSE = Purpose.COPY_SOURCE
 
@@ -195,22 +175,45 @@ def __init__(self, artifacts_dir, osutils):
         :param osutils: An instance of OS Utilities for file manipulation
         """
 
-        super(NodejsNpmrcCleanUpAction, self).__init__()
+        super(NodejsCleanUpAction, self).__init__()
         self.artifacts_dir = artifacts_dir
         self.osutils = osutils
 
     def execute(self):
         """
-        Runs the action.
+        Runs the actions
 
-        :raises lambda_builders.actions.ActionFailedError: when .npmrc copying fails
+        Raises
+        ------
+        ActionFailedError
+            When .npmrc removing or package.json restoring fails
         """
 
         try:
-            npmrc_path = self.osutils.joinpath(self.artifacts_dir, ".npmrc")
-            if self.osutils.file_exists(npmrc_path):
-                LOG.debug(".npmrc cleanup in: %s", self.artifacts_dir)
-                self.osutils.remove_file(npmrc_path)
-
+            self._remove_npmrc()
+            self._restore_package_json()
         except OSError as ex:
             raise ActionFailedError(str(ex))
+
+    def _remove_npmrc(self):
+        """
+        Removes the .npmrc file
+        """
+        npmrc_path = self.osutils.joinpath(self.artifacts_dir, ".npmrc")
+
+        if self.osutils.file_exists(npmrc_path):
+            LOG.debug(".npmrc cleanup in: %s", self.artifacts_dir)
+            self.osutils.remove_file(npmrc_path)
+
+    def _restore_package_json(self):
+        """
+        Restores the original package.json from its backup
+        """
+        org_file = "package.json"
+        bak_file = org_file + ".bak"
+
+        for (root, _, files) in self.osutils.walk(self.artifacts_dir):
+            if bak_file in files:
+                bak_path = self.osutils.joinpath(root, bak_file)
+                self.osutils.copy_file(bak_path, self.osutils.joinpath(root, org_file))
+                self.osutils.remove_file(bak_path)
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 79e66b330..4123f70ed 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -14,7 +14,6 @@
 
 
 class OSUtils(object):
-
     """
     Wrapper around file system functions, to make it easy to
     unit test actions in memory
@@ -42,6 +41,12 @@ def joinpath(self, *args):
     def mkdir(self, path):
         return os.mkdir(path)
 
+    def makedirs(self, path):
+        return os.makedirs(path)
+
+    def normpath(self, *args):
+        return os.path.normpath(*args)
+
     def open_file(self, filename, mode="r"):
         return open(filename, mode)
 
@@ -65,6 +70,9 @@ def abspath(self, path):
     def is_windows(self):
         return platform.system().lower() == "windows"
 
+    def walk(self, dir, topdown=True):
+        return os.walk(dir, topdown)
+
 
 class DependencyUtils(object):
 
@@ -115,78 +123,70 @@ def is_local_dependency(path):
             return False
 
     @staticmethod
-    def package_local_dependency(
-        parent_package_path, rel_package_path, artifacts_dir, scratch_dir, output_dir, osutils, subprocess_npm
-    ):
-        """
-        Helper function to recurse local dependencies and package them to a common directory
-        """
-
-        if rel_package_path.startswith("file:"):
-            rel_package_path = rel_package_path[5:].strip()
-
-        package_path = osutils.abspath(osutils.joinpath(parent_package_path, rel_package_path))
+    def package_dependencies(manifest_path, scratch_dir, packed_manifests, osutils, subprocess_npm):
+        manifest_path = osutils.normpath(manifest_path)
+        LOG.debug("NODEJS processing %s", manifest_path)
+        manifest_hash = str(abs(hash(manifest_path)))
+        if manifest_hash in packed_manifests:
+            # Already processed or circular dependency
+            # If empty, it will be removed from the manifest
+            return packed_manifests[manifest_hash]
+        packed_manifests[manifest_hash] = ""
+        manifest_dir = osutils.dirname(manifest_path)
+        manifest_scratch_dir = osutils.joinpath(scratch_dir, manifest_hash)
+        manifest_scratch_package_dir = osutils.joinpath(manifest_scratch_dir, "package")
+        osutils.makedirs(manifest_scratch_package_dir)
+
+        # Pack and copy module to scratch so that we don't update the customers files in place
+        DependencyUtils.ship_module(manifest_dir, scratch_dir, manifest_scratch_dir, osutils, subprocess_npm)
+
+        # Process local children dependencies
+        local_dependencies = DependencyUtils.get_local_dependencies(manifest_path, osutils)
 
-        if not osutils.dir_exists(scratch_dir):
-            osutils.mkdir(scratch_dir)
-
-        if output_dir is None:
-            # TODO: get a higher level output_dir to keep process locals between jobs
-            output_dir = osutils.joinpath(artifacts_dir, "@aws_lambda_builders_local_dep")
-            if not osutils.dir_exists(output_dir):
-                osutils.mkdir(output_dir)
-            top_level = True
-        else:
-            LOG.debug("NODEJS extracting child dependency for recursive dependency check")
-            DependencyUtils.ship_module(package_path, scratch_dir, artifacts_dir, osutils, subprocess_npm)
-            top_level = False
-
-        local_manifest_path = osutils.joinpath(artifacts_dir, "package", "package.json")
-        local_dependencies = DependencyUtils.get_local_dependencies(local_manifest_path, osutils)
         for (dep_name, dep_path) in local_dependencies.items():
-            dep_scratch_dir = osutils.joinpath(scratch_dir, str(abs(hash(dep_name))))
-
-            # TODO: if dep_scratch_dir exists (anywhere up path), it means we've already processed it this round, skip
-
-            dep_artifacts_dir = osutils.joinpath(dep_scratch_dir, "unpacked")
+            if dep_path.startswith("file:"):
+                dep_path = dep_path[5:].strip()
+            dep_manifest = osutils.joinpath(manifest_dir, dep_path, "package.json")
 
-            LOG.debug("NODEJS packaging dependency, %s, from %s to %s", dep_name, parent_package_path, output_dir)
-
-            dependency_tarfile_path = DependencyUtils.package_local_dependency(
-                package_path, dep_path, dep_artifacts_dir, dep_scratch_dir, output_dir, osutils, subprocess_npm
+            tar_path = DependencyUtils.package_dependencies(
+                dep_manifest, scratch_dir, packed_manifests, osutils, subprocess_npm
             )
 
-            packaged_dependency_tarfile_path = osutils.joinpath(output_dir, osutils.filename(dependency_tarfile_path))
-            osutils.copy_file(dependency_tarfile_path, output_dir)
-
-            LOG.debug("NODEJS packed localized child dependency to %s", packaged_dependency_tarfile_path)
+            manifest_scratch_path = osutils.joinpath(manifest_scratch_package_dir, "package.json")
 
-            LOG.debug("NODEJS updating package.json %s", local_manifest_path)
+            # Make a backup we will use to restore in the final build folder
+            osutils.copy_file(manifest_scratch_path, manifest_scratch_path + ".bak")
 
-            DependencyUtils.update_manifest(local_manifest_path, dep_name, packaged_dependency_tarfile_path, osutils)
+            DependencyUtils.update_manifest(manifest_scratch_path, dep_name, tar_path, osutils)
 
-        if not top_level:
-            localized_package_dir = osutils.joinpath(artifacts_dir, "package")
+        # Pack the current dependency
+        tarfile_name = subprocess_npm.run(["pack", "-q", manifest_scratch_package_dir], cwd=scratch_dir).splitlines()[
+            -1
+        ]
 
-            LOG.debug("NODEJS repackaging child dependency")
+        packed_manifests[manifest_hash] = osutils.joinpath(scratch_dir, tarfile_name)
 
-            tarfile_name = subprocess_npm.run(
-                ["pack", "-q", localized_package_dir], cwd=localized_package_dir
-            ).splitlines()[-1]
+        LOG.debug("NODEJS %s packed to %s", manifest_scratch_package_dir, packed_manifests[manifest_hash])
 
-            return osutils.joinpath(localized_package_dir, tarfile_name)
+        return packed_manifests[manifest_hash]
 
     @staticmethod
     def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
         """
         Helper function to update dependency path to localized tar
         """
-
         with osutils.open_file(manifest_path, "r") as manifest_read_file:
             manifest = json.loads(manifest_read_file.read())
 
         if "dependencies" in manifest and dep_name in manifest["dependencies"]:
-            manifest["dependencies"][dep_name] = "file:{}".format(dependency_tarfile_path)
+            if not dependency_tarfile_path:
+                LOG.debug("NODEJS removing dep '%s' from '%s'", dep_name, manifest_path)
+                manifest["dependencies"].pop(dep_name)
+            else:
+                LOG.debug(
+                    "NODEJS updating dep '%s' of '%s' with '%s'", dep_name, manifest_path, dependency_tarfile_path
+                )
+                manifest["dependencies"][dep_name] = "file:{}".format(dependency_tarfile_path)
 
             with osutils.open_file(manifest_path, "w") as manifest_write_file:
                 manifest_write_file.write(json.dumps(manifest, indent=4))
diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py
index b66cf5162..cbbe825f9 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py
@@ -8,7 +8,7 @@
     NodejsNpmPackAction,
     NodejsNpmInstallAction,
     NodejsNpmrcCopyAction,
-    NodejsNpmrcCleanUpAction,
+    NodejsCleanUpAction,
 )
 from .utils import OSUtils
 from .npm import SubprocessNpm
@@ -54,7 +54,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
             npm_copy_npmrc,
             CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES),
             npm_install,
-            NodejsNpmrcCleanUpAction(artifacts_dir, osutils=osutils),
+            NodejsCleanUpAction(artifacts_dir, osutils=osutils),
         ]
 
     def get_resolvers(self):

From 850e781d7d621bc7a7cce2d54aaf1264c1f44f70 Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Sun, 21 Feb 2021 17:01:50 -0600
Subject: [PATCH 17/24] nodejs_npm Tests passing in python3

---
 .../workflows/nodejs_npm/utils.py             |  1 -
 .../unit/workflows/nodejs_npm/test_actions.py | 38 +++++++---
 tests/unit/workflows/nodejs_npm/test_utils.py | 75 +++++++++++++++++++
 .../workflows/nodejs_npm/test_workflow.py     |  4 +-
 .../nodejs_npm/testdata/module_1/included.js  |  1 +
 .../nodejs_npm/testdata/module_1/package.json |  9 +++
 .../nodejs_npm/testdata/module_2/included.js  |  1 +
 .../nodejs_npm/testdata/module_2/package.json | 13 ++++
 8 files changed, 130 insertions(+), 12 deletions(-)
 create mode 100644 tests/unit/workflows/nodejs_npm/test_utils.py
 create mode 100644 tests/unit/workflows/nodejs_npm/testdata/module_1/included.js
 create mode 100644 tests/unit/workflows/nodejs_npm/testdata/module_1/package.json
 create mode 100644 tests/unit/workflows/nodejs_npm/testdata/module_2/included.js
 create mode 100644 tests/unit/workflows/nodejs_npm/testdata/module_2/package.json

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 4123f70ed..e675068d9 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -75,7 +75,6 @@ def walk(self, dir, topdown=True):
 
 
 class DependencyUtils(object):
-
     """
     Collection of helper functions for managing local NPM dependencies
     """
diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py
index 58c487ec9..8989b49ae 100644
--- a/tests/unit/workflows/nodejs_npm/test_actions.py
+++ b/tests/unit/workflows/nodejs_npm/test_actions.py
@@ -1,14 +1,15 @@
 from json import dumps
-from os.path import basename
-from unittest import TestCase
 from mock import patch
+from os.path import basename, normpath
+from unittest import TestCase
+from re import match
 
 from aws_lambda_builders.actions import ActionFailedError
 from aws_lambda_builders.workflows.nodejs_npm.actions import (
     NodejsNpmPackAction,
     NodejsNpmInstallAction,
     NodejsNpmrcCopyAction,
-    NodejsNpmrcCleanUpAction,
+    NodejsCleanUpAction,
 )
 from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError
 
@@ -59,6 +60,24 @@ def write(self, data):
             raise IOError("file not open for writing")
 
 
+class RegexMatch:
+    expressions = []
+
+    def __init__(self, expressions):
+        self.expressions = expressions
+
+    def __repr__(self):
+        return str(self.expressions)
+
+    def __eq__(self, b):
+        _b = b if isinstance(b, list) else [b]
+
+        if len(self.expressions) != len(_b):
+            return False
+
+        return all([(match(self.expressions[inx], _b[inx]) is not None) for inx in range(len(self.expressions))])
+
+
 class TestNodejsNpmPackAction(TestCase):
     @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils")
     @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm")
@@ -70,6 +89,7 @@ def test_tars_and_unpacks_npm_project(self, OSUtilMock, SubprocessNpmMock):
             "artifacts", "scratch_dir", "manifest", osutils=osutils, subprocess_npm=subprocess_npm
         )
 
+        osutils.normpath.side_effect = lambda value: normpath(value)
         osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
         osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
         osutils.joinpath.side_effect = lambda *args: "/".join(args)
@@ -80,7 +100,7 @@ def test_tars_and_unpacks_npm_project(self, OSUtilMock, SubprocessNpmMock):
 
         action.execute()
 
-        subprocess_npm.run.assert_called_with(["pack", "-q", "file:/abs:/dir:manifest"], cwd="scratch_dir")
+        subprocess_npm.run.assert_called_with(RegexMatch(["pack", "-q", r"scratch_dir/\d+?/package$"]), cwd="scratch_dir")
         osutils.extract_tarfile.assert_called_with("scratch_dir/package.tar", "artifacts")
 
     @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils")
@@ -128,8 +148,8 @@ def test_tars_and_unpacks_local_dependencies(self, OSUtilMock, SubprocessNpmMock
         action.execute()
 
         # pack is called once for top level package, then twice for each local dependency
-        self.assertEqual(subprocess_npm.run.call_count, 1 + 2 * 2)
-        subprocess_npm.run.assert_any_call(["pack", "-q", "file:/abs:/dir:manifest"], cwd="scratch_dir")
+        self.assertEqual(subprocess_npm.run.call_count, 2)
+        subprocess_npm.run.assert_any_call(RegexMatch(["pack", "-q", r"scratch_dir/\d+?/package$"]), cwd="scratch_dir")
 
 
 class TestNodejsNpmInstallAction(TestCase):
@@ -198,13 +218,13 @@ def test_raises_action_failed_when_copying_fails(self, OSUtilMock):
             action.execute()
 
 
-class TestNodejsNpmrcCleanUpAction(TestCase):
+class TestNodejsCleanUpAction(TestCase):
     @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils")
     def test_removes_npmrc_if_npmrc_exists(self, OSUtilMock):
         osutils = OSUtilMock.return_value
         osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b)
 
-        action = NodejsNpmrcCleanUpAction("artifacts", osutils=osutils)
+        action = NodejsCleanUpAction("artifacts", osutils=osutils)
         osutils.file_exists.side_effect = [True]
         action.execute()
 
@@ -215,7 +235,7 @@ def test_skips_npmrc_removal_if_npmrc_doesnt_exist(self, OSUtilMock):
         osutils = OSUtilMock.return_value
         osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b)
 
-        action = NodejsNpmrcCleanUpAction("artifacts", osutils=osutils)
+        action = NodejsCleanUpAction("artifacts", osutils=osutils)
         osutils.file_exists.side_effect = [False]
         action.execute()
 
diff --git a/tests/unit/workflows/nodejs_npm/test_utils.py b/tests/unit/workflows/nodejs_npm/test_utils.py
new file mode 100644
index 000000000..6998e2c6d
--- /dev/null
+++ b/tests/unit/workflows/nodejs_npm/test_utils.py
@@ -0,0 +1,75 @@
+import os
+import shutil
+import tempfile
+from unittest import TestCase
+
+from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils, DependencyUtils
+from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm
+
+
+class TestDependencyUtils(TestCase):
+    TEST_DATA_FOLDER = os.path.join(os.path.dirname(__file__), "testdata")
+
+    def setUp(self):
+        self.osutils = OSUtils()
+        self.subprocess_npm = SubprocessNpm(self.osutils)
+        self.uut = DependencyUtils()
+        self.artifacts_dir = tempfile.mkdtemp()
+        self.scratch_dir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        shutil.rmtree(self.artifacts_dir)
+        shutil.rmtree(self.scratch_dir)
+
+    def test_ship_module(self):
+        self.uut.ship_module(
+            os.path.join(self.TEST_DATA_FOLDER, 'module_1'),
+            self.scratch_dir,
+            self.artifacts_dir,
+            self.osutils,
+            self.subprocess_npm
+        )
+
+        expected_files = {"package.json", "included.js"}
+        output_files = set(os.listdir(os.path.join(self.artifacts_dir, 'package')))
+        self.assertEqual(output_files, expected_files)
+
+    def test_get_local_dependencies_with_none(self):
+        dependencies = self.uut.get_local_dependencies(
+            os.path.join(self.TEST_DATA_FOLDER, 'module_1', 'package.json'),
+            self.osutils,
+        )
+
+        self.assertEqual(dependencies, {})
+
+    def test_get_local_dependencies_with_some(self):
+        dependencies = self.uut.get_local_dependencies(
+            os.path.join(self.TEST_DATA_FOLDER, 'module_2', 'package.json'),
+            self.osutils,
+        )
+
+        self.assertEqual(dependencies, {'@mockcompany/module-a': 'file:../modules/module_a'})
+
+    def test_is_local_dependency_when_is(self):
+        self.assertEqual(self.uut.is_local_dependency('file:../modules/module_a'), True)
+
+    def test_is_local_dependency_when_is_not(self):
+        self.assertEqual(self.uut.is_local_dependency('1.2.3'), False)
+
+    def test_is_local_dependency_when_is_not_string(self):
+        self.assertEqual(self.uut.is_local_dependency(None), False)
+
+    def test_package_dependencies(self):
+        package = self.uut.package_dependencies(
+            os.path.join(self.TEST_DATA_FOLDER, 'module_1', 'package.json'),
+            self.scratch_dir,
+            {},
+            self.osutils,
+            self.subprocess_npm
+        )
+        self.assertEqual(os.path.basename(package), 'nodejs_npm_unit_tests_module_1-1.0.0.tgz')
+
+    def test_update_manifest(self):
+        scratch_manifest = os.path.join(self.scratch_dir, 'scratch_package.json')
+        self.osutils.copy_file(os.path.join(self.TEST_DATA_FOLDER, 'module_2', 'package.json'), scratch_manifest)
+        self.uut.update_manifest(scratch_manifest, '@mockcompany/module-a', 'interim/path.tgz', self.osutils)
diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py
index 3dac6d791..d2250824b 100644
--- a/tests/unit/workflows/nodejs_npm/test_workflow.py
+++ b/tests/unit/workflows/nodejs_npm/test_workflow.py
@@ -6,7 +6,7 @@
     NodejsNpmPackAction,
     NodejsNpmInstallAction,
     NodejsNpmrcCopyAction,
-    NodejsNpmrcCleanUpAction,
+    NodejsCleanUpAction,
 )
 
 
@@ -31,4 +31,4 @@ def test_workflow_sets_up_npm_actions(self):
 
         self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction)
 
-        self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction)
+        self.assertIsInstance(workflow.actions[4], NodejsCleanUpAction)
diff --git a/tests/unit/workflows/nodejs_npm/testdata/module_1/included.js b/tests/unit/workflows/nodejs_npm/testdata/module_1/included.js
new file mode 100644
index 000000000..0d4401a11
--- /dev/null
+++ b/tests/unit/workflows/nodejs_npm/testdata/module_1/included.js
@@ -0,0 +1 @@
+console.log('blank')
\ No newline at end of file
diff --git a/tests/unit/workflows/nodejs_npm/testdata/module_1/package.json b/tests/unit/workflows/nodejs_npm/testdata/module_1/package.json
new file mode 100644
index 000000000..3183772d9
--- /dev/null
+++ b/tests/unit/workflows/nodejs_npm/testdata/module_1/package.json
@@ -0,0 +1,9 @@
+{
+  "name": "nodejs_npm_unit_tests_module_1",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["included.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0"
+}
\ No newline at end of file
diff --git a/tests/unit/workflows/nodejs_npm/testdata/module_2/included.js b/tests/unit/workflows/nodejs_npm/testdata/module_2/included.js
new file mode 100644
index 000000000..0d4401a11
--- /dev/null
+++ b/tests/unit/workflows/nodejs_npm/testdata/module_2/included.js
@@ -0,0 +1 @@
+console.log('blank')
\ No newline at end of file
diff --git a/tests/unit/workflows/nodejs_npm/testdata/module_2/package.json b/tests/unit/workflows/nodejs_npm/testdata/module_2/package.json
new file mode 100644
index 000000000..4b8ea150f
--- /dev/null
+++ b/tests/unit/workflows/nodejs_npm/testdata/module_2/package.json
@@ -0,0 +1,13 @@
+{
+  "name": "nodejs_npm_unit_tests_module_1",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["included.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0",
+  "dependencies": {
+    "@mockcompany/module-a": "file:../modules/module_a",
+    "npm_dep": "1.2.3"
+  }
+}
\ No newline at end of file

From e1172ad99d4c07be793a843063373a9c2398e084 Mon Sep 17 00:00:00 2001
From: Casey D Boyer <caseyb1101@gmail.com>
Date: Sun, 21 Feb 2021 17:07:00 -0600
Subject: [PATCH 18/24] nodejs_npm Tests in python2 passing and formatter
 executed

---
 .../unit/workflows/nodejs_npm/test_actions.py |  4 ++-
 tests/unit/workflows/nodejs_npm/test_utils.py | 28 +++++++++----------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py
index 8989b49ae..b762fbab8 100644
--- a/tests/unit/workflows/nodejs_npm/test_actions.py
+++ b/tests/unit/workflows/nodejs_npm/test_actions.py
@@ -100,7 +100,9 @@ def test_tars_and_unpacks_npm_project(self, OSUtilMock, SubprocessNpmMock):
 
         action.execute()
 
-        subprocess_npm.run.assert_called_with(RegexMatch(["pack", "-q", r"scratch_dir/\d+?/package$"]), cwd="scratch_dir")
+        subprocess_npm.run.assert_called_with(
+            RegexMatch(["pack", "-q", r"scratch_dir/\d+?/package$"]), cwd="scratch_dir"
+        )
         osutils.extract_tarfile.assert_called_with("scratch_dir/package.tar", "artifacts")
 
     @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils")
diff --git a/tests/unit/workflows/nodejs_npm/test_utils.py b/tests/unit/workflows/nodejs_npm/test_utils.py
index 6998e2c6d..544e58320 100644
--- a/tests/unit/workflows/nodejs_npm/test_utils.py
+++ b/tests/unit/workflows/nodejs_npm/test_utils.py
@@ -23,20 +23,20 @@ def tearDown(self):
 
     def test_ship_module(self):
         self.uut.ship_module(
-            os.path.join(self.TEST_DATA_FOLDER, 'module_1'),
+            os.path.join(self.TEST_DATA_FOLDER, "module_1"),
             self.scratch_dir,
             self.artifacts_dir,
             self.osutils,
-            self.subprocess_npm
+            self.subprocess_npm,
         )
 
         expected_files = {"package.json", "included.js"}
-        output_files = set(os.listdir(os.path.join(self.artifacts_dir, 'package')))
+        output_files = set(os.listdir(os.path.join(self.artifacts_dir, "package")))
         self.assertEqual(output_files, expected_files)
 
     def test_get_local_dependencies_with_none(self):
         dependencies = self.uut.get_local_dependencies(
-            os.path.join(self.TEST_DATA_FOLDER, 'module_1', 'package.json'),
+            os.path.join(self.TEST_DATA_FOLDER, "module_1", "package.json"),
             self.osutils,
         )
 
@@ -44,32 +44,32 @@ def test_get_local_dependencies_with_none(self):
 
     def test_get_local_dependencies_with_some(self):
         dependencies = self.uut.get_local_dependencies(
-            os.path.join(self.TEST_DATA_FOLDER, 'module_2', 'package.json'),
+            os.path.join(self.TEST_DATA_FOLDER, "module_2", "package.json"),
             self.osutils,
         )
 
-        self.assertEqual(dependencies, {'@mockcompany/module-a': 'file:../modules/module_a'})
+        self.assertEqual(dependencies, {"@mockcompany/module-a": "file:../modules/module_a"})
 
     def test_is_local_dependency_when_is(self):
-        self.assertEqual(self.uut.is_local_dependency('file:../modules/module_a'), True)
+        self.assertEqual(self.uut.is_local_dependency("file:../modules/module_a"), True)
 
     def test_is_local_dependency_when_is_not(self):
-        self.assertEqual(self.uut.is_local_dependency('1.2.3'), False)
+        self.assertEqual(self.uut.is_local_dependency("1.2.3"), False)
 
     def test_is_local_dependency_when_is_not_string(self):
         self.assertEqual(self.uut.is_local_dependency(None), False)
 
     def test_package_dependencies(self):
         package = self.uut.package_dependencies(
-            os.path.join(self.TEST_DATA_FOLDER, 'module_1', 'package.json'),
+            os.path.join(self.TEST_DATA_FOLDER, "module_1", "package.json"),
             self.scratch_dir,
             {},
             self.osutils,
-            self.subprocess_npm
+            self.subprocess_npm,
         )
-        self.assertEqual(os.path.basename(package), 'nodejs_npm_unit_tests_module_1-1.0.0.tgz')
+        self.assertEqual(os.path.basename(package), "nodejs_npm_unit_tests_module_1-1.0.0.tgz")
 
     def test_update_manifest(self):
-        scratch_manifest = os.path.join(self.scratch_dir, 'scratch_package.json')
-        self.osutils.copy_file(os.path.join(self.TEST_DATA_FOLDER, 'module_2', 'package.json'), scratch_manifest)
-        self.uut.update_manifest(scratch_manifest, '@mockcompany/module-a', 'interim/path.tgz', self.osutils)
+        scratch_manifest = os.path.join(self.scratch_dir, "scratch_package.json")
+        self.osutils.copy_file(os.path.join(self.TEST_DATA_FOLDER, "module_2", "package.json"), scratch_manifest)
+        self.uut.update_manifest(scratch_manifest, "@mockcompany/module-a", "interim/path.tgz", self.osutils)

From 36833dd1ee4270782762afffabce7212bed5ed0b Mon Sep 17 00:00:00 2001
From: Mathieu Grandis <grmathie@amazon.com>
Date: Wed, 24 Feb 2021 16:45:21 -0800
Subject: [PATCH 19/24] chore: Update the DESIGN.md doc now that we track paths

---
 .../workflows/nodejs_npm/DESIGN.md            | 51 +++++++++----------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md
index db3b6f0e6..381c53a44 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md
+++ b/aws_lambda_builders/workflows/nodejs_npm/DESIGN.md
@@ -21,14 +21,14 @@ production dependencies (normally the minimum required for correct execution).
 All these dependency types are mixed in the same directory.
 
 To speed up Lambda startup time and optimise usage costs, the correct thing to
-do in most cases is just to package up production dependencies. During development 
-work we can expect that the local `node_modules` directory contains all the 
+do in most cases is just to package up production dependencies. During development
+work we can expect that the local `node_modules` directory contains all the
 various dependency types, and NPM does not provide a way to directly identify
-just the ones relevant for production. To identify production dependencies, 
+just the ones relevant for production. To identify production dependencies,
 this packager needs to copy the source to a clean temporary directory and re-run
 dependency installation there.
   
-A frequently used trick to speed up NodeJS Lambda deployment is to avoid 
+A frequently used trick to speed up NodeJS Lambda deployment is to avoid
 bundling the `aws-sdk`, since it is already available on the Lambda VM.
 This makes deployment significantly faster for single-file lambdas, for
 example. Although this is not good from a consistency and compatibility
@@ -40,7 +40,7 @@ dependencies.
 
 Other runtimes do not have this flexibility, so instead of adding a specific
 parameter to the SAM CLI, the packager should support a flag to include or
-exclude optional dependencies through environment variables. 
+exclude optional dependencies through environment variables.
 
 NPM also provides support for running user-defined scripts as part of the build
 process, so this packager needs to support standard NPM script execution.
@@ -63,9 +63,9 @@ versions, when using on several machines with different project layout it can
 lead to uninstallable dependencies. 
 
 NPM dependencies are usually plain javascript libraries, but they may include
-native binaries precompiled for a particular platform, or require some system 
-libraries to be installed. A notable example is `sharp`, a popular image 
-manipulation library, that uses symbolic links to system libraries. Another 
+native binaries precompiled for a particular platform, or require some system
+libraries to be installed. A notable example is `sharp`, a popular image
+manipulation library, that uses symbolic links to system libraries. Another
 notable example is `puppeteer`, a library to control a headless Chrome browser,
 that downloads a Chromium binary for the target platform during installation.
 
@@ -81,41 +81,37 @@ is as follows.
 #### Step 1: Prepare a clean copy of the project source files
 
 Execute `npm pack` to perform project-specific packaging using the supplied
-`package.json` manifest, which will automatically exclude temporary files, 
-test resources and other source files unnecessary for running in a production 
+`package.json` manifest, which will automatically exclude temporary files,
+test resources and other source files unnecessary for running in a production
 environment.
 
 This will produce a `tar` archive that needs to be unpacked into the artifacts
 directory.  Note that the archive will actually contain a `package`
 subdirectory containing the files, so it's not enough to just directly unpack
-files. 
+files.
 
 #### Step 2: Rewrite local dependencies
 
 To optimise disk space and avoid including development dependencies from other
 locally linked packages, inspect the `package.json` manifest looking for dependencies
 referring to local file paths (can be identified as they start with `.` or `file:`),
-then for each dependency recursively execute the packaging process 
+then for each dependency recursively execute the packaging process
 
-Local dependencies may include other local dependencies themselves, this is a very 
-common way of sharing configuration or development utilities such as linting or testing 
+Local dependencies may include other local dependencies themselves, this is a very
+common way of sharing configuration or development utilities such as linting or testing
 tools. This means that for each packaged local dependency this packager needs to
 recursively apply the packaging process.
 
-_(out of scope for the current version)_
-
 It also means that the packager needs to track local paths and avoid re-packaging directories it already visited.
 
-_(out of scope for the current version)_
-
 NPM produces a `tar` archive while packaging that can be directly included as a
 dependency.  This will make NPM unpack and install a copy correctly. Once the
 packager produces all `tar` archives required by local dependencies, rewrite
 the manifest to point to `tar` files instead of the original location.
 
 If the project contains a package lock file, this will cause NPM to ignore changes
-to the package.json manifest. In this case, the packager will need to remove 
-`package-lock.json` so that dependency rewrites take effect. 
+to the package.json manifest. In this case, the packager will need to remove
+`package-lock.json` so that dependency rewrites take effect.
 
 #### Step 3: Install dependencies
 
@@ -123,16 +119,17 @@ The packager should then run `npm install` to download an expand all dependencie
 the local `node_modules` subdirectory. This has to be executed in the directory with
 a clean copy of the source files.
 
-Note that NPM can be configured to use proxies or local company repositories using 
-a local file, `.npmrc`. The packaging process from step 1 normally excludes this file, so it needs 
-to be copied before dependency installation, and then removed. 
+Note that NPM can be configured to use proxies or local company repositories using
+a local file, `.npmrc`. The packaging process from step 1 normally excludes this file, so it needs
+to be copied before dependency installation, and then removed.
 
-Some users may want to exclude optional dependencies, or even include development dependencies. 
-To avoid incompatible flags in the `sam` CLI, the packager should allow users to specify 
+Some users may want to exclude optional dependencies, or even include development dependencies.
+To avoid incompatible flags in the `sam` CLI, the packager should allow users to specify
 options for the `npm install` command using an environment variable.
+
 _(out of scope for the current version)_
 
 To fully support dependencies that download or compile binaries for a target platform, this step
-needs to be executed inside a Docker image compatible with AWS Lambda. 
-_(out of scope for the current version)_
+needs to be executed inside a Docker image compatible with AWS Lambda.
 
+_(out of scope for the current version)_

From a81b4d4827233ed35238abfef390a6e210c69fac Mon Sep 17 00:00:00 2001
From: Mathieu Grandis <grmathie@amazon.com>
Date: Wed, 24 Feb 2021 17:30:23 -0800
Subject: [PATCH 20/24] refactor: Removed OSUtils.filename (not used anymore)
 and added missing OSUtils tests and doc

---
 .../workflows/nodejs_npm/utils.py             | 75 +++++++++++++++++--
 .../workflows/nodejs_npm/test_utils.py        | 55 +++++++-------
 .../workflows/nodejs_npm/test_nodejs_npm.py   |  2 -
 .../testdata/modules/module_b/package.json    |  7 +-
 .../unit/workflows/nodejs_npm/test_actions.py |  2 -
 5 files changed, 100 insertions(+), 41 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index e675068d9..b25146169 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -32,9 +32,6 @@ def extract_tarfile(self, tarfile_path, unpack_dir):
     def file_exists(self, filename):
         return os.path.isfile(filename)
 
-    def filename(self, filename):
-        return os.path.basename(filename)
-
     def joinpath(self, *args):
         return os.path.join(*args)
 
@@ -83,6 +80,19 @@ class DependencyUtils(object):
     def ship_module(package_path, from_dir, to_dir, osutils, subprocess_npm):
         """
         Helper function to package a module and extract it to a new directory
+
+        Parameters
+        ----------
+        package_path : str
+            Package path
+        from_dir : str
+            Origin directory path
+        to_dir : str
+            Destination directory path
+        osutils : OSUtils
+            OS utils
+        subprocess_npm : SubprocessNpm
+            NPM process
         """
         tarfile_name = subprocess_npm.run(["pack", "-q", package_path], cwd=from_dir).splitlines()[-1]
 
@@ -98,8 +108,19 @@ def ship_module(package_path, from_dir, to_dir, osutils, subprocess_npm):
     def get_local_dependencies(manifest_path, osutils):
         """
         Helper function to extract all local dependencies in a package.json manifest
-        """
 
+        Parameters
+        ----------
+        manifest_path : str
+            Manifest path
+        osutils : OSUtils
+            OS utils
+
+        Returns
+        -------
+        dict
+            Dict of local dependencies key->value if any, empty otherwise
+        """
         with osutils.open_file(manifest_path) as manifest_file:
             manifest = json.loads(manifest_file.read())
 
@@ -113,9 +134,18 @@ def get_local_dependencies(manifest_path, osutils):
     @staticmethod
     def is_local_dependency(path):
         """
-        Helper function to check if package dependency is a local package
-        """
+        Helper function to check if a dependency is a local package
 
+        Parameters
+        ----------
+        path : str
+            Path to check
+
+        Returns
+        -------
+        boolean
+            True if a local dependency, False otherwise
+        """
         try:
             return path.startswith("file:") or path.startswith(".")
         except AttributeError:
@@ -123,6 +153,28 @@ def is_local_dependency(path):
 
     @staticmethod
     def package_dependencies(manifest_path, scratch_dir, packed_manifests, osutils, subprocess_npm):
+        """
+        Recursively packages NPM dependencies, including local ones.
+        Handles circular dependencies and skips already processed ones.
+
+        Parameters
+        ----------
+        manifest_path : str
+            Path to the manifest to package
+        scratch_dir : str
+            Path to the scratch directory
+        packed_manifests : dict
+            Dict of hashes pointing to packed manifest tar used to avoid
+        osutils : OSUtils
+            OS utils
+        subprocess_npm : SubprocessNpm
+            NPM process
+
+        Returns
+        -------
+        string
+            Path to the packed tar file
+        """
         manifest_path = osutils.normpath(manifest_path)
         LOG.debug("NODEJS processing %s", manifest_path)
         manifest_hash = str(abs(hash(manifest_path)))
@@ -173,6 +225,17 @@ def package_dependencies(manifest_path, scratch_dir, packed_manifests, osutils,
     def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
         """
         Helper function to update dependency path to localized tar
+
+        Parameters
+        ----------
+        manifest_path : str
+            Manifest path to update
+        dep_name : str
+            Dependency name
+        dependency_tarfile_path : str
+            Packed dependency tar file path
+        osutils : OSUtils
+            OS utils
         """
         with osutils.open_file(manifest_path, "r") as manifest_read_file:
             manifest = json.loads(manifest_read_file.read())
diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py
index ae6ee6524..6e500dad9 100644
--- a/tests/functional/workflows/nodejs_npm/test_utils.py
+++ b/tests/functional/workflows/nodejs_npm/test_utils.py
@@ -10,45 +10,32 @@
 
 class TestOSUtils(TestCase):
     def setUp(self):
-
         self.osutils = utils.OSUtils()
 
     def test_copy_file_copies_existing_file_into_a_dir(self):
-
         test_file = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz")
-
         test_dir = tempfile.mkdtemp()
 
         self.osutils.copy_file(test_file, test_dir)
 
         output_files = set(os.listdir(test_dir))
-
         shutil.rmtree(test_dir)
-
         self.assertEqual({"test.tgz"}, output_files)
 
     def test_copy_file_copies_existing_file_into_a_file(self):
-
         test_file = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz")
-
         test_dir = tempfile.mkdtemp()
 
         self.osutils.copy_file(test_file, os.path.join(test_dir, "copied_test.tgz"))
 
         output_files = set(os.listdir(test_dir))
-
         shutil.rmtree(test_dir)
-
         self.assertEqual({"copied_test.tgz"}, output_files)
 
     def test_remove_file_removes_existing_file(self):
-
         test_file = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz")
-
         test_dir = tempfile.mkdtemp()
-
         copied_file = os.path.join(test_dir, "copied_test.tgz")
-
         shutil.copy(test_file, copied_file)
 
         self.osutils.remove_file(copied_file)
@@ -56,27 +43,20 @@ def test_remove_file_removes_existing_file(self):
         self.assertFalse(os.path.isfile(copied_file))
 
     def test_file_exists_checking_if_file_exists_in_a_dir(self):
-
         existing_file = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz")
-
         nonexisting_file = os.path.join(os.path.dirname(__file__), "test_data", "nonexisting.tgz")
 
         self.assertTrue(self.osutils.file_exists(existing_file))
-
         self.assertFalse(self.osutils.file_exists(nonexisting_file))
 
     def test_extract_tarfile_unpacks_a_tar(self):
-
         test_tar = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz")
-
         test_dir = tempfile.mkdtemp()
 
         self.osutils.extract_tarfile(test_tar, test_dir)
 
         output_files = set(os.listdir(test_dir))
-
         shutil.rmtree(test_dir)
-
         self.assertEqual({"test_utils.py"}, output_files)
 
     def test_dirname_returns_directory_for_path(self):
@@ -85,33 +65,26 @@ def test_dirname_returns_directory_for_path(self):
         self.assertEqual(dirname, os.path.dirname(sys.executable))
 
     def test_abspath_returns_absolute_path(self):
-
         result = self.osutils.abspath(".")
 
         self.assertTrue(os.path.isabs(result))
-
         self.assertEqual(result, os.path.abspath("."))
 
     def test_joinpath_joins_path_components(self):
-
         result = self.osutils.joinpath("a", "b", "c")
 
         self.assertEqual(result, os.path.join("a", "b", "c"))
 
     def test_popen_runs_a_process_and_returns_outcome(self):
-
         cwd_py = os.path.join(os.path.dirname(__file__), "..", "..", "testdata", "cwd.py")
 
         p = self.osutils.popen([sys.executable, cwd_py], stdout=self.osutils.pipe, stderr=self.osutils.pipe)
 
         out, err = p.communicate()
-
         self.assertEqual(p.returncode, 0)
-
         self.assertEqual(out.decode("utf8").strip(), os.getcwd())
 
     def test_popen_can_accept_cwd(self):
-
         testdata_dir = os.path.join(os.path.dirname(__file__), "..", "..", "testdata")
 
         p = self.osutils.popen(
@@ -119,9 +92,7 @@ def test_popen_can_accept_cwd(self):
         )
 
         out, err = p.communicate()
-
         self.assertEqual(p.returncode, 0)
-
         self.assertEqual(out.decode("utf8").strip(), os.path.abspath(testdata_dir))
 
     def test_dir_exists(self):
@@ -143,6 +114,21 @@ def test_mkdir_makes_directory(self):
 
         shutil.rmtree(dir_to_create)
 
+    def test_mkdirs_makes_directory_and_subdir(self):
+        dir_to_create = os.path.join(tempfile.gettempdir(), "20201210_some_directory_that_should_not_exist")
+        subdir_to_create = os.path.join(dir_to_create, "subdirectory_that_should_not_exist")
+        self.assertFalse(os.path.isdir(dir_to_create))
+        self.assertFalse(os.path.isdir(subdir_to_create))
+
+        self.osutils.makedirs(subdir_to_create)
+
+        self.assertTrue(os.path.isdir(dir_to_create))
+        self.assertTrue(os.path.isdir(subdir_to_create))
+        shutil.rmtree(dir_to_create)
+
+    def test_normpath_normalizes_paths(self):
+        self.assertEqual("/my/path/with/package.json", self.osutils.normpath("/my/path/with/without/../package.json"))
+
     def test_open_file_opens_file_for_reading(self):
         temp_dir = tempfile.mkdtemp()
 
@@ -173,6 +159,17 @@ def test_open_file_opens_file_for_writing(self):
 
         shutil.rmtree(temp_dir)
 
+    def test_walk_walks_tree(self):
+        all_files = []
+
+        for (_, _, files) in self.osutils.walk(os.path.dirname(__file__)):
+            for f in files:
+                all_files.append(f)
+
+        # Only testing those two as OSes enjoy adding other hidden files (like .DS_Store)
+        self.assertTrue("test_utils.py" in all_files)
+        self.assertTrue("test.tgz" in all_files)
+
 
 class TestDependencyUtils(TestCase):
     def test_is_local_dependency_file_prefix(self):
diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
index 42d792f44..ffe4ed956 100644
--- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
+++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
@@ -121,7 +121,6 @@ def test_builds_project_with_npmrc(self):
         self.assertEqual(expected_modules, output_modules)
 
     def test_fails_if_npm_cannot_resolve_dependencies(self):
-
         source_dir = os.path.join(self.TEST_DATA_FOLDER, "broken-deps")
 
         with self.assertRaises(WorkflowFailedError) as ctx:
@@ -136,7 +135,6 @@ def test_fails_if_npm_cannot_resolve_dependencies(self):
         self.assertIn("No matching version found for minimal-request-promise@0.0.0-NON_EXISTENT", str(ctx.exception))
 
     def test_fails_if_package_json_is_broken(self):
-
         source_dir = os.path.join(self.TEST_DATA_FOLDER, "broken-package")
 
         with self.assertRaises(WorkflowFailedError) as ctx:
diff --git a/tests/integration/workflows/nodejs_npm/testdata/modules/module_b/package.json b/tests/integration/workflows/nodejs_npm/testdata/modules/module_b/package.json
index 1868f25c4..780db8f14 100644
--- a/tests/integration/workflows/nodejs_npm/testdata/modules/module_b/package.json
+++ b/tests/integration/workflows/nodejs_npm/testdata/modules/module_b/package.json
@@ -2,11 +2,14 @@
   "name": "@mockcompany/module-b",
   "version": "1.0.0",
   "description": "",
-  "files": ["index.js"],
+  "files": [
+    "index.js"
+  ],
   "keywords": [],
   "author": "",
   "license": "APACHE2.0",
   "dependencies": {
-    "@mockcompany/module-c": "file:../module_c" 
+    "@mockcompany/module-a": "file:../module_a",
+    "@mockcompany/module-c": "file:../module_c"
   }
 }
\ No newline at end of file
diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py
index b762fbab8..0612278a1 100644
--- a/tests/unit/workflows/nodejs_npm/test_actions.py
+++ b/tests/unit/workflows/nodejs_npm/test_actions.py
@@ -93,7 +93,6 @@ def test_tars_and_unpacks_npm_project(self, OSUtilMock, SubprocessNpmMock):
         osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
         osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
         osutils.joinpath.side_effect = lambda *args: "/".join(args)
-        osutils.filename.side_effect = lambda path: basename(path)
         osutils.open_file.side_effect = MockOpener().open
 
         subprocess_npm.run.return_value = "package.tar"
@@ -142,7 +141,6 @@ def test_tars_and_unpacks_local_dependencies(self, OSUtilMock, SubprocessNpmMock
         osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
         osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
         osutils.joinpath.side_effect = lambda *args: "/".join(args)
-        osutils.filename.side_effect = lambda path: basename(path)
         osutils.open_file.side_effect = MockOpener(file_open_responses).open
 
         subprocess_npm.run.return_value = "package.tar"

From da930a3ab898a6b6972b918efb01ae0049c06177 Mon Sep 17 00:00:00 2001
From: Mathieu Grandis <grmathie@amazon.com>
Date: Wed, 24 Feb 2021 17:42:24 -0800
Subject: [PATCH 21/24] test: Fix test failing on Windows

---
 tests/functional/workflows/nodejs_npm/test_utils.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py
index 6e500dad9..52ca9956f 100644
--- a/tests/functional/workflows/nodejs_npm/test_utils.py
+++ b/tests/functional/workflows/nodejs_npm/test_utils.py
@@ -127,7 +127,9 @@ def test_mkdirs_makes_directory_and_subdir(self):
         shutil.rmtree(dir_to_create)
 
     def test_normpath_normalizes_paths(self):
-        self.assertEqual("/my/path/with/package.json", self.osutils.normpath("/my/path/with/without/../package.json"))
+        result = os.path.normpath("/my/path/with/without/../package.json")
+
+        self.assertEqual(result, self.osutils.normpath("/my/path/with/without/../package.json"))
 
     def test_open_file_opens_file_for_reading(self):
         temp_dir = tempfile.mkdtemp()

From f5dfd234c83fc6e1a587a100105378198f1aaed1 Mon Sep 17 00:00:00 2001
From: Mathieu Grandis <grmathie@amazon.com>
Date: Tue, 16 Mar 2021 18:24:45 -0700
Subject: [PATCH 22/24] refactor: PR review items

---
 .../workflows/nodejs_npm/utils.py             | 62 +++++++++++++++----
 tests/unit/workflows/nodejs_npm/test_utils.py |  2 +-
 2 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index b25146169..1e2a7de23 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -45,7 +45,7 @@ def normpath(self, *args):
         return os.path.normpath(*args)
 
     def open_file(self, filename, mode="r"):
-        return open(filename, mode)
+        return open(filename, mode, encoding="utf-8")
 
     def popen(self, command, stdout=None, stderr=None, env=None, cwd=None):
         p = subprocess.Popen(command, stdout=stdout, stderr=stderr, env=env, cwd=cwd)
@@ -76,10 +76,31 @@ class DependencyUtils(object):
     Collection of helper functions for managing local NPM dependencies
     """
 
+    @staticmethod
+    def package(package_path, from_dir, subprocess_npm):
+        """
+        Packages a module
+
+        Parameters
+        ----------
+        package_path : str
+            Package path
+        from_dir : str
+            Origin directory path
+        subprocess_npm : SubprocessNpm
+            NPM process
+
+        Returns
+        -------
+        str
+            Name of the package tar file
+        """
+        return subprocess_npm.run(["pack", "-q", package_path], cwd=from_dir).splitlines()[-1]
+
     @staticmethod
     def ship_module(package_path, from_dir, to_dir, osutils, subprocess_npm):
         """
-        Helper function to package a module and extract it to a new directory
+        Packages a module and extracts it to a destination directory
 
         Parameters
         ----------
@@ -94,7 +115,7 @@ def ship_module(package_path, from_dir, to_dir, osutils, subprocess_npm):
         subprocess_npm : SubprocessNpm
             NPM process
         """
-        tarfile_name = subprocess_npm.run(["pack", "-q", package_path], cwd=from_dir).splitlines()[-1]
+        tarfile_name = DependencyUtils.package(package_path, from_dir, subprocess_npm)
 
         LOG.debug("NODEJS packed to %s", tarfile_name)
 
@@ -107,7 +128,7 @@ def ship_module(package_path, from_dir, to_dir, osutils, subprocess_npm):
     @staticmethod
     def get_local_dependencies(manifest_path, osutils):
         """
-        Helper function to extract all local dependencies in a package.json manifest
+        Extracts all local dependencies in a package.json manifest
 
         Parameters
         ----------
@@ -126,7 +147,9 @@ def get_local_dependencies(manifest_path, osutils):
 
             if "dependencies" in manifest:
                 return dict(
-                    (k, v) for (k, v) in manifest["dependencies"].items() if DependencyUtils.is_local_dependency(v)
+                    (k, DependencyUtils.strip_file_prefix(v))
+                    for (k, v) in manifest["dependencies"].items()
+                    if DependencyUtils.is_local_dependency(v)
                 )
 
             return {}
@@ -134,7 +157,7 @@ def get_local_dependencies(manifest_path, osutils):
     @staticmethod
     def is_local_dependency(path):
         """
-        Helper function to check if a dependency is a local package
+        Checks if a dependency is a local package
 
         Parameters
         ----------
@@ -151,6 +174,27 @@ def is_local_dependency(path):
         except AttributeError:
             return False
 
+    @staticmethod
+    def strip_file_prefix(dependency):
+        """
+        Strips the "file:" prefix off a local dependency
+        NPM local dependencies can be defined as "./mydep" or "file:./mydep"
+        "file:" should be stripped to build an absolute path to the dependency
+
+        Parameters
+        ----------
+        dependency : str
+            Local dependency
+
+        Returns
+        -------
+        str
+            Local dependency stripped of the "file:" prefix
+        """
+        if dependency.startswith("file:"):
+            dependency = dependency[5:].strip()
+        return dependency
+
     @staticmethod
     def package_dependencies(manifest_path, scratch_dir, packed_manifests, osutils, subprocess_npm):
         """
@@ -195,8 +239,6 @@ def package_dependencies(manifest_path, scratch_dir, packed_manifests, osutils,
         local_dependencies = DependencyUtils.get_local_dependencies(manifest_path, osutils)
 
         for (dep_name, dep_path) in local_dependencies.items():
-            if dep_path.startswith("file:"):
-                dep_path = dep_path[5:].strip()
             dep_manifest = osutils.joinpath(manifest_dir, dep_path, "package.json")
 
             tar_path = DependencyUtils.package_dependencies(
@@ -211,9 +253,7 @@ def package_dependencies(manifest_path, scratch_dir, packed_manifests, osutils,
             DependencyUtils.update_manifest(manifest_scratch_path, dep_name, tar_path, osutils)
 
         # Pack the current dependency
-        tarfile_name = subprocess_npm.run(["pack", "-q", manifest_scratch_package_dir], cwd=scratch_dir).splitlines()[
-            -1
-        ]
+        tarfile_name = DependencyUtils.package(manifest_scratch_package_dir, scratch_dir, subprocess_npm)
 
         packed_manifests[manifest_hash] = osutils.joinpath(scratch_dir, tarfile_name)
 
diff --git a/tests/unit/workflows/nodejs_npm/test_utils.py b/tests/unit/workflows/nodejs_npm/test_utils.py
index 544e58320..353bdd98e 100644
--- a/tests/unit/workflows/nodejs_npm/test_utils.py
+++ b/tests/unit/workflows/nodejs_npm/test_utils.py
@@ -48,7 +48,7 @@ def test_get_local_dependencies_with_some(self):
             self.osutils,
         )
 
-        self.assertEqual(dependencies, {"@mockcompany/module-a": "file:../modules/module_a"})
+        self.assertEqual(dependencies, {"@mockcompany/module-a": "../modules/module_a"})
 
     def test_is_local_dependency_when_is(self):
         self.assertEqual(self.uut.is_local_dependency("file:../modules/module_a"), True)

From bfa2289eb291834ef5c57a65ed09499f7a9b8927 Mon Sep 17 00:00:00 2001
From: Mathieu Grandis <grmathie@amazon.com>
Date: Tue, 16 Mar 2021 18:37:49 -0700
Subject: [PATCH 23/24] fix: Replaced open with io.open for python 2
 compatibility

---
 aws_lambda_builders/workflows/nodejs_npm/utils.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 1e2a7de23..f65d45415 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -2,6 +2,7 @@
 Commonly used utilities
 """
 
+import io
 import json
 import logging
 import os
@@ -44,8 +45,8 @@ def makedirs(self, path):
     def normpath(self, *args):
         return os.path.normpath(*args)
 
-    def open_file(self, filename, mode="r"):
-        return open(filename, mode, encoding="utf-8")
+    def open_file(self, filename, mode="r", encoding="utf-8"):
+        return io.open(filename, mode, encoding=encoding)
 
     def popen(self, command, stdout=None, stderr=None, env=None, cwd=None):
         p = subprocess.Popen(command, stdout=stdout, stderr=stderr, env=env, cwd=cwd)

From ee6576f6d39e67b40e53424f1ef98fc6e5cde2a6 Mon Sep 17 00:00:00 2001
From: Mathieu Grandis <grmathie@amazon.com>
Date: Tue, 16 Mar 2021 18:55:14 -0700
Subject: [PATCH 24/24] fix: Encoding issues

---
 aws_lambda_builders/workflows/nodejs_npm/utils.py   | 2 +-
 tests/functional/workflows/nodejs_npm/test_utils.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index f65d45415..a82b0d73c 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -292,4 +292,4 @@ def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
                 manifest["dependencies"][dep_name] = "file:{}".format(dependency_tarfile_path)
 
             with osutils.open_file(manifest_path, "w") as manifest_write_file:
-                manifest_write_file.write(json.dumps(manifest, indent=4))
+                manifest_write_file.write(json.dumps(manifest, indent=4, ensure_ascii=False))
diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py
index 52ca9956f..9f42361b7 100644
--- a/tests/functional/workflows/nodejs_npm/test_utils.py
+++ b/tests/functional/workflows/nodejs_npm/test_utils.py
@@ -137,7 +137,7 @@ def test_open_file_opens_file_for_reading(self):
         file_to_open = os.path.join(temp_dir, "test_open.txt")
 
         with open(file_to_open, "w") as fid:
-            fid.write("this is text")
+            fid.write(u"this is text")
 
         with self.osutils.open_file(file_to_open) as fid:
             content = fid.read()
@@ -152,7 +152,7 @@ def test_open_file_opens_file_for_writing(self):
         file_to_open = os.path.join(temp_dir, "test_open.txt")
 
         with self.osutils.open_file(file_to_open, "w") as fid:
-            fid.write("this is some other text")
+            fid.write(u"this is some other text")
 
         with self.osutils.open_file(file_to_open) as fid:
             content = fid.read()