From 9911eaedd0d263cfdab49a8ae58f0d48d84fd9e7 Mon Sep 17 00:00:00 2001
From: Slobodan Stojanovic <slobodan@cloudhorizon.com>
Date: Tue, 22 Jan 2019 22:13:40 +0100
Subject: [PATCH 01/10] Local dependencies support for Node.js

Co-authored-by: Gojko Adzic <gojko@gojko.com>
---
 .../workflows/nodejs_npm/actions.py           | 29 +++++++
 .../workflows/nodejs_npm/npm.py               | 76 +++++++++++++++++++
 .../workflows/nodejs_npm/utils.py             | 14 +++-
 .../workflows/nodejs_npm/workflow.py          | 11 ++-
 .../workflows/nodejs_npm/test_utils.py        |  2 +-
 .../workflows/nodejs_npm/test_nodejs_npm.py   | 30 ++++++++
 .../relative-dependencies/lambda/main.js      |  6 ++
 .../relative-dependencies/lambda/package.json | 14 ++++
 .../relative-dependencies/lib/index.js        |  5 ++
 .../relative-dependencies/lib/package.json    | 11 +++
 .../parent_lambda/package.json                | 14 ++++
 .../parent_lambda/parent_main.js              |  6 ++
 tests/unit/workflows/nodejs_npm/test_npm.py   | 56 +++++++++++++-
 .../workflows/nodejs_npm/test_workflow.py     | 10 ++-
 14 files changed, 275 insertions(+), 9 deletions(-)
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/main.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/package.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/index.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/package.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/package.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/parent_main.js

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index ad6e7a927..60f7e7166 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -195,3 +195,32 @@ def execute(self):
 
         except OSError as ex:
             raise ActionFailedError(str(ex))
