Simplify subclassing by adding all arguments to the initializer. Also, clean up the method signatures for existing exporters.

lib/foreman/cli.rb

@@ -56,7 +56,7 @@ class Foreman::CLI < Thor
       error "Unknown export format: #{format} (unable to load file 'foreman/export/#{ format.tr('-', '_') }')."
     end
 
-    formatter.new(engine, options).export(location)
+    formatter.new(location, engine, options).export
   rescue Foreman::Export::Exception => ex
     error ex.message
   end

lib/foreman/export/base.rb

@@ -3,10 +3,12 @@ require "foreman/utils"
 
 class Foreman::Export::Base
 
-  attr_reader :engine, :app, :log, :port, :user, :template, :concurrency
+  attr_reader :location, :engine, :options, :app, :log, :port, :user, :template, :concurrency
 
-  def initialize(engine, options={})
+  def initialize(location, engine, options={})
+    @location    = location
     @engine      = engine
+    @options     = options
     @app         = options[:app]
     @log         = options[:log]
     @port        = options[:port]

lib/foreman/export/bluepill.rb

@@ -3,7 +3,7 @@ require "foreman/export"
 
 class Foreman::Export::Bluepill < Foreman::Export::Base
 
-  def export(location, options={})
+  def export
     error("Must specify a location") unless location
 
     FileUtils.mkdir_p location

lib/foreman/export/inittab.rb

@@ -2,7 +2,7 @@ require "foreman/export"
 
 class Foreman::Export::Inittab < Foreman::Export::Base
 
-  def export(fname=nil, options={})
+  def export
     app = self.app || File.basename(engine.directory)
     user = self.user || app
     log_root = self.log || "/var/log/#{app}"

lib/foreman/export/runit.rb

@@ -4,7 +4,7 @@ require "foreman/export"
 class Foreman::Export::Runit < Foreman::Export::Base
   ENV_VARIABLE_REGEX = /([a-zA-Z_]+[a-zA-Z0-9_]*)=(\S+)/
 
-  def export(location, options={})
+  def export
     error("Must specify a location") unless location
 
     app = self.app || File.basename(engine.directory)

lib/foreman/export/upstart.rb

@@ -3,7 +3,7 @@ require "foreman/export"
 
 class Foreman::Export::Upstart < Foreman::Export::Base
 
-  def export(location, options={})
+  def export
     error("Must specify a location") unless location
 
     FileUtils.mkdir_p location

spec/foreman/cli_spec.rb

@@ -31,8 +31,8 @@ describe "Foreman::CLI", :fakefs do
         write_procfile
         write_env("envfile")
         mock_export = mock(Foreman::Export::Upstart)
-        mock(Foreman::Export::Upstart).new(is_a(Foreman::Engine), { "env" => "envfile" }) { mock_export }
+        mock(Foreman::Export::Upstart).new("/upstart", is_a(Foreman::Engine), { "env" => "envfile" }) { mock_export }
-        mock_export.export("/upstart")
+        mock_export.export
         foreman %{ export upstart /upstart --env envfile }
       end
     end
@@ -63,8 +63,8 @@ describe "Foreman::CLI", :fakefs do
         it "runs successfully" do
           dont_allow(subject).error
           mock_export = mock(Foreman::Export::Upstart)
-          mock(Foreman::Export::Upstart).new(is_a(Foreman::Engine), {}) { mock_export }
+          mock(Foreman::Export::Upstart).new("/tmp/foo", is_a(Foreman::Engine), {}) { mock_export }
-          mock_export.export("/tmp/foo")
+          mock_export.export
           subject.export("upstart", "/tmp/foo")
         end
       end

spec/foreman/export/bluepill_spec.rb

@@ -5,20 +5,25 @@ require "tmpdir"
 
 describe Foreman::Export::Bluepill, :fakefs do
   let(:procfile) { FileUtils.mkdir_p("/tmp/app"); write_procfile("/tmp/app/Procfile") }
-  let(:engine) { Foreman::Engine.new(procfile) }
+  let(:engine)   { Foreman::Engine.new(procfile) }
-  let(:bluepill) { Foreman::Export::Bluepill.new(engine, :concurrency => "alpha=2") }
+  let(:options)  { Hash.new }
+  let(:bluepill) { Foreman::Export::Bluepill.new("/tmp/init", engine, options) }
 
   before(:each) { load_export_templates_into_fakefs("bluepill") }
   before(:each) { stub(bluepill).say }
 
   it "exports to the filesystem" do
-    bluepill.export("/tmp/init")
+    bluepill.export
     normalize_space(File.read("/tmp/init/app.pill")).should == normalize_space(example_export_file("bluepill/app.pill"))
   end
 
-  it "exports to the filesystem with concurrency" do
+  context "with concurrency" do
-    bluepill.export("/tmp/init", :concurrency => "alpha=2")
+    let(:options) { Hash[:concurrency => "alpha=2"] }
 
-    normalize_space(File.read("/tmp/init/app.pill")).should == normalize_space(example_export_file("bluepill/app-concurrency.pill"))
+    it "exports to the filesystem with concurrency" do
+      bluepill.export
+      normalize_space(File.read("/tmp/init/app.pill")).should == normalize_space(example_export_file("bluepill/app-concurrency.pill"))
+    end
   end
