Merge branch 'worker-init' into better-compress-gif

Conflicts:
	CHANGELOG.markdown

.travis.yml

@@ -5,10 +5,8 @@ rvm:
   - 1.9.3
   - 2.0.0
   - 2.1.0
-  - ruby-head
   - jruby-18mode
   - jruby-19mode
-  - jruby-head
   - ree
 script:
   - bundle exec image_optim --info
@@ -28,6 +26,3 @@ env:
   - PATH=~/bin:$PATH
 matrix:
   fast_finish: true
-  allow_failures:
-    - rvm: ruby-head
-    - rvm: jruby-head

CHANGELOG.markdown

@@ -4,6 +4,7 @@
 
 * Remove app and other extensions from gif images [@toy](https://github.com/toy)
 * Change gifsicle interlace option to deinterlace by default for better compression, pass `nil` to leave as is [@toy](https://github.com/toy)
+* Worker can change its initialization by overriding `init` and can initialize multiple instances [#70](https://github.com/toy/image_optim/issues/70) [@toy](https://github.com/toy)
 
 ## v0.18.0 (2014-11-01)
 

lib/image_optim.rb

@@ -64,7 +64,7 @@ def initialize(options = {})
 
     @bin_resolver = BinResolver.new(self)
 
-    @workers_by_format = create_workers_by_format do |klass|
+    @workers_by_format = Worker.create_all_by_format(self) do |klass|
       config.for_worker(klass)
     end
 
@@ -192,25 +192,6 @@ def log_workers_by_format
     end
   end
 
-  # Create hash with format mapped to list of workers sorted by run order
-  def create_workers_by_format(&options_proc)
-    by_format = {}
-    workers = Worker.create_all(self, &options_proc)
-    if skip_missing_workers
-      workers = Worker.reject_missing(workers)
-    else
-      Worker.resolve_all!(workers)
-    end
-    sorted = workers.sort_by.with_index{ |worker, i| [worker.run_order, i] }
-    sorted.each do |worker|
-      worker.image_formats.each do |format|
-        by_format[format] ||= []
-        by_format[format] << worker
-      end
-    end
-    by_format
-  end
-
   # Run method for each item in list
   # if block given yields item and result for item and returns array of yield
   # results

lib/image_optim/worker.rb

@@ -1,6 +1,6 @@
 # encoding: UTF-8
 
-require 'image_optim/bin_resolver/error'
+require 'image_optim/bin_resolver'
 require 'image_optim/configuration_error'
 require 'image_optim/option_definition'
 require 'image_optim/option_helpers'
@@ -14,6 +14,9 @@ class Worker
     @klasses = []
 
     class << self
+      # Default init for worker is new
+      alias_method :init, :new
+
       # List of available workers
       def klasses
         @klasses.to_enum
@@ -42,34 +45,45 @@ def option(name, default, type, description = nil, &proc)
           OptionDefinition.new(name, default, type, description, &proc)
       end
 
-      # Initialize all workers using options from calling options_proc with
-      # klass
-      def create_all(image_optim, &options_proc)
-        Worker.klasses.map do |klass|
-          next unless (options = options_proc[klass])
-          klass.new(image_optim, options)
-        end.compact
-      end
-
-      # Resolve all bins of all workers failing with one joint exception
-      def resolve_all!(workers)
-        errors = BinResolver.collect_errors(workers) do |worker|
-          worker.resolve_used_bins!
+      # Create hash with format mapped to list of workers sorted by run order
+      def create_all_by_format(image_optim, &options_proc)
+        by_format = {}
+        create_all(image_optim, &options_proc).each do |worker|
+          worker.image_formats.each do |format|
+            by_format[format] ||= []
+            by_format[format] << worker
+          end
         end
-        return if errors.empty?
-        fail BinResolver::Error, ['Bin resolving errors:', *errors].join("\n")
+        by_format
       end
 
-      # Resolve all bins of all workers showing warning for missing ones and
-      # returning others
-      def reject_missing(workers)
+      # Create list of workers sorted by run order
+      # Workers are initialized with options provided through options_proc
+      # Resolve all bins of all workers, if there are errors and
+      # skip_missing_workers of image_optim is true - show warnings, otherwise
+      # fail with one joint exception
+      def create_all(image_optim, &options_proc)
+        workers = klasses.map do |klass|
+          next unless (options = options_proc[klass])
+          klass.init(image_optim, options)
+        end.compact.flatten
+
         resolved = []
         errors = BinResolver.collect_errors(workers) do |worker|
           worker.resolve_used_bins!
           resolved << worker
         end
-        errors.each{ |error| warn error }
-        resolved
+
+        unless errors.empty?
+          if image_optim.skip_missing_workers
+            errors.each{ |error| warn error }
+          else
+            message = ['Bin resolving errors:', *errors].join("\n")
+            fail BinResolver::Error, message
+          end
+        end
+
+        resolved.sort_by.with_index{ |worker, i| [worker.run_order, i] }
       end
     end
 

spec/image_optim/worker_spec.rb

@@ -1,9 +1,11 @@
 require 'spec_helper'
 require 'image_optim/worker'
+require 'image_optim/bin_resolver'
 
 describe ImageOptim::Worker do
   before do
     stub_const('Worker', ImageOptim::Worker)
+    stub_const('BinResolver', ImageOptim::BinResolver)
   end
 
   describe :optimize do
@@ -17,4 +19,141 @@
     end
   end
 
+  describe :create_all_by_format do
+    it 'passes arguments to create_all' do
+      image_optim = double
+      options_proc = proc{ true }
+
+      expect(Worker).to receive(:create_all) do |arg, &block|
+        expect(arg).to eq(image_optim)
+        expect(block).to eq(options_proc)
+        []
+      end
+
+      Worker.create_all_by_format(image_optim, &options_proc)
+    end
+
+    it 'create hash by format' do
+      workers = [
+        double(:image_formats => [:a]),
+        double(:image_formats => [:a, :b]),
+        double(:image_formats => [:b, :c]),
+      ]
+
+      expect(Worker).to receive(:create_all).and_return(workers)
+
+      worker_by_format = {
+        :a => [workers[0], workers[1]],
+        :b => [workers[1], workers[2]],
+        :c => [workers[2]],
+      }
+
+      expect(Worker.create_all_by_format(double)).to eq(worker_by_format)
+    end
+  end
+
+  describe :create_all do
+    def worker_double(override = {})
+      stubs = {:resolve_used_bins! => nil, :run_order => 0}.merge(override)
+      instance_double(Worker, stubs)
+    end
+
+    let(:image_optim){ double }
+
+    it 'creates all workers for which options_proc returns true' do
+      workers = Array.new(3){ worker_double }
+      klasses = workers.map{ |worker| double(:init => worker) }
+      options_proc = proc{ |klass| klass != klasses[1] }
+
+      allow(Worker).to receive(:klasses).and_return(klasses)
+
+      expect(Worker.create_all(image_optim, &options_proc)).
+        to eq([workers[0], workers[2]])
+    end
+
+    it 'handles workers initializing multiple instances' do
+      workers = [
+        worker_double,
+        [worker_double, worker_double, worker_double],
+        worker_double
+      ]
+      klasses = workers.map{ |worker| double(:init => worker) }
+
+      allow(Worker).to receive(:klasses).and_return(klasses)
+
+      expect(Worker.create_all(image_optim){ true }).
+        to eq(workers.flatten)
+    end
+
+    describe 'with missing workers' do
+      let(:workers) do
+        Array.new(3) do |i|
+          worker = worker_double
+          unless i == 1
+            allow(worker).to receive(:resolve_used_bins!).
+              and_raise(BinResolver::BinNotFound, "not found #{i}")
+          end
+          worker
+        end
+      end
+      let(:klasses){ workers.map{ |worker| double(:init => worker) } }
+
+      before do
+        allow(Worker).to receive(:klasses).and_return(klasses)
+      end
+
+      describe 'if skip_missing_workers is true' do
+        define :bin_not_found do |message|
+          match do |error|
+            error.is_a?(BinResolver::BinNotFound) && error.message == message
+          end
+        end
+
+        it 'shows warnings and returns resolved workers ' do
+          allow(image_optim).to receive(:skip_missing_workers).and_return(true)
+
+          expect(Worker).to receive(:warn).
+            once.with(bin_not_found('not found 0'))
+          expect(Worker).to receive(:warn).
+            once.with(bin_not_found('not found 2'))
+
+          expect(Worker.create_all(image_optim){ true }).
+            to eq([workers[1]])
+        end
+      end
+
+      describe 'if skip_missing_workers is false' do
+        it 'fails with a joint exception' do
+          allow(image_optim).to receive(:skip_missing_workers).and_return(false)
+
+          expect do
+            Worker.create_all(image_optim){ true }
+          end.to raise_error(BinResolver::Error, /not found 0\nnot found 2/)
+        end
+      end
+    end
+
+    it 'orders workers by run_order' do
+      image_optim = double
+      run_orders = [10, -10, 0, 0, 0, 10, -10]
+      workers = run_orders.map do |run_order|
+        worker_double(:run_order => run_order)
+      end
+      klasses_list = workers.map{ |worker| double(:init => worker) }
+
+      [
+        klasses_list,
+        klasses_list.reverse,
+        klasses_list.shuffle,
+      ].each do |klasses|
+        allow(Worker).to receive(:klasses).and_return(klasses)
+
+        expected_order = klasses.map(&:init).sort_by.with_index do |worker, i|
+          [worker.run_order, i]
+        end
+
+        expect(Worker.create_all(image_optim){ true }).to eq(expected_order)
+      end
+    end
+  end
 end

spec/image_optim_spec.rb

@@ -28,36 +28,6 @@ def temp_copy(image)
     allow(ImageOptim::Config).to receive(:local).and_return({})
   end
 
-  describe 'workers' do
-    they 'are ordered by run_order' do
-      image_optim = ImageOptim.new
-      original_klasses = ImageOptim::Worker.klasses
-      formats = original_klasses.map do |klass|
-        klass.new(image_optim, {}).image_formats
-      end.flatten.uniq
-
-      [
-        original_klasses,
-        original_klasses.to_a.reverse,
-        original_klasses.to_a.shuffle,
-      ].each do |klasses|
-        expect(ImageOptim::Worker).to receive(:klasses).and_return(klasses)
-
-        image_optim = ImageOptim.new
-
-        formats.each do |format|
-          path = ImageOptim::ImagePath.new("test.#{format}")
-          expect(path).to receive(:format).and_return(format)
-
-          workers = image_optim.workers_for_image(path)
-          expect(workers).to eq(workers.sort_by.with_index do |worker, i|
-            [worker.run_order, i]
-          end)
-        end
-      end
-    end
-  end
-
   disable_all_workers = Hash[ImageOptim::Worker.klasses.map do |klass|
     [klass.bin_sym, false]
   end]