+
+class NodejsNpmRewriteLocalDependenciesAction(BaseAction):
+
+    """
+    A Lambda Builder Action that rewrites local dependencies
+    """
+
+    NAME = 'RewriteLocalDependencies'
+    DESCRIPTION = "Rewrites local dependencies"
+    PURPOSE = Purpose.RESOLVE_DEPENDENCIES
+
+    def __init__(
+        self,
+        work_dir,
+        original_package_dir,
+        scratch_dir,
+        npm_modules_utils
+    ):
+        super(NodejsNpmRewriteLocalDependenciesAction, self).__init__()
+        self.work_dir = work_dir
+        self.original_package_dir = original_package_dir
+        self.scratch_dir = scratch_dir
+        self.npm_modules_utils = npm_modules_utils
+
+    def execute(self):
+        try:
+            self.npm_modules_utils.rewrite_local_dependencies(self.work_dir, self.original_package_dir)
+        except OSError as ex:
+            raise ActionFailedError(str(ex))
diff --git a/aws_lambda_builders/workflows/nodejs_npm/npm.py b/aws_lambda_builders/workflows/nodejs_npm/npm.py
index b737ff4d1..309636136 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/npm.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/npm.py
@@ -3,6 +3,7 @@
 """
 
 import logging
+import json
 
 LOG = logging.getLogger(__name__)
 
@@ -88,3 +89,78 @@ def run(self, args, cwd=None):
             raise NpmExecutionError(message=err.decode('utf8').strip())
 
         return out.decode('utf8').strip()
+
+class NpmModulesUtils(object):
+
+    """
+
+    """
+
+    def __init__(self, osutils, subprocess_npm, scratch_dir):
+        self.osutils = osutils
+        self.subprocess_npm = subprocess_npm
+        self.scratch_dir = scratch_dir
+
+    def clean_copy(self, package_dir):
+        target_dir = self.osutils.tempdir(self.scratch_dir)
+
+        package_path = 'file:{}'.format(self.osutils.abspath(package_dir))
+
+        LOG.debug('NODEJS packaging %s to %s', package_path, self.scratch_dir)
+
+        tarfile_name = self.subprocess_npm.run(['pack', '-q', package_path], cwd=self.scratch_dir)
+
+        LOG.debug('NODEJS packed to %s', tarfile_name)
+
+        tarfile_path = self.osutils.joinpath(self.scratch_dir, tarfile_name)
+
+        LOG.debug('NODEJS extracting to %s', target_dir)
+
+        self.osutils.extract_tarfile(tarfile_path, target_dir)
+        return self.osutils.joinpath(target_dir, 'package')
+
+    def is_local_dependency(self, module_path):
+        return module_path.startswith('file:') or module_path.startswith('.') or module_path.startswith('/')
+
+    def get_local_dependencies(self, package_dir, dependency_key='dependencies'):
+        package_json = json.loads(self.osutils.get_text_contents(self.osutils.joinpath(package_dir, 'package.json')))
+        if not dependency_key in package_json.keys():
+            return {}
+
+        dependencies = package_json[dependency_key]
+
+        return dict([(name, module_path) for (name, module_path) in dependencies.items() if self.is_local_dependency(module_path)])
+
+    def has_local_dependencies(self, package_dir):
+        return len(self.get_local_dependencies(package_dir, 'dependencies')) > 0 or \
+            len(self.get_local_dependencies(package_dir, 'optionalDependencies')) > 0
+
+    def pack_to_tar(self, module_dir):
+        package_path = "file:{}".format(self.osutils.abspath(module_dir))
+
+        tarfile_name = self.subprocess_npm.run(['pack', '-q', package_path], cwd=self.scratch_dir)
+
+        return self.osutils.joinpath(self.scratch_dir, tarfile_name)
+
+    def update_dependency(self, package_dir, name, module_path, dependency_key):
+        package_json_path = self.osutils.joinpath(package_dir, 'package.json')
+        package_json = json.loads(self.osutils.get_text_contents(package_json_path))
+
+        package_json[dependency_key][name] = module_path
+
+        self.osutils.write_text_contents(package_json_path, json.dumps(package_json))
+
+    def rewrite_local_dependencies(self, work_dir, original_package_dir):
+        for dependency_key in ['dependencies', 'optionalDependencies']:
+            for (name, module_path) in self.get_local_dependencies(work_dir, dependency_key).items():
+                if module_path.startswith('file:'):
+                    module_path = module_path[5:]
+
+                physical_dir = self.osutils.joinpath(original_package_dir, module_path)
+                if self.has_local_dependencies(physical_dir):
+                    module_path = self.clean_copy(physical_dir)
+                    self.rewrite_local_dependencies(module_path, physical_dir)
+                    physical_dir = module_path
+
+                new_module_path = 'file:{}'.format(self.pack_to_tar(physical_dir))
+                self.update_dependency(work_dir, name, new_module_path, dependency_key)
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index b34863788..73b4653f7 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -7,7 +7,8 @@
 import tarfile
 import subprocess
 import shutil
-
+import io
+import tempfile
 
 class OSUtils(object):
 
@@ -48,3 +49,14 @@ def abspath(self, path):
 
     def is_windows(self):
         return platform.system().lower() == 'windows'
+
+    def get_text_contents(self, filename, encoding='utf-8'):
+        with io.open(filename, 'r', encoding=encoding) as f:
+            return f.read()
+
+    def write_text_contents(self, filename, contents, encoding='utf-8'):
+        with io.open(filename, 'w', encoding=encoding) as f:
+            f.write(contents)
+
+    def tempdir(self, parent_dir):
+        return tempfile.mkdtemp(dir=parent_dir)
diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py
index dc6be8ea4..efa0970bb 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py
@@ -4,9 +4,10 @@
 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, NodejsNpmRewriteLocalDependenciesAction
 from .utils import OSUtils
-from .npm import SubprocessNpm
+from .npm import SubprocessNpm, NpmModulesUtils
 
 
 class NodejsNpmWorkflow(BaseWorkflow):
@@ -62,6 +63,12 @@ def __init__(self,
             npm_pack,
             npm_copy_npmrc,
             CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES),
+            NodejsNpmRewriteLocalDependenciesAction(
+                artifacts_dir,
+                source_dir,
+                tar_dest_dir,
+                npm_modules_utils=NpmModulesUtils(osutils, subprocess_npm, scratch_dir)
+            ),
             npm_install,
             NodejsNpmrcCleanUpAction(artifacts_dir, osutils=osutils)
         ]
diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py
index 01fd52f01..6dcea892b 100644
--- a/tests/functional/workflows/nodejs_npm/test_utils.py
+++ b/tests/functional/workflows/nodejs_npm/test_utils.py
@@ -65,7 +65,7 @@ def test_file_exists_checking_if_file_exists_in_a_dir(self):
         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")
diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
index a8533d5ce..e6a326139 100644
--- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
+++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
@@ -93,3 +93,33 @@ def test_fails_if_package_json_is_broken(self):
                                runtime=self.runtime)
 
         self.assertIn("Unexpected end of JSON input", str(ctx.exception))
+
+    def test_builds_project_with_relative_dependencies(self):
+        source_dir = os.path.join(self.TEST_DATA_FOLDER, "relative-dependencies", "lambda")
+
+        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", "main.js", "node_modules"}
+        output_files = set(os.listdir(self.artifacts_dir))
+        self.assertEquals(expected_files, output_files)
+
+        expected_modules = {"lib"}
+        output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules")))
+        self.assertEquals(expected_modules, output_modules)
+
+    def test_builds_project_with_several_levels_of_relative_dependencies(self):
+        source_dir = os.path.join(self.TEST_DATA_FOLDER, 'relative-dependencies', 'parent_lambda')
+
+        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', 'parent_main.js', 'node_modules'}
+        output_files = set(os.listdir(self.artifacts_dir))
+        self.assertEquals(expected_files, output_files)
+
+        expected_modules = {'lambda', 'lib'}
+        output_modules = set(os.listdir(os.path.join(self.artifacts_dir, 'node_modules')))
+        self.assertEquals(expected_modules, output_modules)
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/main.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/main.js
new file mode 100644
index 000000000..3a185fe95
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/main.js
@@ -0,0 +1,6 @@
+/*global exports,  require */
+const lib = require('lib');
+exports.handler = function (event, context) {
+	'use strict';
+	context.succeed(lib());
+};
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/package.json
new file mode 100644
index 000000000..262bd67d1
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/package.json
@@ -0,0 +1,14 @@
+{
+  "name": "lambda",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "lib": "file:../lib"
+  }
+}
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/index.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/index.js
new file mode 100644
index 000000000..498b8cccc
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/index.js
@@ -0,0 +1,5 @@
+/*global module */
+module.exports = function () {
+	'use strict';
+	return 'hello relative';
+};
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/package.json
new file mode 100644
index 000000000..d702d8735
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/package.json
@@ -0,0 +1,11 @@
+{
+  "name": "lib",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC"
+}
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/package.json
new file mode 100644
index 000000000..99463da80
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/package.json
@@ -0,0 +1,14 @@
+{
+  "name": "parent_lambda",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "lambda": "file:../lambda"
+  }
+}
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/parent_main.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/parent_main.js
new file mode 100644
index 000000000..3a185fe95
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/parent_main.js
@@ -0,0 +1,6 @@
+/*global exports,  require */
+const lib = require('lib');
+exports.handler = function (event, context) {
+	'use strict';
+	context.succeed(lib());
+};
diff --git a/tests/unit/workflows/nodejs_npm/test_npm.py b/tests/unit/workflows/nodejs_npm/test_npm.py
index 8eae867da..c2e3b6bd1 100644
--- a/tests/unit/workflows/nodejs_npm/test_npm.py
+++ b/tests/unit/workflows/nodejs_npm/test_npm.py
@@ -1,7 +1,7 @@
 from unittest import TestCase
 from mock import patch
 
-from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm, NpmExecutionError
+from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm, NpmExecutionError, NpmModulesUtils
 
 
 class FakePopen:
@@ -89,3 +89,57 @@ def test_raises_ValueError_if_args_empty(self):
             self.under_test.run([])
 
         self.assertEqual(raised.exception.args[0], "requires at least one arg")
+
+class TestNpmModulesUtils(TestCase):
+
+    @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils")
+    @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm")
+    def setUp(self, OSUtilMock, SubprocessNpmMock):
+
+        self.osutils = OSUtilMock.return_value
+        self.subprocess_npm = SubprocessNpmMock.return_value
+        self.under_test = NpmModulesUtils(self.osutils, self.subprocess_npm, 'scratch_dir')
+
+    def test_get_local_dependencies_reads_package_json(self): 
+        self.osutils.get_text_contents.side_effect = ['{}']
+        self.osutils.joinpath.side_effect = ['joined/path']
+
+        self.under_test.get_local_dependencies('some/dir')
+
+        self.osutils.get_text_contents.assert_called_with('joined/path')
+        self.osutils.joinpath.assert_called_with('some/dir', 'package.json')
+
+    def test_get_local_dependencies_returns_empty_dict_when_no_dependencies(self):
+        self.osutils.get_text_contents.side_effect = ['{}']
+
+        result = self.under_test.get_local_dependencies('some/dir')
+
+        self.assertEqual(result, {})
+
+    def test_get_local_dependencies_returns_empty_dict_whitout_local_dependencies(self):
+        self.osutils.get_text_contents.side_effect = ['{"dependencies":{"claudia":"*"}}']
+
+        result = self.under_test.get_local_dependencies('some/dir')
+
+        self.assertEqual(result, {})
+
+    def test_get_local_dependencies_return_dict_with_local_dependencies(self):
+        self.osutils.get_text_contents.side_effect = ['{"dependencies":{"claudia":"file:/claudia/"}}']
+
+        result = self.under_test.get_local_dependencies('some/dir')
+
+        self.assertEqual(result, {'claudia': 'file:/claudia/'})
+
+    def test_get_local_dependencies_filters_only_local_dependencies(self):
+        self.osutils.get_text_contents.side_effect = ['{"dependencies":{"claudia":"*","aws-sdk":"*","foo":"file:/foo","bar":"./bar","baz":"/baz"}}']
+
+        result = self.under_test.get_local_dependencies('some/dir')
+
+        self.assertEqual(result, {'foo':'file:/foo','bar':'./bar','baz':'/baz'})
+
+    def test_get_local_dependencies_from_a_defined_key(self):
+        self.osutils.get_text_contents.side_effect = ['{"optionalDependencies":{"claudia":"file:/claudia/"}}']
+
+        result = self.under_test.get_local_dependencies('some/dir', 'optionalDependencies')
+
+        self.assertEqual(result, {'claudia':'file:/claudia/'})
diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py
index c2fe05be3..f8020f67f 100644
--- a/tests/unit/workflows/nodejs_npm/test_workflow.py
+++ b/tests/unit/workflows/nodejs_npm/test_workflow.py
@@ -3,7 +3,7 @@
 from aws_lambda_builders.actions import CopySourceAction
 from aws_lambda_builders.workflows.nodejs_npm.workflow import NodejsNpmWorkflow
 from aws_lambda_builders.workflows.nodejs_npm.actions import \
-    NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction
+    NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction, NodejsNpmRewriteLocalDependenciesAction
 
 
 class TestNodejsNpmWorkflow(TestCase):
@@ -17,7 +17,7 @@ def test_workflow_sets_up_npm_actions(self):
 
         workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest")
 
-        self.assertEqual(len(workflow.actions), 5)
+        self.assertEqual(len(workflow.actions), 6)
 
         self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction)
 
@@ -25,6 +25,8 @@ def test_workflow_sets_up_npm_actions(self):
 
         self.assertIsInstance(workflow.actions[2], CopySourceAction)
 
-        self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction)
+        self.assertIsInstance(workflow.actions[3], NodejsNpmRewriteLocalDependenciesAction)
 
-        self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction)
+        self.assertIsInstance(workflow.actions[4], NodejsNpmInstallAction)
+
+        self.assertIsInstance(workflow.actions[5], NodejsNpmrcCleanUpAction)

From 28a4961d8ce3151280543938699f0c222bc77126 Mon Sep 17 00:00:00 2001
From: Slobodan Stojanovic <slobodan@cloudhorizon.com>
Date: Tue, 29 Jan 2019 12:21:34 +0100
Subject: [PATCH 02/10] Remove package lock from local dependencies

Co-authored-by: Gojko Adzic <gojko@gojko.com>
---
 .../workflows/nodejs_npm/actions.py           | 26 ++++++++++-
 .../workflows/nodejs_npm/npm.py               | 22 +++------
 .../workflows/nodejs_npm/utils.py             |  3 ++
 .../workflows/nodejs_npm/workflow.py          |  3 +-
 .../workflows/nodejs_npm/test_nodejs_npm.py   | 45 +++++++++++++++++++
 .../main_lambda_with_lock_file.js             |  6 +++
 .../lambda_with_lock_file/package-lock.json   | 20 +++++++++
 .../lambda_with_lock_file/package.json        | 14 ++++++
 .../main_lambda_with_tar_dependency.js        |  6 +++
 .../lambda_with_tar_dependency/package.json   | 14 ++++++
 .../main_lambda_without_lock_file.js          |  6 +++
 .../lambda_without_lock_file/package.json     | 14 ++++++
 .../lib_with_lock_file/index.js               |  5 +++
 .../lib_with_lock_file/package-lock.json      | 11 +++++
 .../lib_with_lock_file/package.json           | 14 ++++++
 15 files changed, 190 insertions(+), 19 deletions(-)
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/main_lambda_with_lock_file.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package-lock.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/main_lambda_with_tar_dependency.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/package.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/main_lambda_without_lock_file.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/package.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/index.js
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package-lock.json
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package.json

diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py
index 60f7e7166..8e8114b08 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/actions.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py
@@ -211,16 +211,38 @@ def __init__(
         work_dir,
         original_package_dir,
         scratch_dir,
-        npm_modules_utils
+        npm_modules_utils,
+        osutils
     ):
         super(NodejsNpmRewriteLocalDependenciesAction, self).__init__()
         self.work_dir = work_dir
         self.original_package_dir = original_package_dir
         self.scratch_dir = scratch_dir
         self.npm_modules_utils = npm_modules_utils
+        self.osutils = osutils
+
+    def __rewrite_local_dependencies(self, work_dir, original_package_dir):
+        for dependency_key in ['dependencies', 'optionalDependencies']:
+            for (name, module_path) in self.npm_modules_utils.get_local_dependencies(work_dir, dependency_key).items():
+                if module_path.startswith('file:'):
+                    module_path = module_path[5:]
+
+                actual_path = self.osutils.joinpath(original_package_dir, module_path)
+                if self.osutils.is_dir(actual_path):
+                    if self.npm_modules_utils.has_local_dependencies(actual_path):
+                        module_path = self.npm_modules_utils.clean_copy(actual_path, delete_package_lock=True)
+                        self.__rewrite_local_dependencies(module_path, actual_path)
+                        actual_path = module_path
+
+                    new_module_path = self.npm_modules_utils.pack_to_tar(actual_path)
+                else:
+                    new_module_path = actual_path
+
+                new_module_path = 'file:{}'.format(new_module_path)
+                self.npm_modules_utils.update_dependency(work_dir, name, new_module_path, dependency_key)
 
     def execute(self):
         try:
-            self.npm_modules_utils.rewrite_local_dependencies(self.work_dir, self.original_package_dir)
+            self.__rewrite_local_dependencies(self.work_dir, self.original_package_dir)
         except OSError as ex:
             raise ActionFailedError(str(ex))
diff --git a/aws_lambda_builders/workflows/nodejs_npm/npm.py b/aws_lambda_builders/workflows/nodejs_npm/npm.py
index 309636136..f7d00f5c4 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/npm.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/npm.py
@@ -101,7 +101,7 @@ def __init__(self, osutils, subprocess_npm, scratch_dir):
         self.subprocess_npm = subprocess_npm
         self.scratch_dir = scratch_dir
 
-    def clean_copy(self, package_dir):
+    def clean_copy(self, package_dir, delete_package_lock=False):
         target_dir = self.osutils.tempdir(self.scratch_dir)
 
         package_path = 'file:{}'.format(self.osutils.abspath(package_dir))
@@ -117,6 +117,11 @@ def clean_copy(self, package_dir):
         LOG.debug('NODEJS extracting to %s', target_dir)
 
         self.osutils.extract_tarfile(tarfile_path, target_dir)
+
+        package_lock = self.osutils.joinpath(target_dir, 'package', 'package-lock.json')
+        if delete_package_lock and self.osutils.file_exists(package_lock):
+            self.osutils.remove_file(package_lock)
+
         return self.osutils.joinpath(target_dir, 'package')
 
     def is_local_dependency(self, module_path):
@@ -149,18 +154,3 @@ def update_dependency(self, package_dir, name, module_path, dependency_key):
         package_json[dependency_key][name] = module_path
 
         self.osutils.write_text_contents(package_json_path, json.dumps(package_json))
-
-    def rewrite_local_dependencies(self, work_dir, original_package_dir):
-        for dependency_key in ['dependencies', 'optionalDependencies']:
-            for (name, module_path) in self.get_local_dependencies(work_dir, dependency_key).items():
-                if module_path.startswith('file:'):
-                    module_path = module_path[5:]
-
-                physical_dir = self.osutils.joinpath(original_package_dir, module_path)
-                if self.has_local_dependencies(physical_dir):
-                    module_path = self.clean_copy(physical_dir)
-                    self.rewrite_local_dependencies(module_path, physical_dir)
-                    physical_dir = module_path
-
-                new_module_path = 'file:{}'.format(self.pack_to_tar(physical_dir))
-                self.update_dependency(work_dir, name, new_module_path, dependency_key)
diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py
index 73b4653f7..699147c5d 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/utils.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py
@@ -60,3 +60,6 @@ def write_text_contents(self, filename, contents, encoding='utf-8'):
 
     def tempdir(self, parent_dir):
         return tempfile.mkdtemp(dir=parent_dir)
+
+    def is_dir(self, path):
+        return os.path.isdir(path)
diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py
index efa0970bb..0ceb9abf8 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py
@@ -67,7 +67,8 @@ def __init__(self,
                 artifacts_dir,
                 source_dir,
                 tar_dest_dir,
-                npm_modules_utils=NpmModulesUtils(osutils, subprocess_npm, scratch_dir)
+                npm_modules_utils=NpmModulesUtils(osutils, subprocess_npm, scratch_dir),
+                osutils=osutils
             ),
             npm_install,
             NodejsNpmrcCleanUpAction(artifacts_dir, osutils=osutils)
diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
index e6a326139..ac36d77dd 100644
--- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
+++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
@@ -123,3 +123,48 @@ def test_builds_project_with_several_levels_of_relative_dependencies(self):
         expected_modules = {'lambda', 'lib'}
         output_modules = set(os.listdir(os.path.join(self.artifacts_dir, 'node_modules')))
         self.assertEquals(expected_modules, output_modules)
+
+    def test_builds_project_with_locked_dependencies(self):
+        source_dir = os.path.join(self.TEST_DATA_FOLDER, 'relative-dependencies', 'lambda_with_lock_file')
+
+        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', 'main_lambda_with_lock_file.js', 'node_modules'}
+        output_files = set(os.listdir(self.artifacts_dir))
+        self.assertEquals(expected_files, output_files)
+
+        expected_modules = {'lib', 'lib_with_lock_file'}
+        output_modules = set(os.listdir(os.path.join(self.artifacts_dir, 'node_modules')))
+        self.assertEquals(expected_modules, output_modules)
+
+    def test_builds_project_with_locked_sub_dependencies(self):
+        source_dir = os.path.join(self.TEST_DATA_FOLDER, 'relative-dependencies', 'lambda_without_lock_file')
+
+        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', 'main_lambda_without_lock_file.js', 'node_modules'}
+        output_files = set(os.listdir(self.artifacts_dir))
+        self.assertEquals(expected_files, output_files)
+
+        expected_modules = {'lib', 'lib_with_lock_file'}
+        output_modules = set(os.listdir(os.path.join(self.artifacts_dir, 'node_modules')))
+        self.assertEquals(expected_modules, output_modules)
+
+    def test_builds_project_with_local_file_dependencies(self):
+        source_dir = os.path.join(self.TEST_DATA_FOLDER, 'relative-dependencies', 'lambda_with_tar_dependency')
+
+        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', 'main_lambda_with_tar_dependency.js', 'node_modules'}
+        output_files = set(os.listdir(self.artifacts_dir))
+        self.assertEquals(expected_files, output_files)
+
+        expected_modules = {'lib'}
+        output_modules = set(os.listdir(os.path.join(self.artifacts_dir, 'node_modules')))
+        self.assertEquals(expected_modules, output_modules)
\ No newline at end of file
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/main_lambda_with_lock_file.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/main_lambda_with_lock_file.js
new file mode 100644
index 000000000..3a185fe95
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/main_lambda_with_lock_file.js
@@ -0,0 +1,6 @@
+/*global exports,  require */
+const lib = require('lib');
+exports.handler = function (event, context) {
+	'use strict';
+	context.succeed(lib());
+};
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package-lock.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package-lock.json
new file mode 100644
index 000000000..ad99ce1f8
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package-lock.json
@@ -0,0 +1,20 @@
+{
+  "name": "lambda",
+  "version": "1.0.0",
+  "lockfileVersion": 1,
+  "requires": true,
+  "dependencies": {
+    "lib_with_lock_file": {
+      "version": "file:../INVALID_DIR",
+      "requires": {
+        "lib": "file:../lib_INVALID_DIR"
+      },
+      "dependencies": {
+        "lib": {
+          "version": "file:../lib_INVALID_DIR",
+          "bundled": true
+        }
+      }
+    }
+  }
+}
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package.json
new file mode 100644
index 000000000..4e514c113
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package.json
@@ -0,0 +1,14 @@
+{
+  "name": "lambda",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "lib_with_lock_file": "file:../lib_with_lock_file"
+  }
+}
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/main_lambda_with_tar_dependency.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/main_lambda_with_tar_dependency.js
new file mode 100644
index 000000000..3a185fe95
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/main_lambda_with_tar_dependency.js
@@ -0,0 +1,6 @@
+/*global exports,  require */
+const lib = require('lib');
+exports.handler = function (event, context) {
+	'use strict';
+	context.succeed(lib());
+};
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/package.json
new file mode 100644
index 000000000..58d667219
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/package.json
@@ -0,0 +1,14 @@
+{
+  "name": "lambda",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "lib": "file:../lib-1.0.0.tgz"
+  }
+}
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/main_lambda_without_lock_file.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/main_lambda_without_lock_file.js
new file mode 100644
index 000000000..3a185fe95
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/main_lambda_without_lock_file.js
@@ -0,0 +1,6 @@
+/*global exports,  require */
+const lib = require('lib');
+exports.handler = function (event, context) {
+	'use strict';
+	context.succeed(lib());
+};
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/package.json
new file mode 100644
index 000000000..4e514c113
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/package.json
@@ -0,0 +1,14 @@
+{
+  "name": "lambda",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "lib_with_lock_file": "file:../lib_with_lock_file"
+  }
+}
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/index.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/index.js
new file mode 100644
index 000000000..498b8cccc
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/index.js
@@ -0,0 +1,5 @@
+/*global module */
+module.exports = function () {
+	'use strict';
+	return 'hello relative';
+};
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package-lock.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package-lock.json
new file mode 100644
index 000000000..c47bbdcdd
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package-lock.json
@@ -0,0 +1,11 @@
+{
+  "name": "lib_with_lock_file",
+  "version": "1.0.0",
+  "lockfileVersion": 1,
+  "requires": true,
+  "dependencies": {
+    "lib": {
+      "version": "file:../lib"
+    }
+  }
+}
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package.json
new file mode 100644
index 000000000..244604e6a
--- /dev/null
+++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package.json
@@ -0,0 +1,14 @@
+{
+  "name": "lib_with_lock_file",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "lib": "file:../lib"
+  }
+}

From a0be178b3fa3dfe83eb7ef36b0f13ad4e870cd52 Mon Sep 17 00:00:00 2001
From: Slobodan Stojanovic <slobodan@cloudhorizon.com>
Date: Tue, 19 Mar 2019 11:49:14 +0100
Subject: [PATCH 03/10] Add tests

Co-authored-by: Gojko Adzic <gojko@gojko.com>
---
 .../workflows/nodejs_npm/test_utils.py        |  34 ++++++
 .../workflows/nodejs_npm/test_nodejs_npm.py   |   2 +-
 .../unit/workflows/nodejs_npm/test_actions.py | 101 +++++++++++++++++-
 tests/unit/workflows/nodejs_npm/test_npm.py   |  11 +-
 .../workflows/nodejs_npm/test_workflow.py     |   3 +-
 5 files changed, 143 insertions(+), 8 deletions(-)

diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py
index 6dcea892b..f23c3bd4e 100644
--- a/tests/functional/workflows/nodejs_npm/test_utils.py
+++ b/tests/functional/workflows/nodejs_npm/test_utils.py
@@ -2,6 +2,7 @@
 import shutil
 import sys
 import tempfile
+import io
 
 from unittest import TestCase
 
@@ -127,3 +128,36 @@ 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_reads_file_content(self):
+        scratch_dir = tempfile.mkdtemp()
+        filename = scratch_dir + '/test_write.txt'
+
+        with io.open(filename, 'w', encoding='utf-8') as f:
+            f.write('hello')
+
+        content = self.osutils.get_text_contents(filename)
+        self.assertEqual(content, 'hello')
+
+    def test_writes_text_context(self):
+        scratch_dir = tempfile.mkdtemp()
+        filename = scratch_dir + '/test_write.txt'
+        self.osutils.write_text_contents(filename, 'hello')
+        with io.open(filename, 'r', encoding='utf-8') as f:
+            content = f.read()
+            self.assertEqual(content, 'hello')
+
+    def test_should_create_temporary_dir(self):
+        scratch_dir = tempfile.mkdtemp()
+        temp_dir = self.osutils.tempdir(scratch_dir)
+        os.path.isdir(temp_dir)
+
+    def test_is_dir_returns_true_if_dir_exists(self):
+        dir_exists = self.osutils.is_dir(os.path.dirname(__file__))
+
+        self.assertTrue(dir_exists)
+
+    def test_is_dir_returns_false_if_dir_does_not_exist(self):
+        dir_does_not_exist = self.osutils.is_dir(os.path.join(os.path.dirname(__file__), 'some_random_dirname'))
+
+        self.assertFalse(dir_does_not_exist)
diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
index ac36d77dd..405188ebc 100644
--- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
+++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
@@ -167,4 +167,4 @@ def test_builds_project_with_local_file_dependencies(self):
 
         expected_modules = {'lib'}
         output_modules = set(os.listdir(os.path.join(self.artifacts_dir, 'node_modules')))
-        self.assertEquals(expected_modules, output_modules)
\ No newline at end of file
+        self.assertEquals(expected_modules, output_modules)
diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py
index 612ccb189..88f27aa2b 100644
--- a/tests/unit/workflows/nodejs_npm/test_actions.py
+++ b/tests/unit/workflows/nodejs_npm/test_actions.py
@@ -1,9 +1,10 @@
 from unittest import TestCase
-from mock import patch
+from mock import patch, call
 
 from aws_lambda_builders.actions import ActionFailedError
 from aws_lambda_builders.workflows.nodejs_npm.actions import \
-    NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction
+    NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, \
+    NodejsNpmrcCleanUpAction, NodejsNpmRewriteLocalDependenciesAction
 from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError
 
 
@@ -153,3 +154,99 @@ def test_skips_npmrc_removal_if_npmrc_doesnt_exist(self, OSUtilMock):
         action.execute()
 
         osutils.remove_file.assert_not_called()
+
+
+class TestNodejsNpmRewriteLocalDependenciesAction(TestCase):
+
+    @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils")
+    @patch("aws_lambda_builders.workflows.nodejs_npm.npm.NpmModulesUtils")
+    def setUp(self, OSUtilMock, NpmModulesUtils):
+        self.osutils = OSUtilMock.return_value
+        self.osutils.joinpath.side_effect = \
+            lambda original_package_dir, module_path: original_package_dir + '/' + module_path
+
+        self.npm_modules_utils = NpmModulesUtils.return_value
+        self.npm_modules_utils.pack_to_tar.side_effect = lambda actual_path: actual_path + '.tar'
+        self.npm_modules_utils.clean_copy.side_effect = lambda actual_path, delete_package_lock: actual_path + '_copy'
+
+        self.action = NodejsNpmRewriteLocalDependenciesAction(
+            'work_dir',
+            'original_package_dir',
+            'scratch_dir',
+            self.npm_modules_utils,
+            self.osutils
+        )
+
+    def test_does_not_rewrite_if_no_local_dependencies(self):
+        self.npm_modules_utils.get_local_dependencies.side_effect = [{}, {}]
+
+        self.action.execute()
+
+        self.npm_modules_utils.update_dependency.assert_not_called()
+
+    def test_rewrites_a_single_production_dependency(self):
+        def local_deps(work_dir, dependency_key):
+            if work_dir == "work_dir" and dependency_key == "dependencies":
+                return {"dep": "../dep"}
+            else:
+                return {}
+
+        self.npm_modules_utils.get_local_dependencies.side_effect = local_deps
+
+        self.action.execute()
+
+        self.npm_modules_utils.update_dependency.assert_called_with(
+            'work_dir', 'dep', 'file:original_package_dir/../dep_copy.tar', 'dependencies'
+        )
+
+    def test_rewrites_a_single_optional_dependency(self):
+        def local_deps(work_dir, dependency_key):
+            if work_dir == "work_dir" and dependency_key == "optionalDependencies":
+                return {"dep": "../opt_dep"}
+            else:
+                return {}
+
+        self.npm_modules_utils.get_local_dependencies.side_effect = local_deps
+
+        self.action.execute()
+
+        self.npm_modules_utils.update_dependency.assert_called_with(
+            'work_dir', 'dep', 'file:original_package_dir/../opt_dep_copy.tar', 'optionalDependencies'
+        )
+
+    def test_strips_file_prefix_when_rewriting_dependencies(self):
+        def local_deps(work_dir, dependency_key):
+            if work_dir == "work_dir" and dependency_key == "dependencies":
+                return {"dep": "file:/dep"}
+            else:
+                return {}
+
+        self.npm_modules_utils.get_local_dependencies.side_effect = local_deps
+
+        self.action.execute()
+
+        self.osutils.joinpath.assert_called_with('original_package_dir', '/dep')
+
+    def test_rewrites_production_dependencies_recursively(self):
+        def local_deps(work_dir, dependency_key):
+            if work_dir == 'work_dir' and dependency_key == 'dependencies':
+                return {"dep": "../dep"}
+            elif work_dir == 'original_package_dir/../dep_copy' and dependency_key == 'dependencies':
+                return {"subdep": "../subdep"}
+            else:
+                return {}
+
+        self.npm_modules_utils.get_local_dependencies.side_effect = local_deps
+
+        self.action.execute()
+
+        calls = [
+            call(
+                'original_package_dir/../dep_copy',
+                'subdep',
+                'file:original_package_dir/../dep/../subdep_copy.tar',
+                'dependencies'
+            ),
+            call('work_dir', 'dep', 'file:original_package_dir/../dep_copy.tar', 'dependencies')
+        ]
+        self.npm_modules_utils.update_dependency.assert_has_calls(calls)
diff --git a/tests/unit/workflows/nodejs_npm/test_npm.py b/tests/unit/workflows/nodejs_npm/test_npm.py
index c2e3b6bd1..5e2c44ae0 100644
--- a/tests/unit/workflows/nodejs_npm/test_npm.py
+++ b/tests/unit/workflows/nodejs_npm/test_npm.py
@@ -90,6 +90,7 @@ def test_raises_ValueError_if_args_empty(self):
 
         self.assertEqual(raised.exception.args[0], "requires at least one arg")
 
+
 class TestNpmModulesUtils(TestCase):
 
     @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils")
@@ -100,7 +101,7 @@ def setUp(self, OSUtilMock, SubprocessNpmMock):
         self.subprocess_npm = SubprocessNpmMock.return_value
         self.under_test = NpmModulesUtils(self.osutils, self.subprocess_npm, 'scratch_dir')
 
-    def test_get_local_dependencies_reads_package_json(self): 
+    def test_get_local_dependencies_reads_package_json(self):
         self.osutils.get_text_contents.side_effect = ['{}']
         self.osutils.joinpath.side_effect = ['joined/path']
 
@@ -131,15 +132,17 @@ def test_get_local_dependencies_return_dict_with_local_dependencies(self):
         self.assertEqual(result, {'claudia': 'file:/claudia/'})
 
     def test_get_local_dependencies_filters_only_local_dependencies(self):
-        self.osutils.get_text_contents.side_effect = ['{"dependencies":{"claudia":"*","aws-sdk":"*","foo":"file:/foo","bar":"./bar","baz":"/baz"}}']
+        self.osutils.get_text_contents.side_effect = [
+            '{"dependencies":{"claudia":"*","aws-sdk":"*","foo":"file:/foo","bar":"./bar","baz":"/baz"}}'
+        ]
 
         result = self.under_test.get_local_dependencies('some/dir')
 
-        self.assertEqual(result, {'foo':'file:/foo','bar':'./bar','baz':'/baz'})
+        self.assertEqual(result, {'foo': 'file:/foo', 'bar': './bar', 'baz': '/baz'})
 
     def test_get_local_dependencies_from_a_defined_key(self):
         self.osutils.get_text_contents.side_effect = ['{"optionalDependencies":{"claudia":"file:/claudia/"}}']
 
         result = self.under_test.get_local_dependencies('some/dir', 'optionalDependencies')
 
-        self.assertEqual(result, {'claudia':'file:/claudia/'})
+        self.assertEqual(result, {'claudia': 'file:/claudia/'})
diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py
index f8020f67f..5f3d29d71 100644
--- a/tests/unit/workflows/nodejs_npm/test_workflow.py
+++ b/tests/unit/workflows/nodejs_npm/test_workflow.py
@@ -3,7 +3,8 @@
 from aws_lambda_builders.actions import CopySourceAction
 from aws_lambda_builders.workflows.nodejs_npm.workflow import NodejsNpmWorkflow
 from aws_lambda_builders.workflows.nodejs_npm.actions import \
-    NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction, NodejsNpmRewriteLocalDependenciesAction
+    NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, \
+    NodejsNpmrcCleanUpAction, NodejsNpmRewriteLocalDependenciesAction
 
 
 class TestNodejsNpmWorkflow(TestCase):

From 3fb4d40bb5ac2ea0e1804894e03f65cd2b5e07fe Mon Sep 17 00:00:00 2001
From: Gojko Adzic <gojko@gojko.com>
Date: Sat, 23 Mar 2019 23:24:22 +0100
Subject: [PATCH 04/10] docs and linting

---
 .../workflows/nodejs_npm/npm.py               | 89 +++++++++++++++++--
 1 file changed, 84 insertions(+), 5 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/npm.py b/aws_lambda_builders/workflows/nodejs_npm/npm.py
index f7d00f5c4..7827682e2 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/npm.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/npm.py
@@ -90,18 +90,44 @@ def run(self, args, cwd=None):
 
         return out.decode('utf8').strip()
 
+
 class NpmModulesUtils(object):
 
     """
