core: BoxRemove works, tests

lib/vagrant/action/builtin/box_remove.rb

@@ -12,24 +12,67 @@ module Vagrant
 
         def call(env)
           box_name     = env[:box_name]
-          box_provider = env[:box_provider].to_sym
+          box_provider = env[:box_provider]
+          box_provider = box_provider.to_sym if box_provider
+          box_version  = env[:box_version]
 
-          box = nil
-          begin
-            box = env[:box_collection].find(box_name, box_provider)
-          rescue Vagrant::Errors::BoxUpgradeRequired
-            env[:box_collection].upgrade(box_name)
-            retry
+          boxes = {}
+          env[:box_collection].all.each do |n, v, p|
+            boxes[n] ||= {}
+            boxes[n][p] ||= []
+            boxes[n][p] << v
           end
 
-          raise Vagrant::Errors::BoxNotFound, :name => box_name, :provider => box_provider if !box
+          all_box = boxes[box_name]
+          if !all_box
+            raise Errors::BoxRemoveNotFound, name: box_name
+          end
+
+          all_versions = nil
+          if !box_provider
+            if all_box.length == 1
+              # There is only one provider, just use that.
+              all_versions = all_box.values.first
+              box_provider = all_box.keys.first
+            else
+              raise Errors::BoxRemoveMultiProvider,
+                name: box_name,
+                providers: all_box.keys.map(&:to_s).sort.join(", ")
+            end
+          else
+            all_versions = all_box[box_provider]
+            if !all_versions
+              raise Errors::BoxRemoveProviderNotFound,
+                name: box_name,
+                provider: box_provider.to_s,
+                providers: all_box.keys.map(&:to_s).sort.join(", ")
+            end
+          end
+
+          if !box_version
+            if all_versions.length == 1
+              # There is only one version, just use that.
+              box_version = all_versions.first
+            else
+              # There are multiple versions, we can't choose.
+              raise Errors::BoxRemoveMultiVersion,
+                name: box_name,
+                provider: box_provider.to_s,
+                versions: all_versions.join(", ")
+            end
+          end
+
+          box = env[:box_collection].find(
+            box_name, box_provider, box_version)
+
           env[:ui].info(I18n.t("vagrant.commands.box.removing",
-                              :name => box_name,
-                              :provider => box_provider))
+                              :name => box.name,
+                              :provider => box.provider))
           box.destroy!
 
           # Passes on the removed box to the rest of the middleware chain
           env[:box_removed] = box
+
           @app.call(env)
         end
       end

lib/vagrant/errors.rb

@@ -160,14 +160,26 @@ module Vagrant
       error_key(:box_metadata_malformed)
     end
 
-    class BoxNotFound < VagrantError
-      error_key(:box_not_found)
-    end
-
     class BoxProviderDoesntMatch < VagrantError
       error_key(:box_provider_doesnt_match)
     end
 
+    class BoxRemoveNotFound < VagrantError
+      error_key(:box_remove_not_found)
+    end
+
+    class BoxRemoveProviderNotFound < VagrantError
+      error_key(:box_remove_provider_not_found)
+    end
+
+    class BoxRemoveMultiProvider < VagrantError
+      error_key(:box_remove_multi_provider)
+    end
+
+    class BoxRemoveMultiVersion < VagrantError
+      error_key(:box_remove_multi_version)
+    end
+
     class BoxUnpackageFailure < VagrantError
       error_key(:untar_failure, "vagrant.actions.box.unpackage")
     end

templates/locales/en.yml

@@ -315,12 +315,33 @@ en:
         that this issue can be fixed.
 
         %{error}
-      box_not_found: Box '%{name}' with '%{provider}' provider could not be found.
       box_provider_doesnt_match: |-
         The box you attempted to add doesn't match the provider you specified.
 
         Provider expected: %{expected}
         Provider of box: %{actual}
