commands/box: broken box adding, but more options/tests

plugins/commands/box/command/add.rb

@@ -8,18 +8,10 @@ module VagrantPlugins
           options = {}
 
           opts = OptionParser.new do |o|
-            o.banner = "Usage: vagrant box add <name> <url> [--provider provider] [-h]"
+            o.banner = "Usage: vagrant box add <url> [-h]"
             o.separator ""
 
-            o.on("--checksum VALUE", String, "Checksum") do |c|
+            o.on("-c", "--clean", "Clean any temporary download files") do |c|
-              options[:checksum] = c
-            end
-
-            o.on("--checksum-type VALUE", String, "Checksum type") do |c|
-              options[:checksum_type] = c.to_sym
-            end
-
-            o.on("-c", "--clean", "Remove old temporary download if it exists.") do |c|
               options[:clean] = c
             end
 
@@ -27,45 +19,66 @@ module VagrantPlugins
               options[:force] = f
             end
 
-            o.on("--insecure", "If set, SSL certs will not be validated.") do |i|
+            o.on("--insecure", "Do not validate SSL certificates") do |i|
               options[:insecure] = i
             end
 
-            o.on("--cacert certfile", String, "CA certificate") do |c|
+            o.on("--cacert certfile", String, "CA certificate for SSL download") do |c|
               options[:ca_cert] = c
             end
 
             o.on("--cert certfile", String,
-                 "The client SSL cert") do |c|
+                 "A client SSL cert, if needed") do |c|
               options[:client_cert] = c
             end
 
-            o.on("--provider provider", String,
+            o.on("--provider VALUE", String, "Provider the box should satisfy") do |p|
-                 "The provider that backs the box.") do |p|
               options[:provider] = p
             end
+
+            o.separator ""
+            o.separator "The options below only apply if you're adding a box file directly,"
+            o.separator "and not using a Vagrant server or a box structured like 'user/box':"
+            o.separator ""
+
+            o.on("--checksum VALUE", String, "Checksum for the box") do |c|
+              options[:checksum] = c
+            end
+
+            o.on("--checksum-type VALUE", String, "Checksum type (md5, sha1, sha256)") do |c|
+              options[:checksum_type] = c.to_sym
+            end
+
+            o.on("--name VALUE", String, "Name of the box") do |n|
+              options[:name] = n
+            end
           end
 
           # Parse the options
           argv = parse_options(opts)
           return if !argv
-          raise Vagrant::Errors::CLIInvalidUsage, :help => opts.help.chomp if argv.length < 2
+          if argv.empty? || argv.length > 2
+            raise Vagrant::Errors::CLIInvalidUsage,
+              help: opts.help.chomp
+          end
 
-          # Get the provider if one was set
+          url = argv[0]
-          provider = nil
+          if argv.length == 2
-          provider = options[:provider].to_sym if options[:provider]
+            options[:name] = argv[0]
+            url = argv[1]
+          end
 
           @env.action_runner.run(Vagrant::Action.action_box_add, {
-            :box_name     => argv[0],
+            box_url: url,
-            :box_provider => provider,
+            box_name: options[:name],
-            :box_url      => argv[1],
+            box_provider: options[:provider],
-            :box_checksum_type => options[:checksum_type],
+            box_checksum_type: options[:checksum_type],
-            :box_checksum => options[:checksum],
+            box_checksum: options[:checksum],
-            :box_clean    => options[:clean],
+            box_clean: options[:clean],
-            :box_force    => options[:force],
+            box_force: options[:force],
-            :box_download_ca_cert => options[:ca_cert],
+            box_download_ca_cert: options[:ca_cert],
-            :box_download_client_cert => options[:client_cert],
+            box_download_client_cert: options[:client_cert],
-            :box_download_insecure => options[:insecure],
+            box_download_insecure: options[:insecure],
           })
 
           # Success, exit status 0

test/unit/plugins/commands/box/command/add_test.rb

@@ -0,0 +1,67 @@
+require File.expand_path("../../../../../base", __FILE__)
+
+require Vagrant.source_root.join("plugins/commands/box/command/add")
+
+describe VagrantPlugins::CommandBox::Command::Add do
+  include_context "unit"
+
+  let(:argv)     { [] }
+  let(:iso_env) do
+    # We have to create a Vagrantfile so there is a root path
+    env = isolated_environment
+    env.vagrantfile("")
+    env.create_vagrant_env
+  end
+
+  subject { described_class.new(argv, iso_env) }
+
+  let(:action_runner) { double("action_runner") }
+
+  before do
+    iso_env.stub(action_runner: action_runner)
+  end
+
+  context "with no arguments" do
+    it "shows help" do
+      expect { subject.execute }.
+        to raise_error(Vagrant::Errors::CLIInvalidUsage)
+    end
+  end
+
+  context "with one argument" do
+    let(:argv) { ["foo"] }
+
+    it "executes the runner with the proper actions" do
+      action_runner.should_receive(:run).with do |action, **opts|
+        expect(opts[:box_name]).to be_nil
+        expect(opts[:box_url]).to eq("foo")
+        true
+      end
+
+      subject.execute
+    end
+  end
+
+  context "with two arguments" do
+    let(:argv) { ["foo", "bar"] }
+
+    it "executes the runner with the proper actions" do
+      action_runner.should_receive(:run).with do |action, **opts|
+        expect(opts[:box_name]).to eq("foo")
+        expect(opts[:box_url]).to eq("bar")
+        true
+      end
+
+      subject.execute
+    end
+  end
+
+  context "with more than two arguments" do
+    let(:argv) { ["one", "two", "three"] }
+
+    it "shows help" do
+      expect { subject.execute }.
+        to raise_error(Vagrant::Errors::CLIInvalidUsage)
+    end
+  end
+end

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

@@ -0,0 +1,8 @@
+require File.expand_path("../../../../base", __FILE__)
+
+describe Vagrant::Action::Builtin::BoxAdd do
+  let(:app) { lambda { |env| } }
+  let(:env) { {} }
+
+  subject { described_class.new(app, env) }
+end