-
+    Utility class that abstracts operations on NPM packages
+    and manifest files
     """
 
     def __init__(self, osutils, subprocess_npm, scratch_dir):
+        """
+        :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils
+        :param osutils: An instance of OS Utilities for file manipulation
+
+        :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm
+        :param subprocess_npm: An instance of NPM Subprocess for executing the NPM binary
+
+        :type scratch_dir: str
+        :param scratch_dir: a writable temporary directory
+        """
+
         self.osutils = osutils
         self.subprocess_npm = subprocess_npm
         self.scratch_dir = scratch_dir
 
     def clean_copy(self, package_dir, delete_package_lock=False):
+
+        """
+        Produces a clean copy of a NPM package source from a project directory,
+        so it can be packaged without temporary files, development or test resources
+        or dependencies.
+
+        :type package_dir: str
+        :param package_dir: Path to a NPM project directory
+
+        :type delete_package_lock: bool
+        :param delete_package_lock: If true, package-lock.json will be removed in the copy
+        """
+
         target_dir = self.osutils.tempdir(self.scratch_dir)
 
         package_path = 'file:{}'.format(self.osutils.abspath(package_dir))
@@ -125,29 +151,82 @@ def clean_copy(self, package_dir, delete_package_lock=False):
         return self.osutils.joinpath(target_dir, 'package')
 
     def is_local_dependency(self, module_path):
+        """
+        Calculates if the module path from a dependency reference is
+        local or remote
+
+        :type module_path: str
+        :param module_path: Dependency reference value (from package.json)
+
+        """
+
         return module_path.startswith('file:') or module_path.startswith('.') or module_path.startswith('/')
 
     def get_local_dependencies(self, package_dir, dependency_key='dependencies'):
+        """
+        Returns a dictionary with only local dependencies from a package.json manifest
+
+        :type package_dir: str
+        :param package_dir: path to a NPM project directory (containing package.json)
+
+        :type dependency_key: str
+        :param dependency_key: dependency type to return, corresponds to a key of package.json.
+            (for example, 'dependencies' or 'optionalDependencies')
+        """
+
         package_json = json.loads(self.osutils.get_text_contents(self.osutils.joinpath(package_dir, 'package.json')))
-        if not dependency_key in package_json.keys():
+        if dependency_key not in package_json.keys():
             return {}
 
         dependencies = package_json[dependency_key]
 
-        return dict([(name, module_path) for (name, module_path) in dependencies.items() if self.is_local_dependency(module_path)])
+        return dict(
+            [(name, module_path) for (name, module_path) in dependencies.items()
+                if self.is_local_dependency(module_path)]
+        )
 
     def has_local_dependencies(self, package_dir):
+        """
+        Checks if a NPM project has local dependencies
+
+        :type package_dir: str
+        :param package_dir: path to a NPM project directory (containing package.json)
+        """
         return len(self.get_local_dependencies(package_dir, 'dependencies')) > 0 or \
             len(self.get_local_dependencies(package_dir, 'optionalDependencies')) > 0
 
-    def pack_to_tar(self, module_dir):
-        package_path = "file:{}".format(self.osutils.abspath(module_dir))
+    def pack_to_tar(self, package_dir):
+        """
+        Runs npm pack to produce a tar containing project sources, which can be used
+        as a target for project dependencies
+
+        :type package_dir: str
+        :param package_dir: path to a NPM project directory (containing package.json)
+        """
+        package_path = "file:{}".format(self.osutils.abspath(package_dir))
 
         tarfile_name = self.subprocess_npm.run(['pack', '-q', package_path], cwd=self.scratch_dir)
 
         return self.osutils.joinpath(self.scratch_dir, tarfile_name)
 
     def update_dependency(self, package_dir, name, module_path, dependency_key):
+        """
+        Updates package.json by rewriting a dependency to point to a specified module path
+
+        :type package_dir: str
+        :param package_dir: path to a NPM project directory (containing package.json)
+
+        :type name: str
+        :param name: the name of the dependency (sub-key in package.json)
+
+        :type module_path: str
+        :param module_path: new destination for the dependency
+
+        :type dependency_key: str
+        :param dependency_key: dependency type to return, corresponds to a key of package.json.
+            (for example, 'dependencies' or 'optionalDependencies')
+        """
+
         package_json_path = self.osutils.joinpath(package_dir, 'package.json')
         package_json = json.loads(self.osutils.get_text_contents(package_json_path))
 

From c97ee731997dd5f57014066443015a16f31c7bf3 Mon Sep 17 00:00:00 2001
From: Gojko Adzic <gojko@gojko.com>
Date: Sun, 24 Mar 2019 00:42:12 +0100
Subject: [PATCH 05/10] tests for npm package utils

---
 .../workflows/nodejs_npm/npm.py               |   7 +-
 tests/unit/workflows/nodejs_npm/test_npm.py   | 155 ++++++++++++++++++
 2 files changed, 160 insertions(+), 2 deletions(-)

diff --git a/aws_lambda_builders/workflows/nodejs_npm/npm.py b/aws_lambda_builders/workflows/nodejs_npm/npm.py
index 7827682e2..93e3c103e 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/npm.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/npm.py
@@ -160,7 +160,10 @@ def is_local_dependency(self, module_path):
 
         """
 
