From 77b5fa94b7cd857d7f24172b9e8c64da401a0b44 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 22 Jan 2014 11:42:39 -0800 Subject: [PATCH] core: BoxCollection#all returns versions --- lib/vagrant/box_collection.rb | 37 +++---- test/unit/support/isolated_environment.rb | 23 ++++ test/unit/vagrant/box_collection_test.rb | 128 ++++++++++------------ 3 files changed, 98 insertions(+), 90 deletions(-) diff --git a/lib/vagrant/box_collection.rb b/lib/vagrant/box_collection.rb index 1ddc5d10a..6ef8476a8 100644 --- a/lib/vagrant/box_collection.rb +++ b/lib/vagrant/box_collection.rb @@ -188,9 +188,8 @@ module Vagrant # This returns an array of all the boxes on the system, given by # their name and their provider. # - # @return [Array] Array of `[name, provider]` pairs of the boxes - # installed on this system. An optional third element in the array - # may specify `:v1` if the box is a version 1 box. + # @return [Array] Array of `[name, version, provider]` of the boxes + # installed on this system. def all results = [] @@ -203,25 +202,21 @@ module Vagrant box_name = child.basename.to_s - # If this is a V1 box, we still return that name, but specify - # that the box is a V1 box. - if v1_box?(child) - @logger.debug("V1 box found: #{box_name}") - results << [box_name, :virtualbox, :v1] - next - end - - # Otherwise, traverse the subdirectories and see what providers + # Otherwise, traverse the subdirectories and see what versions # we have. - child.children(true).each do |provider| - # Verify this is a potentially valid box. If it looks - # correct enough then include it. - if provider.directory? && provider.join("metadata.json").file? - provider_name = provider.basename.to_s.to_sym - @logger.debug("Box: #{box_name} (#{provider_name})") - results << [box_name, provider_name] - else - @logger.debug("Invalid box, ignoring: #{provider}") + child.children(true).each do |versiondir| + version = versiondir.basename.to_s + + versiondir.children(true).each do |provider| + # Verify this is a potentially valid box. If it looks + # correct enough then include it. + if provider.directory? && provider.join("metadata.json").file? + provider_name = provider.basename.to_s.to_sym + @logger.debug("Box: #{box_name} (#{provider_name})") + results << [box_name, version, provider_name] + else + @logger.debug("Invalid box, ignoring: #{provider}") + end end end end diff --git a/test/unit/support/isolated_environment.rb b/test/unit/support/isolated_environment.rb index 4f8e7552b..f69268ce7 100644 --- a/test/unit/support/isolated_environment.rb +++ b/test/unit/support/isolated_environment.rb @@ -89,6 +89,29 @@ module Unit box_dir end + # Creates a fake box to exist in this environment according + # to the "gen-3" box format. + # + # @param [String] name + # @param [String] version + # @param [String] provider + # @return [Pathname] + def box3(name, version, provider, **opts) + # Create the directory for the box + box_dir = boxes_dir.join(name, version, provider.to_s) + box_dir.mkpath + + # Create the metadata.json for it + box_metadata_file = box_dir.join("metadata.json") + box_metadata_file.open("w") do |f| + f.write(JSON.generate({ + :provider => provider.to_s + })) + end + + box_dir + end + # This creates a "box" file that is a valid V1 box. # # @return [Pathname] Path to the newly created box. diff --git a/test/unit/vagrant/box_collection_test.rb b/test/unit/vagrant/box_collection_test.rb index 021ccf1e6..54cca1ac7 100644 --- a/test/unit/vagrant/box_collection_test.rb +++ b/test/unit/vagrant/box_collection_test.rb @@ -16,6 +16,65 @@ describe Vagrant::BoxCollection do instance.directory.should == environment.boxes_dir end + describe "#all" do + it "should return an empty array when no boxes are there" do + instance.all.should == [] + end + + it "should return the boxes and their providers" do + # Create some boxes + environment.box3("foo", "1.0", :virtualbox) + environment.box3("foo", "1.0", :vmware) + environment.box3("bar", "0", :ec2) + + # Verify some output + results = instance.all + results.length.should == 3 + results.include?(["foo", "1.0", :virtualbox]).should be + results.include?(["foo", "1.0", :vmware]).should be + results.include?(["bar", "0", :ec2]).should be + end + + it 'does not raise an exception when a file appears in the boxes dir' do + Tempfile.new('a_file', environment.boxes_dir) + expect { instance.all }.to_not raise_error + end + end + + describe "finding" do + it "should return nil if the box does not exist" do + instance.find("foo", :i_dont_exist).should be_nil + end + + it "should return a box if the box does exist" do + # Create the "box" + environment.box2("foo", :virtualbox) + + # Actual test + result = instance.find("foo", :virtualbox) + result.should_not be_nil + result.should be_kind_of(box_class) + result.name.should == "foo" + end + + it "should throw an exception if it is a v1 box" do + # Create a V1 box + environment.box1("foo") + + # Test! + expect { instance.find("foo", :virtualbox) }. + to raise_error(Vagrant::Errors::BoxUpgradeRequired) + end + + it "should return nil if there is a V1 box but we're looking for another provider" do + # Create a V1 box + environment.box1("foo") + + # Test + instance.find("foo", :another_provider).should be_nil + end + end + describe "adding" do it "should add a valid box to the system" do box_path = environment.box2_file(:virtualbox) @@ -141,75 +200,6 @@ describe Vagrant::BoxCollection do end end - describe "listing all" do - it "should return an empty array when no boxes are there" do - instance.all.should == [] - end - - it "should return the boxes and their providers" do - # Create some boxes - environment.box2("foo", :virtualbox) - environment.box2("foo", :vmware) - environment.box2("bar", :ec2) - - # Verify some output - results = instance.all - results.length.should == 3 - results.include?(["foo", :virtualbox]).should be - results.include?(["foo", :vmware]).should be - results.include?(["bar", :ec2]).should be - end - - it "should return V1 boxes as well" do - # Create some boxes, including a V1 box - environment.box1("bar") - environment.box2("foo", :vmware) - - # Verify some output - results = instance.all.sort - results.should == [["bar", :virtualbox, :v1], ["foo", :vmware]] - end - - it 'does not raise an exception when a file appears in the boxes dir' do - Tempfile.new('a_file', environment.boxes_dir) - expect { instance.all }.to_not raise_error - end - end - - describe "finding" do - it "should return nil if the box does not exist" do - instance.find("foo", :i_dont_exist).should be_nil - end - - it "should return a box if the box does exist" do - # Create the "box" - environment.box2("foo", :virtualbox) - - # Actual test - result = instance.find("foo", :virtualbox) - result.should_not be_nil - result.should be_kind_of(box_class) - result.name.should == "foo" - end - - it "should throw an exception if it is a v1 box" do - # Create a V1 box - environment.box1("foo") - - # Test! - expect { instance.find("foo", :virtualbox) }. - to raise_error(Vagrant::Errors::BoxUpgradeRequired) - end - - it "should return nil if there is a V1 box but we're looking for another provider" do - # Create a V1 box - environment.box1("foo") - - # Test - instance.find("foo", :another_provider).should be_nil - end - end - describe "upgrading" do it "should upgrade a V1 box to V2" do # Create a V1 box