+
 end

spec/foreman/export/runit_spec.rb

@@ -6,7 +6,7 @@ require "tmpdir"
 describe Foreman::Export::Runit, :fakefs do
   let(:procfile) { FileUtils.mkdir_p("/tmp/app"); write_procfile("/tmp/app/Procfile", 'bar=baz') }
   let(:engine) { Foreman::Engine.new(procfile) }
-  let(:runit) { Foreman::Export::Runit.new(engine, :concurrency => 'alpha=2') }
+  let(:runit) { Foreman::Export::Runit.new('/tmp/init', engine, :concurrency => 'alpha=2,bravo=1') }
 
   before(:each) { load_export_templates_into_fakefs("runit") }
   before(:each) { stub(runit).say }
@@ -15,7 +15,7 @@ describe Foreman::Export::Runit, :fakefs do
   it "exports to the filesystem" do
     FileUtils.mkdir_p('/tmp/init')
 
-    runit.export('/tmp/init')
+    runit.export
 
     File.read("/tmp/init/app-alpha-1/run").should == example_export_file('runit/app-alpha-1-run')
     File.read("/tmp/init/app-alpha-1/log/run").should ==
@@ -36,6 +36,6 @@ describe Foreman::Export::Runit, :fakefs do
   end
 
   it "creates a full path to the export directory" do
-    expect { runit.export('/tmp/init', :concurrency => "alpha=2,bravo=1") }.to_not raise_error(Errno::ENOENT)
+    expect { runit.export }.to_not raise_error(Errno::ENOENT)
   end
 end

spec/foreman/export/upstart_spec.rb

@@ -5,14 +5,15 @@ require "tmpdir"
 
 describe Foreman::Export::Upstart, :fakefs do
   let(:procfile) { FileUtils.mkdir_p("/tmp/app"); write_procfile("/tmp/app/Procfile") }
-  let(:engine) { Foreman::Engine.new(procfile) }
+  let(:engine)   { Foreman::Engine.new(procfile) }
-  let(:upstart) { Foreman::Export::Upstart.new(engine, :concurrency => "alpha=2") }
+  let(:options)  { Hash.new }
+  let(:upstart)  { Foreman::Export::Upstart.new("/tmp/init", engine, options) }
 
   before(:each) { load_export_templates_into_fakefs("upstart") }
   before(:each) { stub(upstart).say }
 
   it "exports to the filesystem" do
-    upstart.export("/tmp/init")
+    upstart.export
 
     File.read("/tmp/init/app.conf").should         == example_export_file("upstart/app.conf")
     File.read("/tmp/init/app-alpha.conf").should   == example_export_file("upstart/app-alpha.conf")
@@ -21,19 +22,23 @@ describe Foreman::Export::Upstart, :fakefs do
     File.read("/tmp/init/app-bravo-1.conf").should == example_export_file("upstart/app-bravo-1.conf")
   end
 
-  it "exports to the filesystem with concurrency" do
+  context "with concurrency" do
-    upstart.export("/tmp/init", :concurrency => "alpha=2")
+    let(:options) { Hash[:concurrency => "alpha=2"] }
 
-    File.read("/tmp/init/app.conf").should         == example_export_file("upstart/app.conf")
+    it "exports to the filesystem with concurrency" do
-    File.read("/tmp/init/app-alpha.conf").should   == example_export_file("upstart/app-alpha.conf")
+      upstart.export
-    File.read("/tmp/init/app-alpha-1.conf").should == example_export_file("upstart/app-alpha-1.conf")
+
-    File.read("/tmp/init/app-alpha-2.conf").should == example_export_file("upstart/app-alpha-2.conf")
+      File.read("/tmp/init/app.conf").should            == example_export_file("upstart/app.conf")
-    File.exists?("/tmp/init/app-bravo-1.conf").should == false
+      File.read("/tmp/init/app-alpha.conf").should      == example_export_file("upstart/app-alpha.conf")
+      File.read("/tmp/init/app-alpha-1.conf").should    == example_export_file("upstart/app-alpha-1.conf")
+      File.read("/tmp/init/app-alpha-2.conf").should    == example_export_file("upstart/app-alpha-2.conf")
+      File.exists?("/tmp/init/app-bravo-1.conf").should == false
+    end
   end
 
   context "with alternate templates" do
     let(:template_root) { "/tmp/alternate" }
-    let(:upstart) { Foreman::Export::Upstart.new(engine, :template => template_root) }
+    let(:upstart) { Foreman::Export::Upstart.new("/tmp/init", engine, :template => template_root) }
 
     before do
       FileUtils.mkdir_p template_root
@@ -41,7 +46,7 @@ describe Foreman::Export::Upstart, :fakefs do
     end
 
     it "can export with alternate template files" do
-      upstart.export("/tmp/init")
+      upstart.export
 
       File.read("/tmp/init/app.conf").should == "alternate_template\n"
     end
@@ -62,7 +67,7 @@ describe Foreman::Export::Upstart, :fakefs do
     end
 
     it "can export with alternate template files" do
-      upstart.export("/tmp/init")
+      upstart.export
 
       File.read("/tmp/init/app.conf").should == "default_alternate_template\n"
     end