-        return module_path.startswith('file:') or module_path.startswith('.') or module_path.startswith('/')
+        return module_path.startswith('file:') or \
+            module_path.startswith('.') or \
+            module_path.startswith('/') or \
+            module_path.startswith('~/')
 
     def get_local_dependencies(self, package_dir, dependency_key='dependencies'):
         """
@@ -209,7 +212,7 @@ def pack_to_tar(self, package_dir):
 
         return self.osutils.joinpath(self.scratch_dir, tarfile_name)
 
-    def update_dependency(self, package_dir, name, module_path, dependency_key):
+    def update_dependency(self, package_dir, name, module_path, dependency_key='dependencies'):
         """
         Updates package.json by rewriting a dependency to point to a specified module path
 
diff --git a/tests/unit/workflows/nodejs_npm/test_npm.py b/tests/unit/workflows/nodejs_npm/test_npm.py
index 5e2c44ae0..abdeb57a9 100644
--- a/tests/unit/workflows/nodejs_npm/test_npm.py
+++ b/tests/unit/workflows/nodejs_npm/test_npm.py
@@ -1,5 +1,6 @@
 from unittest import TestCase
 from mock import patch
+from parameterized import parameterized
 
 from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm, NpmExecutionError, NpmModulesUtils
 
@@ -100,6 +101,9 @@ def setUp(self, OSUtilMock, SubprocessNpmMock):
         self.osutils = OSUtilMock.return_value
         self.subprocess_npm = SubprocessNpmMock.return_value
         self.under_test = NpmModulesUtils(self.osutils, self.subprocess_npm, 'scratch_dir')
