From d842c6c8875d983a03cf01c2cd310b1c06d469d7 Mon Sep 17 00:00:00 2001
From: Kyle Finley <kyle@finley.sh>
Date: Thu, 21 May 2020 15:16:04 -0700
Subject: [PATCH] the friendly error when npm can't be found has returned

---
 CHANGELOG.md                |  3 +++
 runway/module/__init__.py   |  2 +-
 tests/module/test_module.py | 22 +++++++++++++++++-----
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a09650516..695aa3c65 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - `runway test` will now return a non-zero exit code if any non-required tests failed
 - `static-react` sample uses npm instead of yarn
 
+### Fixed
+- the friendly error when npm can't be found has returned
+
 ## [1.8.0] - 2020-05-16
 ### Fixed
 - the value of `environments` is once again used to determine if a serverless module should be skipped
diff --git a/runway/module/__init__.py b/runway/module/__init__.py
index b4cb5b827..9a798738a 100644
--- a/runway/module/__init__.py
+++ b/runway/module/__init__.py
@@ -163,7 +163,6 @@ def __init__(self, context, path, options=None):
                 definition.
 
         """
-        self.check_for_npm()  # fail fast
         options = options or {}
         super(RunwayModuleNpm, self).__init__(context, path, options)
         del self.options  # remove the attr set by the parent class
@@ -178,6 +177,7 @@ def __init__(self, context, path, options=None):
         for k, v in options.items():
             setattr(self, k, v)
 
+        self.check_for_npm()  # fail fast
         warn_on_boto_env_vars(self.context.env_vars)
 
     def check_for_npm(self):
diff --git a/tests/module/test_module.py b/tests/module/test_module.py
index 2911b0189..1671c8d6c 100644
--- a/tests/module/test_module.py
+++ b/tests/module/test_module.py
@@ -42,22 +42,24 @@ def test_check_for_npm(self, mock_which, caplog, runway_context):
                 'please ensure it is installed correctly.'] == \
             caplog.messages
 
-    @patch.object(RunwayModuleNpm, 'check_for_npm', MagicMock(return_value=None))
+    @patch('runway.module.which')
     @patch('runway.module.warn_on_boto_env_vars')
-    def test_init(self, mock_warn, runway_context, tmp_path):
+    def test_init(self, mock_warn, mock_which, caplog, runway_context, tmp_path):
         """Test init and the attributes set in init."""
         # pylint: disable=no-member
+        caplog.set_level(logging.ERROR, logger='runway')
+        mock_which.side_effect = [True, True, False]
         options = {
             'environments': True,  # can only be True of a falsy value (but not dict)
             'options': {'test_option': 'option_value'},
             'parameters': {'test_parameter': 'parameter_value'},
             'extra': 'something'
         }
-        obj = RunwayModuleNpm(context=runway_context,
+        obj = RunwayModuleNpm(context=runway_context,  # side_effect[0]
                               path=str(tmp_path),
                               options=options.copy())
 
-        obj.check_for_npm.assert_called_once()
+        mock_which.assert_called_once()
         assert obj.context == runway_context
         assert obj.environments == options['environments']
         assert obj.extra == options['extra']
@@ -67,13 +69,23 @@ def test_init(self, mock_warn, runway_context, tmp_path):
         assert str(obj.path) == str(tmp_path)
         mock_warn.assert_called_once_with(runway_context.env_vars)
 
-        obj = RunwayModuleNpm(context=runway_context,
+        obj = RunwayModuleNpm(context=runway_context,  # side_effect[1]
                               path=tmp_path)
         assert not obj.environments
         assert not obj.options
         assert not obj.parameters
         assert obj.path == tmp_path
 
+        caplog.clear()
+        with pytest.raises(SystemExit) as excinfo:
+            obj = RunwayModuleNpm(context=runway_context,  # side_effect[2]
+                                  path=tmp_path)
+        assert excinfo.value.code > 0  # non-zero exit code
+        assert caplog.messages == [
+            '{}: "npm" not found in path or is not executable; '
+            'please ensure it is installed correctly.'.format(tmp_path.name)
+        ]
+
     @patch('runway.module.format_npm_command_for_logging')
     def test_log_npm_command(self, mock_log, caplog, patch_module_npm,
                              runway_context):