From 341c3438f6e930a373b282935543be246798b5df Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 19 Aug 2018 10:38:37 -1000 Subject: [PATCH] Test improvements (#1133) - Improvements to test helpers - Tests now properly exit if the config.build_test_command fails - Source path for the project using Webpacker would default to "app/assets/javascript" even if when the node_modules directory was set to "client". Fix now makes the (mis) configuration of this crystal clear. --- docs/basics/configuration.md | 46 +++++++++++++++++-- lib/react_on_rails/configuration.rb | 2 +- lib/react_on_rails/test_helper.rb | 10 ++++ lib/react_on_rails/utils.rb | 11 ++++- lib/react_on_rails/webpacker_utils.rb | 6 +++ .../webpack_assets_compiler_spec.rb | 3 ++ spec/react_on_rails/utils_spec.rb | 31 +++++++++++++ 7 files changed, 102 insertions(+), 7 deletions(-) diff --git a/docs/basics/configuration.md b/docs/basics/configuration.md index f1631799c..fe88ba842 100644 --- a/docs/basics/configuration.md +++ b/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. diff --git a/lib/react_on_rails/configuration.rb b/lib/react_on_rails/configuration.rb index 6488b0814..c0e3bf1cc 100644 --- a/lib/react_on_rails/configuration.rb +++ b/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 diff --git a/lib/react_on_rails/test_helper.rb b/lib/react_on_rails/test_helper.rb index 2ddedbf64..cf61eb677 100644 --- a/lib/react_on_rails/test_helper.rb +++ b/lib/react_on_rails/test_helper.rb @@ -75,6 +75,16 @@ 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::WebpackerUtils.using_webpacker? && + ReactOnRails::Utils.using_webpacker_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 diff --git a/lib/react_on_rails/utils.rb b/lib/react_on_rails/utils.rb index edcd807a3..48c99ddc3 100644 --- a/lib/react_on_rails/utils.rb +++ b/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,13 @@ def self.source_path end end + def self.using_webpacker_source_path_is_not_defined_and_custom_node_modules? + return false unless ReactOnRails::WebpackerUtils.using_webpacker? + + !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 diff --git a/lib/react_on_rails/webpacker_utils.rb b/lib/react_on_rails/webpacker_utils.rb index 3e43acdbb..69bd5639b 100644 --- a/lib/react_on_rails/webpacker_utils.rb +++ b/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 diff --git a/spec/react_on_rails/test_helper/webpack_assets_compiler_spec.rb b/spec/react_on_rails/test_helper/webpack_assets_compiler_spec.rb index 102f2fb75..0db09757f 100644 --- a/spec/react_on_rails/test_helper/webpack_assets_compiler_spec.rb +++ b/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 diff --git a/spec/react_on_rails/utils_spec.rb b/spec/react_on_rails/utils_spec.rb index 35beb6608..98ef7451b 100644 --- a/spec/react_on_rails/utils_spec.rb +++ b/spec/react_on_rails/utils_spec.rb @@ -71,6 +71,37 @@ module ReactOnRails end end + if ReactOnRails::WebpackerUtils.using_webpacker? + 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.using_webpacker_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.using_webpacker_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.using_webpacker_source_path_is_not_defined_and_custom_node_modules?).to eq(true) + end + end + end + describe ".server_bundle_js_file_path" do before do allow(Rails).to receive(:root).and_return(Pathname.new("."))