+        self.osutils.joinpath.side_effect = lambda a, b, c='': a + '/' + b + '/' + c if c else a + '/' + b
+        self.osutils.abspath.side_effect = lambda a: '/absolute/' + a
+        self.osutils.tempdir.side_effect = lambda a: a + '/tempdir'
 
     def test_get_local_dependencies_reads_package_json(self):
         self.osutils.get_text_contents.side_effect = ['{}']
@@ -146,3 +150,154 @@ def test_get_local_dependencies_from_a_defined_key(self):
         result = self.under_test.get_local_dependencies('some/dir', 'optionalDependencies')
 
         self.assertEqual(result, {'claudia': 'file:/claudia/'})
+
+    @parameterized.expand([
+        # NPM Version numbers and ranges from https://docs.npmjs.com/files/package.json
+        "1.0.0 - 2.9999.9999",
+        ">=1.0.2 <2.1.2",
+        "<1.0.2",
+        "<=1.0.2",
+        ">1.0.2 <=2.3.4",
+        "2.0.1",
+        "<1.0.0 || >=2.3.1 <2.4.5 || >=2.5.2 <3.0.0",
+        "~1.2",
+        "~1.2.3",
+        "2.x",
+        "3.3.x",
+        "latest",
+        "*",
+        # URLs as Dependencies
+        "http://asdf.com/asdf.tar.gz",
+        # Git URLs as Dependencies
+        "git+ssh://git@github.com:npm/cli.git#v1.0.27"
+        "git+ssh://git@github.com:npm/cli#semver:^5.0"
+        "git+https://isaacs@github.com/npm/cli.git"
+        "git://github.com/npm/cli.git#v1.0.27"
+        # GitHub URLs
+        "expressjs/express",
+        "mochajs/mocha#4727d357ea",
+        "user/repo#feature\/branch"
+    ])
+    def test_is_local_dependency_returns_false_for_remote_dependencies(self, dependency_ref):
+        print('using', dependency_ref)
+        self.assertFalse(self.under_test.is_local_dependency(dependency_ref))
+
+    @parameterized.expand([
+        "../foo/bar",
+        "~/foo/bar",
+        "./foo/bar",
+        "/foo/bar",
+        "file:../foo/bar"
+    ])
+    def test_is_local_dependency_returns_true_for_local_dependencies(self, dependency_ref):
+        self.assertTrue(self.under_test.is_local_dependency(dependency_ref))
+
+    def test_has_local_dependencies_returns_false_if_no_dependencies(self):
+        self.osutils.get_text_contents.side_effect = lambda path: '{"name":"test-project"}'
+
+        self.assertFalse(self.under_test.has_local_dependencies('project_path'))
+        self.osutils.get_text_contents.assert_called_with('project_path/package.json')
+
+    @parameterized.expand([
+        '{"optionalDependencies":{"claudia":"1.0.0"}}',
+        '{"dependencies":{"claudia":"1.0.0"}}',
+        '{"dependencies":{"claudia":"1.0.0"}, "optionalDependencies":{"express":"*"}}',
+    ])
+    def test_has_local_dependencies_returns_false_if_only_remote_dependencies(self, package_manifest):
+        self.osutils.get_text_contents.side_effect = lambda path: package_manifest
+
+        self.assertFalse(self.under_test.has_local_dependencies('project_path'))
+
+    def test_has_local_dependencies_ignores_development_dependencies(self):
+        package_manifest = '{"devDependencies":{"claudia":"file:../claudia"}}'
+        self.osutils.get_text_contents.side_effect = lambda path: package_manifest
+
+        self.assertFalse(self.under_test.has_local_dependencies('project_path'))
+
+    @parameterized.expand([
+        '{"optionalDependencies":{"claudia":"file:../local"}}',
+        '{"optionalDependencies":{"express": "*", "claudia":"file:../local"}}',
+        '{"dependencies":{"claudia":"file:../local"}}',
+        '{"dependencies":{"express": "*", "claudia":"file:../local"}}',
+        '{"dependencies":{"claudia":"file:../local"}, "optionalDependencies":{"express":"*"}}',
+        '{"dependencies":{"claudia":"*"}, "optionalDependencies":{"express":"file:../local"}}',
+    ])
+    def test_has_local_dependencies_returns_false_if_some_local_dependencies(self, package_manifest):
+        self.osutils.get_text_contents.side_effect = lambda path: package_manifest
+
+        self.assertTrue(self.under_test.has_local_dependencies('project_path'))
+
+    def test_pack_to_tar_uses_subprocess_npm_to_execute_npm_pack(self):
+        self.subprocess_npm.run.side_effect = ['archive.tar']
+        result = self.under_test.pack_to_tar('package_dir')
+        self.assertEquals(result, 'scratch_dir/archive.tar')
+        self.subprocess_npm.run.assert_called_with(['pack', '-q', 'file:/absolute/package_dir'], cwd='scratch_dir')
+
+    def test_update_dependency_rewrites_production_dependencies_without_specific_dependency_key(self):
+        package_manifest = '{"dependencies":{"claudia":"file:../claudia"}}'
+        self.osutils.get_text_contents.side_effect = lambda path: package_manifest
+        self.under_test.update_dependency('package_dir', 'claudia', 'file:/a.tar')
+        self.osutils.get_text_contents.assert_called_with('package_dir/package.json')
+        self.osutils.write_text_contents.assert_called_with(
+            'package_dir/package.json',
+            '{"dependencies": {"claudia": "file:/a.tar"}}'
+        )
+
+    def test_update_dependency_rewrites_production_dependencies_does_not_modify_other_dependencies(self):
+        package_manifest = '{"dependencies":{"express": "*", "claudia":"file:../claudia"}}'
+        self.osutils.get_text_contents.side_effect = lambda path: package_manifest
+        self.under_test.update_dependency('package_dir', 'claudia', 'file:/a.tar')
+        self.osutils.get_text_contents.assert_called_with('package_dir/package.json')
+        self.osutils.write_text_contents.assert_called_with(
+            'package_dir/package.json',
+            '{"dependencies": {"express": "*", "claudia": "file:/a.tar"}}'
+        )
+
+    def test_update_dependency_rewrites_production_dependencies_with_specific_dependency_key(self):
+        package_manifest = '{"dependencies":{"claudia":"file:../claudia"}}'
+        self.osutils.get_text_contents.side_effect = lambda path: package_manifest
+        self.under_test.update_dependency('package_dir', 'claudia', 'file:/a.tar', 'dependencies')
+        self.osutils.get_text_contents.assert_called_with('package_dir/package.json')
+        self.osutils.write_text_contents.assert_called_with(
+            'package_dir/package.json',
+            '{"dependencies": {"claudia": "file:/a.tar"}}'
+        )
+
+    def test_update_dependency_rewrites_optional_dependencies_if_requested(self):
+        package_manifest = '{"optionalDependencies":{"claudia":"file:../claudia"}}'
+        self.osutils.get_text_contents.side_effect = lambda path: package_manifest
+        self.under_test.update_dependency('package_dir', 'claudia', 'file:/a.tar', 'optionalDependencies')
+        self.osutils.get_text_contents.assert_called_with('package_dir/package.json')
+        self.osutils.write_text_contents.assert_called_with(
+            'package_dir/package.json',
+            '{"optionalDependencies": {"claudia": "file:/a.tar"}}'
+        )
+
+    def test_clean_copy_packs_a_project_to_tar_and_extracts_from_tar_into_temp_dir(self):
+        self.subprocess_npm.run.side_effect = ['archive.tar']
+
+        result = self.under_test.clean_copy('project_dir')
+
+        self.assertEquals(result, 'scratch_dir/tempdir/package')
+        self.subprocess_npm.run.assert_called_with(['pack', '-q', 'file:/absolute/project_dir'], cwd='scratch_dir')
+        self.osutils.extract_tarfile.assert_called_with('scratch_dir/archive.tar', 'scratch_dir/tempdir')
+
+    def test_clean_copy_removes_package_lock_if_requested_and_exists(self):
+        self.subprocess_npm.run.side_effect = ['archive.tar']
+        self.osutils.file_exists.side_effect = [True]
+
+        result = self.under_test.clean_copy('project_dir', True)
+
+        self.assertEquals(result, 'scratch_dir/tempdir/package')
+        self.osutils.file_exists.assert_called_with('scratch_dir/tempdir/package/package-lock.json')
+        self.osutils.remove_file.assert_called_with('scratch_dir/tempdir/package/package-lock.json')
+
+    def test_clean_copy_does_not_removes_package_lock_if_it_does_not_exist_even_if_requested(self):
+        self.subprocess_npm.run.side_effect = ['archive.tar']
+        self.osutils.file_exists.side_effect = [False]
+
+        result = self.under_test.clean_copy('project_dir', True)
+
+        self.assertEquals(result, 'scratch_dir/tempdir/package')
+        self.osutils.file_exists.assert_called_with('scratch_dir/tempdir/package/package-lock.json')
+        self.osutils.remove_file.assert_not_called()

