Fixed tests and include tag code to use register's file_system

lib/liquid/tags/include.rb

@@ -30,6 +30,11 @@ module Liquid
           @attributes[key] = value
         end
 
+        if partial_parsable_at_parse_time?
+          source = read_template_from_file_system_at_parse
+          @partial = Liquid::Template.parse(source, pass_options)
+        end
+
       else
         raise SyntaxError.new(options[:locale].t("errors.syntax.include".freeze))
       end
@@ -66,27 +71,40 @@ module Liquid
         template_name = context[@template_name]
 
         if cached = cached_partials[template_name]
-          return cached
+          cached
+        else
+          if @partial
+            partial = @partial
+          else
+            partial = Liquid::Template.parse(read_template_from_file_system(context), pass_options)
+          end
+          cached_partials[template_name] = partial
+          context.registers[:cached_partials] = cached_partials
+          partial
         end
-        source = read_template_from_file_system(context)
-        partial = Liquid::Template.parse(source, pass_options)
-        cached_partials[template_name] = partial
-        context.registers[:cached_partials] = cached_partials
-        partial
       end
 
       def read_template_from_file_system(context)
         file_system = context.registers[:file_system] || Liquid::Template.file_system
 
-        # make read_template_file call backwards-compatible.
-        case file_system.method(:read_template_file).arity
-        when 1
-          file_system.read_template_file(context[@template_name])
-        when 2
-          file_system.read_template_file(context[@template_name], context)
-        else
-          raise ArgumentError, "file_system.read_template_file expects two parameters: (template_name, context)"
-        end
+        file_system.read_template_file(context[@template_name])
+      end
+
+      def read_template_from_file_system_at_parse
+        parsed_file_system.read_template_file(parsed_template_name)
+      end
+
+      def parsed_file_system
+        options[:file_system]
+      end
+
+      def partial_parsable_at_parse_time?
+        template_name_is_string_constant = parsed_template_name.is_a?(String)
+        options[:file_system] && template_name_is_string_constant
+      end
+
+      def parsed_template_name
+        Expression.parse(@template_name)
       end
 
       def pass_options

performance/benchmark.rb

@@ -1,14 +1,17 @@
-require 'benchmark'
+require 'benchmark/ips'
 require File.dirname(__FILE__) + '/theme_runner'
 
 Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first
 profiler = ThemeRunner.new
 
-N = 100
-Benchmark.bmbm do |x|
-  x.report("parse:")                 { N.times { profiler.parse } }
-  x.report("marshal load:")          { N.times { profiler.marshal_load } }
-  x.report("render:")                { N.times { profiler.render } }
-  x.report("marshal load & render:") { N.times { profiler.load_and_render } }
-  x.report("parse & render:")        { N.times { profiler.parse_and_render } }
+Benchmark.ips do |x|
+  x.time = 60
+  x.warmup = 5
+
+  puts
+  puts "Running benchmark for #{x.time} seconds (with #{x.warmup} seconds warmup)."
+  puts
+
+  x.report("parse:") { profiler.compile }
+  x.report("parse & run:") { profiler.run }
 end

performance/profile.rb

@@ -3,13 +3,13 @@ require File.dirname(__FILE__) + '/theme_runner'
 
 Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first
 profiler = ThemeRunner.new
-profiler.parse_and_render
+profiler.run
 
 [:cpu, :object].each do |profile_type|
   puts "Profiling in #{profile_type.to_s} mode..."
   results = StackProf.run(mode: profile_type) do
     100.times do
-      profiler.parse_and_render
+      profiler.run
     end
   end
   StackProf::Report.new(results).print_text(false, 20)

performance/theme_runner.rb

@@ -32,59 +32,45 @@ class ThemeRunner
 
       [File.read(test), (File.file?(theme_path) ? File.read(theme_path) : nil), test]
     end.compact
-    @parsed = @tests.map do |liquid, layout, template_name|
-      [Liquid::Template.parse(liquid), Liquid::Template.parse(layout), template_name]
-    end
-    @marshaled = @parsed.map do |liquid, layout, template_name|
-      [Marshal.dump(liquid), Marshal.dump(layout), template_name]
-    end
   end
 
-  def parse
+  def compile
+    # Dup assigns because will make some changes to them
+
     @tests.each do |liquid, layout, template_name|
-      Liquid::Template.parse(liquid)
-      Liquid::Template.parse(layout)
+
+      tmpl = Liquid::Template.new
+      tmpl.parse(liquid)
+      tmpl = Liquid::Template.new
+      tmpl.parse(layout)
     end
   end
 
-  def marshal_load
-    @marshaled.each do |liquid, layout, template_name|
-      Marshal.load(liquid)
-      Marshal.load(layout)
-    end
-  end
-
-  def render
-    @parsed.each do |liquid, layout, template_name|
-      render_once(liquid, layout, template_name)
-    end
-  end
-
-  def load_and_render
-    @marshaled.each do |liquid, layout, template_name|
-      render_once(Marshal.load(liquid), Marshal.load(layout), template_name)
-    end
-  end
-
-  def parse_and_render
-    @tests.each do |liquid, layout, template_name|
-      render_once(Liquid::Template.parse(liquid), Liquid::Template.parse(layout), template_name)
-    end
-  end
-
-  def render_once(template, layout, template_name)
+   def run
     # Dup assigns because will make some changes to them
     assigns = Database.tables.dup
 
