Change VM loading to be lazy loaded so config can be loaded without VMs

lib/vagrant/environment.rb

@@ -100,7 +100,7 @@ module Vagrant
     def vms
       return parent.vms if parent
       load! if !loaded?
-      @vms ||= {}
+      @vms ||= load_vms!
     end
 
     # Returns the primray VM associated with this environment
@@ -227,7 +227,6 @@ module Vagrant
         @loaded = true
         self.class.check_virtualbox!
         load_config!
-        load_vm!
         actions.run(:environment_load)
       end
 
@@ -287,33 +286,23 @@ module Vagrant
     end
 
     # Loads the persisted VM (if it exists) for this environment.
-    def load_vm!
-      # This environment represents a single sub VM. The VM is then
-      # probably (read: should be) set on the VM attribute, so we do
-      # nothing.
-      return if parent
-
-      # First load the defaults (blank, noncreated VMs)
-      load_blank_vms!
+    def load_vms!
+      result = {}
 
       # Load the VM UUIDs from the local data store
       (local_data[:active] || {}).each do |name, uuid|
-        vms[name.to_sym] = Vagrant::VM.find(uuid, self, name.to_sym)
+        result[name.to_sym] = Vagrant::VM.find(uuid, self, name.to_sym)
       end
-    end
 
-    # Loads blank VMs into the `vms` attribute.
-    def load_blank_vms!
-      # Clear existing vms
-      vms.clear
-
-      # Load up the blank VMs
-      defined_vms = config.vm.defined_vms.keys
-      defined_vms = [DEFAULT_VM] if defined_vms.empty?
-
-      defined_vms.each do |name|
-        vms[name] = Vagrant::VM.new(:name => name, :env => self)
+      # For any VMs which aren't created, create a blank VM instance for
+      # them
+      all_keys = config.vm.defined_vms.keys
+      all_keys = [DEFAULT_VM] if all_keys.empty?
+      all_keys.each do |name|
+        result[name] = Vagrant::VM.new(:name => name, :env => self) if !result.has_key?(name)
       end
+
+      result
     end
   end
 end

test/vagrant/environment_test.rb

@@ -193,15 +193,13 @@ class EnvironmentTest < Test::Unit::TestCase
       assert env.global_data.equal?(store)
     end
 
-    should "return the parent's local data if a parent exists" do
+    should "return the parent's global data if a parent exists" do
       env = vagrant_env(vagrantfile(<<-vf))
         config.vm.define :web
         config.vm.define :db
       vf
 
       result = env.global_data
-
-      Vagrant::DataStore.expects(:new).never
       assert env.vms[:web].env.global_data.equal?(result)
     end
   end
@@ -323,8 +321,9 @@ class EnvironmentTest < Test::Unit::TestCase
   context "accessing the VMs hash" do
     should "load the environment if its not already loaded" do
       env = @klass.new(:cwd => vagrantfile)
-      env.expects(:load!).once
+      assert !env.loaded?
       env.vms
+      assert env.loaded?
     end
 
     should "not load the environment if its already loaded" do
@@ -350,7 +349,6 @@ class EnvironmentTest < Test::Unit::TestCase
         call_seq = sequence("call_sequence")
         @klass.expects(:check_virtualbox!).once.in_sequence(call_seq)
         env.expects(:load_config!).once.in_sequence(call_seq)
-        env.expects(:load_vm!).once.in_sequence(call_seq)
         env.actions.expects(:run).with(:environment_load).once.in_sequence(call_seq)
         assert_equal env, env.load!
       end
@@ -445,13 +443,6 @@ class EnvironmentTest < Test::Unit::TestCase
         @env = vagrant_env
       end
 
-      should "blank the VMs" do
-        load_seq = sequence("load_seq")
-        @env.stubs(:root_path).returns("foo")
-        @env.expects(:load_blank_vms!).in_sequence(load_seq)
-        @env.load_vm!
-      end
-
       should "load all the VMs from the dotfile" do
         @env.local_data[:active] = { "foo" => "bar", "bar" => "baz" }
 
@@ -462,56 +453,18 @@ class EnvironmentTest < Test::Unit::TestCase
           results[key.to_sym] = vm
         end
 
-        @env.load_vm!
+        returned = @env.load_vms!
 
         results.each do |key, value|
-          assert_equal value, @env.vms[key]
+          assert_equal value, returned[key]
         end
       end
 
-      should "do nothing if the parent is set" do
-        env = vagrant_env(vagrantfile(<<-vf))
-          config.vm.define :web
-          config.vm.define :db
-        vf
-
-        subenv = env.vms[:web].env
-        subenv.expects(:load_blank_vms!).never
-        subenv.load_vm!
-      end
-
       should "uuid should be nil if local data contains nothing" do
         assert @env.local_data.empty? # sanity
-        @env.load_vm!
+        @env.load_vms!
         assert_nil @env.vm
       end
     end
-
-    context "loading blank VMs" do
-      should "blank the VMs" do
-        env = vagrant_env(vagrantfile(<<-vf))
-          config.vm.define :foo
-          config.vm.define :bar
-        vf
-
-        env.load_blank_vms!
-
-        assert_equal 2, env.vms.length
-        assert(env.vms.all? { |name, vm| !vm.created? })
-
-        sorted_vms = env.vms.keys.sort { |a,b| a.to_s <=> b.to_s }
-        assert_equal [:bar, :foo], sorted_vms
-      end
-
-      should "load the default VM blank if no multi-VMs are specified" do
-        env = vagrant_env
-        assert env.config.vm.defined_vms.empty? # sanity
-
-        env.load_blank_vms!
-
-        assert_equal 1, env.vms.length
-        assert !env.vms.values.first.created?
-      end
-    end
   end
 end