From add4d273245bc97cf0615fa8df4ef09d32de43a1 Mon Sep 17 00:00:00 2001
From: Slobodan Stojanovic <slobodan@cloudhorizon.com>
Date: Sun, 24 Mar 2019 21:46:09 +0100
Subject: [PATCH 06/10] Minor cleanup

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

diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py
index f23c3bd4e..a96aeb5c0 100644
--- a/tests/functional/workflows/nodejs_npm/test_utils.py
+++ b/tests/functional/workflows/nodejs_npm/test_utils.py
@@ -131,7 +131,7 @@ def test_popen_can_accept_cwd(self):
 
     def test_reads_file_content(self):
         scratch_dir = tempfile.mkdtemp()
-        filename = scratch_dir + '/test_write.txt'
+        filename = os.path.join(scratch_dir, 'test_write.txt')
 
         with io.open(filename, 'w', encoding='utf-8') as f:
             f.write('hello')
@@ -141,7 +141,7 @@ def test_reads_file_content(self):
 
     def test_writes_text_context(self):
         scratch_dir = tempfile.mkdtemp()
-        filename = scratch_dir + '/test_write.txt'
+        filename = os.path.join(scratch_dir, 'test_write.txt')
         self.osutils.write_text_contents(filename, 'hello')
         with io.open(filename, 'r', encoding='utf-8') as f:
             content = f.read()

