Improvements to test helpers

- Tests now properly exist if the config.build_test_command fails
- Source path for project using Webpacker would default to "app/javascript" even if when the node_modules
  directory was set to "client". Fix now makes the configuration of this crystal clear.

docs/basics/configuration.md

@@ -1,5 +1,40 @@
 Here is the full set of config options. This file is `/config/initializers/react_on_rails.rb`
 
+First, you should have a `/config/webpacker.yml` setup.
+
+Here is the setup when using the recommended `/client` directory for your node_modules and source files:
+
+```yaml
+# Note: Base output directory of /public is assumed for static files
+default: &default
+  compile: false
+  # Used in your webpack configuration. Must be created in the
+  # public_output_path folder
+  manifest: manifest.json
+  cache_manifest: false
+  source_path: client/app
+
+development:
+  <<: *default
+  # generated files for development, in /public/webpack/dev
+  public_output_path: webpack/dev
+
+test:
+  <<: *default
+  # generated files for tests, in /public/webpack/test
+  public_output_path: webpack/test
+
+production:
+  <<: *default
+  # generated files for tests, in /public/webpack/production
+  public_output_path: webpack/production
+  cache_manifest: true
+```
+
+Here's a representative `/config/initializers/react_on_rails.rb` setup when using this `/client` directory
+for all client files, including your sources and node_modules.
+
+
 ```ruby
 # frozen_string_literal: true
 
@@ -24,7 +59,7 @@ ReactOnRails.configure do |config|
 
   # defaults to "" (top level)
   #
-  config.node_modules_location = ""
+  config.node_modules_location = "client" # Recommended!
 
   # This configures the script to run to build the production assets by webpack. Set this to nil
   # if you don't want react_on_rails building this file for you.
@@ -52,17 +87,18 @@ ReactOnRails.configure do |config|
   #   public_output_path: packs-test
   # which means files in /public/packs-test
   #
-  # Alternately, you may configure this. It is relative to your Rails root directory.
-  # A custom, non-webpacker, config might use something like:
+  # Alternately, you may configure this if you are NOT using webpacker. It is relative to your Rails 
+  # root directory. A custom, non-webpacker, config might use something like:
   #
   # config.generated_assets_dir = File.join(%w[public webpack], Rails.env)
   # This setting should not be used if using webpacker. 
 
+  # CONFIGURE YOUR SOURCE FILES 
   # The test helper needs to know where your JavaScript files exist. The default is configured
   # by your config/webpacker.yml soure_path:
-  # source_path: app/javascript
+  # source_path: client/app/javascript # if using recommended /client directory
   #
-  # If you have a non-default `node_modules_location`, that is assumed to be the location of your source
+  # If you are not using webpacker, the `node_modules_location` is assumed to be the location of your source
   # files.
 
   # Define the files we need to check for webpack compilation when running tests.

lib/react_on_rails/configuration.rb

@@ -116,7 +116,7 @@ def error_if_using_webpacker_and_generated_assets_dir_not_match_public_output_pa
       webpacker_public_output_path = ReactOnRails::WebpackerUtils.webpacker_public_output_path
 
       if File.expand_path(generated_assets_dir) == webpacker_public_output_path.to_s
-        Rails.logger.warn("You specified /config/initializers/react_on_rails.rb generated_assets_dir "\
+        Rails.logger.warn("You specified generated_assets_dir in `config/initializers/react_on_rails.rb` "\
         "with Webpacker. Remove this line from your configuration file.")
       else
         msg = <<-MSG.strip_heredoc

lib/react_on_rails/test_helper.rb

@@ -75,6 +75,15 @@ def self.ensure_assets_compiled(webpack_assets_status_checker: nil,
             "outdated/missing bundles based on source_path #{source_path}"
           puts
           @printed_once = true
+
+          if ReactOnRails::Utils.source_path_is_not_defined_and_custom_node_modules?
+            msg = <<-MSG.strip_heredoc
+              WARNING: Define config.webpacker.yml to include sourcePath to configure
+              the location of your JavaScript source for React on Rails.
+              Default location of #{source_path} is used.
+            MSG
+            puts ReactOnRails::Utils.wrap_message(msg, :orange)
+          end
         end
       end
 

lib/react_on_rails/utils.rb

@@ -57,7 +57,9 @@ def self.invoke_and_exit_if_failed(cmd, failure_message)
         MSG
         # rubocop:enable Layout/IndentHeredoc
         puts wrap_message(msg)
-        exit(1)
+
+        # Rspec catches exit without! in the exit callbacks
+        exit!(1)
       end
       [stdout, stderr, status]
     end
@@ -135,6 +137,11 @@ def self.source_path
       end
     end
 
+    def self.source_path_is_not_defined_and_custom_node_modules?
+      !ReactOnRails::WebpackerUtils.webpacker_source_path_explicit? &&
+        ReactOnRails.configuration.node_modules_location.present?
+    end
+
     def self.generated_assets_full_path
       if ReactOnRails::WebpackerUtils.using_webpacker?
         ReactOnRails::WebpackerUtils.webpacker_public_output_path

lib/react_on_rails/webpacker_utils.rb

@@ -31,6 +31,12 @@ def self.manifest_exists?
       Webpacker.config.public_manifest_path.exist?
     end
 
+    def self.webpacker_source_path_explicit?
+      # WARNING: Calling private method `data` on Webpacker::Configuration, lib/webpacker/configuration.rb
+      config_webpacker_yml = Webpacker.config.send(:data)
+      config_webpacker_yml[:source_path].present?
+    end
+
     def self.check_manifest_not_cached
       return unless using_webpacker? && Webpacker.config.cache_manifest?
       msg = <<-MSG.strip_heredoc

spec/react_on_rails/test_helper/webpack_assets_compiler_spec.rb

@@ -10,6 +10,9 @@
         allow(ReactOnRails.configuration)
           .to receive(:build_test_command)
           .and_return(invalid_command)
+
+        # Mock this out or else it quits the test suite!
+        allow(ReactOnRails::Utils).to receive(:exit!).and_raise(SystemExit)
       end
 
       it "exits immediately" do

spec/react_on_rails/utils_spec.rb

@@ -71,6 +71,35 @@ module ReactOnRails
       end
     end
 
+    describe ".source_path_is_not_defined_and_custom_node_modules?" do
+      it "returns false if node_modules is blank" do
+        allow(ReactOnRails).to receive_message_chain("configuration.node_modules_location")
+          .and_return("")
+        allow(Webpacker).to receive_message_chain("config.send").with(:data)
+                                                                .and_return({})
+
+        expect(ReactOnRails::Utils.source_path_is_not_defined_and_custom_node_modules?).to eq(false)
+      end
+
+      it "returns false if source_path is defined in the config/webpacker.yml and node_modules defined" do
+        allow(ReactOnRails).to receive_message_chain("configuration.node_modules_location")
+          .and_return("client")
+        allow(Webpacker).to receive_message_chain("config.send").with(:data)
+                                                                .and_return(source_path: "client/app")
+
+        expect(ReactOnRails::Utils.source_path_is_not_defined_and_custom_node_modules?).to eq(false)
+      end
+
+      it "returns true if node_modules is not blank and the source_path is not defined in config/webpacker.yml" do
+        allow(ReactOnRails).to receive_message_chain("configuration.node_modules_location")
+          .and_return("node_modules")
+        allow(Webpacker).to receive_message_chain("config.send").with(:data)
+                                                                .and_return({})
+
+        expect(ReactOnRails::Utils.source_path_is_not_defined_and_custom_node_modules?).to eq(true)
+      end
+    end
+
     describe ".server_bundle_js_file_path" do
       before do
         allow(Rails).to receive(:root).and_return(Pathname.new("."))