better error handling

.gitignore

@@ -6,4 +6,5 @@ test_files/*
 .idea/*
 .bundle/*
 .rvmrc
-junk.*
+junk.*
+.auto_tagger

auto_tagger.gemspec

@@ -9,16 +9,15 @@ Gem::Specification.new do |s|
 
   s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version=
   s.authors = ["Jeff Dean", "Brian Takita", "Mike Grafton", "Bruce Krysiak", "Pat Nakajima", "Jay Zeschin", "Mike Barinek", "Sarah Mei"]
-  s.date = %q{2010-09-09}
+  s.date = %q{2010-09-10}
   s.default_executable = %q{autotag}
   s.email = %q{jeff@zilkey.com}
   s.executables = ["autotag"]
   s.extra_rdoc_files = [
     "README.md"
   ]
   s.files = [
-    ".auto_tagger",
-     ".document",
+    ".document",
      ".gitignore",
      "CHANGELOG",
      "Gemfile",

bin/autotag

@@ -2,10 +2,14 @@
 require File.expand_path(File.join(File.dirname(__FILE__), "..", "lib", "auto_tagger"))
 
 begin
-  # The dup is to keep ARGV intact, so that tools like ruby-debug can respawn.
   success, message = AutoTagger::CommandLine.new(ARGV.dup).execute
-  puts message
-  Kernel.exit(success ? 0 : 1)
+  if success
+    puts message
+    Kernel.exit 0
+  else
+    puts "Error occured: #{message}"
+    Kernel.exit 1
+  end
 rescue SystemExit => e
   Kernel.exit(e.status)
 rescue Exception => e

features/autotag.feature

@@ -83,3 +83,10 @@ Feature: Deployment
     When I run autotag with "create demo --date-separator -"
     Then a hyphen-delimited "demo" tag should be created
     And the exit code should be 0
+
+  @1.0.0
+  Scenario: user runs autotag with "cleanup" with no stage
+    Given a repo
+    When I run autotag with "cleanup"
+    Then I should see "Error occured: You must provide a stage"
+    And the exit code should be 1

lib/auto_tagger/base.rb

@@ -59,9 +59,10 @@ def push
     private :push
 
     def cleanup
-      length = delete_locally
-      delete_on_remote if configuration.push_refs?
-      return length
+      refs = refs_to_remove
+      delete_local_refs(refs)
+      delete_remote_refs(refs) if configuration.push_refs?
+      refs.length
     end
 
     def delete_locally
@@ -70,14 +71,28 @@ def delete_locally
       refs.length
     end
 
+    def delete_local_refs(refs)
+      refs.each { |ref| ref.delete_locally }
+    end
+
+    private :delete_local_refs
+
     def delete_on_remote
       refs = refs_to_remove
-      cmd = ["push #{configuration.remote}"]
-      cmd += refs.map { |ref| ":#{ref.name}" }
-      repo.exec cmd.join(" ")
+      delete_remote_refs(refs)
       refs.length
     end
 
+    def delete_remote_refs(refs)
+      if refs.any?
+        cmd = ["push #{configuration.remote}"]
+        cmd += refs.map { |ref| ":#{ref.name}" }
+        repo.exec cmd.join(" ")
+      end
+    end
+
+    private :delete_remote_refs
+
     def list
       ensure_stage
       fetch
@@ -91,6 +106,7 @@ def release_tag_entries
     end
 
     def refs_for_stage(stage)
+      raise StageCannotBeBlankError if stage.to_s.strip == ""
       ref_path = Regexp.escape(configuration.ref_path)
       matcher = /refs\/#{ref_path}\/#{Regexp.escape(stage)}\/.*/
       repo.refs.all.select do |ref|

lib/auto_tagger/command_line.rb

@@ -6,36 +6,53 @@ def initialize(args)
     end
 
     def execute
-      message = case options[:command]
+      case options[:command]
         when :version
-          "AutoTagger version #{AutoTagger.version}"
+          [true, "AutoTagger version #{AutoTagger.version}"]
         when :help
-          options[:help_text]
+          [true, options[:help_text]]
         when :cleanup
-          purged = AutoTagger::Base.new(options).cleanup
-          "Deleted: #{purged}"
+          begin
+            purged = AutoTagger::Base.new(options).cleanup
+            [true, "Deleted: #{purged}"]
+          rescue AutoTagger::Base::StageCannotBeBlankError
+            [false, "You must provide a stage"]
+          end
         when :delete_locally
-          purged = AutoTagger::Base.new(options).delete_locally
-          "Deleted: #{purged}"
+          begin
+            purged = AutoTagger::Base.new(options).delete_locally
+            [true, "Deleted: #{purged}"]
+          rescue AutoTagger::Base::StageCannotBeBlankError
+            [false, "You must provide a stage"]
+          end
         when :delete_on_remote