-    assigns['page_title'] = 'Page title'
-    assigns['template'] = File.basename(template_name, File.extname(template_name))
-    template.registers[:file_system] = ThemeRunner::FileSystem.new(File.dirname(template_name))
+    @tests.each do |liquid, layout, template_name|
 
-    content_for_layout = template.render!(assigns)
+      # Compute page_tempalte outside of profiler run, uninteresting to profiler
+      page_template = File.basename(template_name, File.extname(template_name))
+      compile_and_render(liquid, layout, assigns, page_template, template_name)
+
+    end
+  end
+
+
+  def compile_and_render(template, layout, assigns, page_template, template_file)
+    tmpl = Liquid::Template.new
+    tmpl.assigns['page_title'] = 'Page title'
+    tmpl.assigns['template'] = page_template
+    tmpl.registers[:file_system] = ThemeRunner::FileSystem.new(File.dirname(template_file))
+
+    content_for_layout = tmpl.parse(template).render!(assigns)
 
     if layout
       assigns['content_for_layout'] = content_for_layout
-      layout.render!(assigns)
+      tmpl.parse(layout).render!(assigns)
     else
       content_for_layout
     end

test/integration/blank_test.rb

@@ -9,7 +9,7 @@ class FoobarTag < Liquid::Tag
 end
 
 class BlankTestFileSystem
-  def read_template_file(template_path, context)
+  def read_template_file(template_path)
     template_path
   end
 end

test/integration/render_profiling_test.rb

@@ -4,7 +4,7 @@ class RenderProfilingTest < Minitest::Test
   include Liquid
 
   class ProfilingFileSystem
-    def read_template_file(template_path, context)
+    def read_template_file(template_path)
       "Rendering template {% assign template_name = '#{template_path}'%}\n{{ template_name }}"
     end
   end

test/integration/tags/include_tag_test.rb

@@ -1,7 +1,7 @@
 require 'test_helper'
 
 class TestFileSystem
-  def read_template_file(template_path, context)
+  def read_template_file(template_path)
     case template_path
     when "product"
       "Product: {{ product.title }} "
@@ -33,15 +33,22 @@ class TestFileSystem
   end
 end
 
+# FileSystems at parse time don't have a context
+class ParseFileSystem
+  def read_template_file(template_path)
+    'from ParseFileSystem'
+  end
+end
+
 class OtherFileSystem
-  def read_template_file(template_path, context)
+  def read_template_file(template_path)
     'from OtherFileSystem'
   end
 end
 
 class CountingFileSystem
   attr_reader :count
-  def read_template_file(template_path, context)
+  def read_template_file(template_path)
     @count ||= 0
     @count += 1
     'from CountingFileSystem'
@@ -77,6 +84,15 @@ class IncludeTagTest < Minitest::Test
       Template.parse("{% include 'pick_a_source' %}").render!({}, :registers => {:file_system => OtherFileSystem.new})
   end
 
+  def test_include_tag_can_use_file_system_at_parse_so_it_can_be_parsed_before_rendered
+    assert_equal 'from ParseFileSystem',
+      Template.parse("{% include 'pick_a_source' %}",file_system: ParseFileSystem.new).render!({}, :registers => {:file_system => OtherFileSystem.new})
+  end
+
+  def test_include_tag_use_registers_file_system_when_it_cant_be_preparse
+    assert_equal 'from OtherFileSystem',
+      Template.parse("{% include template %}",file_system: ParseFileSystem.new).render!({}, :registers => {:file_system => OtherFileSystem.new})
+  end 
 
   def test_include_tag_with
     assert_template_result "Product: Draft 151cm ",
@@ -125,7 +141,7 @@ class IncludeTagTest < Minitest::Test
   def test_recursively_included_template_does_not_produce_endless_loop
 
     infinite_file_system = Class.new do
-      def read_template_file(template_path, context)
+      def read_template_file(template_path)
         "-{% include 'loop' %}"
       end
     end
@@ -138,18 +154,6 @@ class IncludeTagTest < Minitest::Test
 
   end
 
-  def test_backwards_compatability_support_for_overridden_read_template_file
-    infinite_file_system = Class.new do
-      def read_template_file(template_path) # testing only one argument here.
-        "- hi mom"
-      end
-    end
-
-    Liquid::Template.file_system = infinite_file_system.new
-
-    Template.parse("{% include 'hi_mom' %}").render!
-  end
-
   def test_dynamically_choosen_template
     assert_template_result "Test123", "{% include template %}", "template" => 'Test123'
     assert_template_result "Test321", "{% include template %}", "template" => 'Test321'