+      box_remove_multi_provider: |-
+        You requested to remove the box '%{name}'. This box has
+        multiple providers. You must explicitly select a single
+        provider to remove with `--provider`.
+
+        Available providers: %{providers}
+      box_remove_multi_version: |-
+        You requested to remove the box '%{name}' with provider
+        '%{provider}'. This box has multiple versions. You must
+        explicitly specify which version you want to remove with
+        the `--box-version` flag.
+
+        Versions: %{versions}
+      box_remove_not_found: |-
+        The box you requested to be removed could not be found. No
+        boxes named '%{name}' could be found.
+      box_remove_provider_not_found: |-
+        You requested to remove the box '%{name}' with provider
+        '%{provider}'. The box '%{name}' exists but not with
+        the provider specified. Please double-check and try again.
+
+        The providers for this are: %{providers}
       box_upgrade_required: |-
         The box '%{name}' is still stored on disk in the Vagrant 1.0.x
         format. This box must be upgraded in order to work properly with

test/unit/vagrant/action/builtin/box_remove_test.rb

@@ -0,0 +1,108 @@
+require File.expand_path("../../../../base", __FILE__)
+
+describe Vagrant::Action::Builtin::BoxRemove do
+  include_context "unit"
+
+  let(:app) { lambda { |env| } }
+  let(:env) { {
+    box_collection: box_collection,
+    ui: Vagrant::UI::Silent.new,
+  } }
+
+  subject { described_class.new(app, env) }
+
+  let(:box_collection) { double("box_collection") }
+  let(:iso_env) { isolated_environment }
+
+  let(:box) do
+    box_dir = iso_env.box3("foo", "1.0", :virtualbox)
+    Vagrant::Box.new("foo", :virtualbox, "1.0", box_dir)
+  end
+
+  it "deletes the box if it is the only option" do
+    box_collection.stub(all: [["foo", "1.0", :virtualbox]])
+
+    env[:box_name] = "foo"
+
+    box_collection.should_receive(:find).with(
+      "foo", :virtualbox, "1.0").and_return(box)
+    box.should_receive(:destroy!).once
+    app.should_receive(:call).with(env).once
+
+    subject.call(env)
+
+    expect(env[:box_removed]).to equal(box)
+  end
+
+  it "deletes the box with the specified provider if given" do
+    box_collection.stub(
+      all: [
+        ["foo", "1.0", :virtualbox],
+        ["foo", "1.0", :vmware],
+      ])
+
+    env[:box_name] = "foo"
+    env[:box_provider] = "virtualbox"
+
+    box_collection.should_receive(:find).with(
+      "foo", :virtualbox, "1.0").and_return(box)
+    box.should_receive(:destroy!).once
+    app.should_receive(:call).with(env).once
+
+    subject.call(env)
+
+    expect(env[:box_removed]).to equal(box)
+  end
+
+  it "errors if the box doesn't exist" do
+    box_collection.stub(all: [])
+
+    app.should_receive(:call).never
+
+    expect { subject.call(env) }.
+      to raise_error(Vagrant::Errors::BoxRemoveNotFound)
+  end
+
+  it "errors if the specified provider doesn't exist" do
+    env[:box_name] = "foo"
+    env[:box_provider] = "bar"
+
+    box_collection.stub(all: [["foo", "1.0", :virtualbox]])
+
+    app.should_receive(:call).never
+
+    expect { subject.call(env) }.
+      to raise_error(Vagrant::Errors::BoxRemoveProviderNotFound)
+  end
+
+  it "errors if there are multiple providers" do
+    env[:box_name] = "foo"
+
+    box_collection.stub(
+      all: [
+        ["foo", "1.0", :virtualbox],
+        ["foo", "1.0", :vmware],
+      ])
+
+    app.should_receive(:call).never
+
+    expect { subject.call(env) }.
+      to raise_error(Vagrant::Errors::BoxRemoveMultiProvider)
+  end
+
+  it "errors if the specified provider has multiple versions" do
+    env[:box_name] = "foo"
+    env[:box_provider] = "virtualbox"
+
+    box_collection.stub(
+      all: [
+        ["foo", "1.0", :virtualbox],
+        ["foo", "1.1", :virtualbox],
+      ])
+
+    app.should_receive(:call).never
+
+    expect { subject.call(env) }.
+      to raise_error(Vagrant::Errors::BoxRemoveMultiVersion)
+  end
+end