-          purged = AutoTagger::Base.new(options).delete_on_remote
-          "Deleted: #{purged}"
+          begin
+            purged = AutoTagger::Base.new(options).delete_on_remote
+            [true, "Deleted: #{purged}"]
+          rescue AutoTagger::Base::StageCannotBeBlankError
+            [false, "You must provide a stage"]
+          end
         when :list
-          AutoTagger::Base.new(options).list.join("\n")
+          begin
+            [true, AutoTagger::Base.new(options).list.join("\n")]
+          rescue AutoTagger::Base::StageCannotBeBlankError
+            [false, "You must provide a stage"]
+          end
         when :config
-          AutoTagger::Configuration.new(options).settings.map do |key, value|
-            "#{key} : #{value}"
-          end.join("\n")
+          [true, AutoTagger::Configuration.new(options).settings.map { |key, value| "#{key} : #{value}" }.join("\n")]
         else
-          create_message = []
-          if options[:deprecated]
-            create_message << AutoTagger::Deprecator.string("Please use `autotag create #{options[:stage]}` instead")
+          begin
+            create_message = []
+            if options[:deprecated]
+              create_message << AutoTagger::Deprecator.string("Please use `autotag create #{options[:stage]}` instead")
+            end
+            ref = AutoTagger::Base.new(options).create_ref
+            create_message << "Created ref #{ref.name}"
+            [true, create_message.join("\n")]
+          rescue AutoTagger::Base::StageCannotBeBlankError
+            [false, "You must provide a stage"]
           end
-          ref = AutoTagger::Base.new(options).create_ref
-          create_message << "Created ref #{ref.name}"
-          create_message.join("\n")
       end
-      [true, message]
     end
 
     private

lib/auto_tagger/options.rb

@@ -24,7 +24,9 @@ def self.from_command_line(args)
           "    autotag list demo",
           "",
           "    autotag cleanup demo --refs-to-keep=2",
-          "    autotag cleanup demo --refs-to-keep=2 --dry-run",
+          "    autotag cleanup demo --refs-to-keep=2",
+          "    autotag delete_locally demo",
+          "    autotag delete_on_remote demo",
           "",
           "",
         ].join("\n")

spec/auto_tagger/base_spec.rb

@@ -115,6 +115,13 @@
   end
 
   describe "#delete_locally" do
+    it "blows up if you don't enter a stage" do
+      base = AutoTagger::Base.new({})
+      proc do
+        base.delete_locally
+      end.should raise_error(AutoTagger::Base::StageCannotBeBlankError)
+    end
+
     it "executes the local delete commands for all the refs that match" do
       base = AutoTagger::Base.new :stage => "ci"
       base.repo.stub(:exec) { true }
@@ -133,6 +140,20 @@
   end
 
   describe "#delete_on_remote" do
+    it "blows up if you don't enter a stage" do
+      base = AutoTagger::Base.new({})
+      proc do
+        base.delete_on_remote
+      end.should raise_error(AutoTagger::Base::StageCannotBeBlankError)
+    end
+
+    it "does not push if there are no tags" do
+      base = AutoTagger::Base.new :stage => "ci"
+      base.stub(:refs_for_stage).with("ci") {[]}
+      base.repo.should_not_receive(:exec)
+      base.delete_on_remote
+    end
+
     it "executes the remote delete commands in a batch" do
       base = AutoTagger::Base.new :stage => "ci"
       base.repo.stub(:exec) { true }
@@ -149,27 +170,34 @@
   end
 
   describe "#cleanup" do
+    it "blows up if you don't enter a stage" do
+      base = AutoTagger::Base.new({})
+      proc do
+        base.cleanup
+      end.should raise_error(AutoTagger::Base::StageCannotBeBlankError)
+    end
+
     it "executes delete locally" do
       base = AutoTagger::Base.new :stage => "ci"
       base.repo.stub(:exec) { true }
       base.stub(:refs_for_stage).with("ci") { [AutoTagger::Git::Ref.new(base.repo, "abc123", "refs/tags/ci/2008")] }
-      base.should_receive(:delete_locally)
+      base.should_receive(:delete_local_refs)
       base.cleanup
     end
 
     it "executes delete on remote if push refs is true" do
       base = AutoTagger::Base.new :stage => "ci"
       base.repo.stub(:exec) { true }
       base.stub(:refs_for_stage).with("ci") { [AutoTagger::Git::Ref.new(base.repo, "abc123", "refs/tags/ci/2008")] }
-      base.should_receive(:delete_on_remote)
+      base.should_receive(:delete_remote_refs)
       base.cleanup
     end
 
     it "does not execute delete on remote if push refs is false" do
       base = AutoTagger::Base.new :stage => "ci", :offline => true
       base.repo.stub(:exec) { true }
       base.stub(:refs_for_stage).with("ci") { [AutoTagger::Git::Ref.new(base.repo, "abc123", "refs/tags/ci/2008")] }
-      base.should_not_receive(:delete_on_remote)
+      base.should_not_receive(:delete_remote_refs)
       base.cleanup
     end
   end