Fixes a gnarly test bug with server side rendering

Background: Running tests multiple times in a row could result in
errors, and could result in complete passing. In other words, the order
the tests ran in determined whether or not the tests would pass.

Ruby Test::Unit uses a psuedorandom seed to randomize the
order that tests are run in; you can specify a certain seed with
TESTOPS="--seed=xyz", which fixes the seed and thus the order the tests
will be run in.

The main source of the bug was that many of the tests rely on replacing
asset files and asserting that the results are as expected; however, the
asset pipeline uses mtime to determine when files have been changed.

*mtime's resolution is only 1 second, meaning that when a series of
actions took less than a second to run, the mtime wouldn't actually
change, and Rails asset pipeline wouldn't update the file list.*

This commit adds a hook to force reset the internal concatinated
react.js and components.js string. Another test requires a few calls to
sleep(1), because it is specifically testing that the asset pipeline
picks up changes to the file on disk correctly and renders them server
side.

Author: xionon

lib/react/rails/railtie.rb

@@ -64,7 +64,7 @@ class Railtie < ::Rails::Railtie
 
         do_setup = lambda do
           cfg = app.config.react
-          React::Renderer.setup!( cfg.react_js.call, cfg.components_js.call,
+          React::Renderer.setup!( cfg.react_js, cfg.components_js,
                                 {:size => cfg.size, :timeout => cfg.timeout})
         end
 

lib/react/renderer.rb

@@ -10,7 +10,7 @@ def self.setup!(react_js, components_js, args={})
       @@react_js = react_js
       @@components_js = components_js
       @@pool.shutdown{} if @@pool
-      @@combined_js = nil if defined? @@combined_js
+      reset_combined_js!
       @@pool = ConnectionPool.new(:size => args[:size]||10, :timeout => args[:timeout]||20) { self.new }
     end
 
@@ -20,8 +20,8 @@ def self.render(component, args={})
       end
     end
 
-    def self.combined_js
-      @@combined_js ||= <<-CODE
+    def self.setup_combined_js
+      <<-CODE
         var global = global || this;
 
         var console = global.console || {};
@@ -31,12 +31,20 @@ def self.combined_js
           }
         });
 
-        #{@@react_js};
+        #{@@react_js.call};
         React = global.React;
-        #{@@components_js};
+        #{@@components_js.call};
       CODE
     end
 
+    def self.reset_combined_js!
+      @@combined_js = setup_combined_js
+    end
+
+    def self.combined_js
+      @@combined_js
+    end
+
     def context
       @context ||= ExecJS.compile(self.class.combined_js)
     end

test/react_test.rb

@@ -31,6 +31,8 @@ class ReactTest < ActionDispatch::IntegrationTest
 
     assert_response :success
     assert_equal "'test_confirmation_token_react_content';\n", @response.body
+
+    React::Renderer.reset_combined_js!
   end
 
 end

test/server_rendered_html_test.rb

@@ -2,30 +2,34 @@
 require 'fileutils'
 
 class ServerRenderedHtmlTest  < ActionDispatch::IntegrationTest
+  # Rails' asset pipeline has trouble picking up changes to files if they happen too fast.
+  # By sleeping for a little bit at certain points, we can make sure that rails notices the
+  # change in the file mtime, and calls our renderer setup functions appropriately
+  def wait_to_ensure_asset_pipeline_detects_changes
+    sleep(1)
+  end
+
   test 'react server rendering reloads jsx after changes to the jsx files' do
     file_with_updates = File.expand_path('../helper_files/TodoListWithUpdates.js.jsx', __FILE__)
     file_without_updates = File.expand_path('../helper_files/TodoListWithoutUpdates.js.jsx', __FILE__)
     app_file = File.expand_path('../dummy/app/assets/javascripts/components/TodoList.js.jsx', __FILE__)
 
-    sleep(1)
     FileUtils.cp app_file, file_without_updates
-    # fixes inconsistent test runs; phantomjs doesn't always see that the file was
-    # updated without manually calling #touch on it
     FileUtils.touch app_file
 
     begin
       get '/server/1'
       refute_match 'Updated', response.body
 
-      sleep(1)
+      wait_to_ensure_asset_pipeline_detects_changes
       FileUtils.cp file_with_updates, app_file
       FileUtils.touch app_file
 
       get '/server/1'
       assert_match 'Updated', response.body
     ensure
       # if we have a test failure, we want to make sure that we revert the dummy file
-      sleep(1)
+      wait_to_ensure_asset_pipeline_detects_changes
       FileUtils.mv file_without_updates, app_file
       FileUtils.touch app_file
     end