From 94e08f388de3fb83174bd2c082f2c20aa615f6f6 Mon Sep 17 00:00:00 2001
From: Slobodan Stojanovic <slobodan@cloudhorizon.com>
Date: Sun, 24 Mar 2019 22:51:43 +0100
Subject: [PATCH 07/10] Add tgz file to fix tests

---
 .gitignore                                        |   1 +
 .../testdata/relative-dependencies/lib-1.0.0.tgz  | Bin 0 -> 336 bytes
 tests/unit/workflows/nodejs_npm/test_npm.py       |   4 ++--
 3 files changed, 3 insertions(+), 2 deletions(-)
 create mode 100644 tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib-1.0.0.tgz

diff --git a/.gitignore b/.gitignore
index ab4e9c47e..033a02778 100644
--- a/.gitignore
+++ b/.gitignore
@@ -161,6 +161,7 @@ typings/
 
 # Output of 'npm pack'
 *.tgz
+!tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib-1.0.0.tgz
 
 # Except test file
 !tests/functional/workflows/ruby_bundler/test_data/test.tgz
diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib-1.0.0.tgz b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib-1.0.0.tgz
new file mode 100644
index 0000000000000000000000000000000000000000..e1b7eeb5d0b3ec059efd3d442a04c857f2aeace7
GIT binary patch
literal 336
zcmV-W0k8faiwFP!000006YY~dZ^AGThO?4iaq<8ysuG9LL;`iHI&|*JVv+;g8auKb
zgjDh0Yh&6<6%s?~2U4EJ_ItMP{P8xddSOdUo6siDM#<>RP)dvO7>?<loR$z}`FNBS
z#cf`e5zVrK<`B_y7jlf&Sf*Vm-`yvi1wPsf;WY*zlC6<UfCxS(H(uDFHe5-sWeH7a
zP-`@m=8Zi>g1BaUKjN~+t@}Pf4HJzE_ykU{Xsn-x)k?vPJZP=-1f&8V0MlT_7aZ#u
zfqoxw%PnNYt=#Q*W362&{h3qn3Z>bX{P=WF;$8gL*LblUe4oBQ-%sQ?;r}czzWSe*
z#i;ZDZ;%d`Ld}_gwW?czFihjnOK{sLZ4KPPqLr05JzQNoJNE{yK`>VH$_}P+Por(M
igux1hP@qvT%QxsEyJ=_3j*gCwf6E&iR^+V!3;+OPuc1Q#

literal 0
HcmV?d00001

diff --git a/tests/unit/workflows/nodejs_npm/test_npm.py b/tests/unit/workflows/nodejs_npm/test_npm.py
index abdeb57a9..253ceca1c 100644
--- a/tests/unit/workflows/nodejs_npm/test_npm.py
+++ b/tests/unit/workflows/nodejs_npm/test_npm.py
@@ -244,13 +244,13 @@ def test_update_dependency_rewrites_production_dependencies_without_specific_dep
         )
 
     def test_update_dependency_rewrites_production_dependencies_does_not_modify_other_dependencies(self):
-        package_manifest = '{"dependencies":{"express": "*", "claudia":"file:../claudia"}}'
+        package_manifest = '{"dependencies":{"claudia":"file:../claudia", "express": "*"}}'
         self.osutils.get_text_contents.side_effect = lambda path: package_manifest
         self.under_test.update_dependency('package_dir', 'claudia', 'file:/a.tar')
         self.osutils.get_text_contents.assert_called_with('package_dir/package.json')
         self.osutils.write_text_contents.assert_called_with(
             'package_dir/package.json',
-            '{"dependencies": {"express": "*", "claudia": "file:/a.tar"}}'
+            '{"dependencies": {"claudia": "file:/a.tar", "express": "*"}}'
         )
 
     def test_update_dependency_rewrites_production_dependencies_with_specific_dependency_key(self):

From c43b5371484f49e8464e9f88093df8eaed662b90 Mon Sep 17 00:00:00 2001
From: Slobodan Stojanovic <slobodan@cloudhorizon.com>
Date: Sun, 24 Mar 2019 23:12:26 +0100
Subject: [PATCH 08/10] Fix python 2.7 test

---
 tests/functional/workflows/nodejs_npm/test_utils.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py
index a96aeb5c0..23796dab3 100644
--- a/tests/functional/workflows/nodejs_npm/test_utils.py
+++ b/tests/functional/workflows/nodejs_npm/test_utils.py
@@ -134,7 +134,7 @@ def test_reads_file_content(self):
         filename = os.path.join(scratch_dir, 'test_write.txt')
 
         with io.open(filename, 'w', encoding='utf-8') as f:
-            f.write('hello')
+            f.write(u'hello')
 
         content = self.osutils.get_text_contents(filename)
         self.assertEqual(content, 'hello')

From 6523d9b939b99128f5fee759f60320d3ca37cb63 Mon Sep 17 00:00:00 2001
From: Slobodan Stojanovic <slobodan@cloudhorizon.com>
Date: Sun, 24 Mar 2019 23:21:25 +0100
Subject: [PATCH 09/10] Fix python 2.7 test

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

diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py
index 23796dab3..5c7c0e55c 100644
--- a/tests/functional/workflows/nodejs_npm/test_utils.py
+++ b/tests/functional/workflows/nodejs_npm/test_utils.py
@@ -142,10 +142,10 @@ def test_reads_file_content(self):
     def test_writes_text_context(self):
         scratch_dir = tempfile.mkdtemp()
         filename = os.path.join(scratch_dir, 'test_write.txt')
-        self.osutils.write_text_contents(filename, 'hello')
+        self.osutils.write_text_contents(filename, u'hello')
         with io.open(filename, 'r', encoding='utf-8') as f:
             content = f.read()
-            self.assertEqual(content, 'hello')
+            self.assertEqual(content, u'hello')
 
     def test_should_create_temporary_dir(self):
         scratch_dir = tempfile.mkdtemp()

From 1934e41796cae0feccd6b471dd6de1a3395bcc20 Mon Sep 17 00:00:00 2001
From: Gojko Adzic <gojko@gojko.com>
Date: Mon, 25 Mar 2019 14:16:17 +0100
Subject: [PATCH 10/10] ensure json is unicode for python 2.7 madness

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

diff --git a/aws_lambda_builders/workflows/nodejs_npm/npm.py b/aws_lambda_builders/workflows/nodejs_npm/npm.py
index 93e3c103e..fb921a06f 100644
--- a/aws_lambda_builders/workflows/nodejs_npm/npm.py
+++ b/aws_lambda_builders/workflows/nodejs_npm/npm.py
@@ -234,5 +234,5 @@ def update_dependency(self, package_dir, name, module_path, dependency_key='depe
         package_json = json.loads(self.osutils.get_text_contents(package_json_path))
 
         package_json[dependency_key][name] = module_path
-
-        self.osutils.write_text_contents(package_json_path, json.dumps(package_json))
+        package_json_contents = json.dumps(package_json, ensure_ascii=False)
+        self.osutils.write_text_contents(package_json_path, package_json_contents)