Refactor create domain tests (#1364)

Adjust create domain tests to exercise both with a box defined and
undefined. Switch the default vagrantfile definition to have a box
defined as it is the expected behaviour.

spec/support/sharedcontext.rb

@@ -9,6 +9,7 @@ shared_context 'unit' do
   let(:vagrantfile) do
     <<-EOF
     Vagrant.configure('2') do |config|
+      config.vm.box = "vagrant-libvirt/test"
       config.vm.define :test
       config.vm.provider :libvirt do |libvirt|
         #{vagrantfile_providerconfig}

spec/unit/action/create_domain_spec.rb

@@ -5,6 +5,7 @@ require 'support/sharedcontext'
 require 'support/libvirt_context'
 
 require 'vagrant-libvirt/errors'
+require 'vagrant-libvirt/util/byte_number'
 require 'vagrant-libvirt/action/create_domain'
 
 describe VagrantPlugins::ProviderLibvirt::Action::CreateDomain do
@@ -16,6 +17,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::CreateDomain do
   let(:libvirt_client) { double('libvirt_client') }
   let(:servers) { double('servers') }
   let(:volumes) { double('volumes') }
+  let(:domain_volume) { double('domain_volume') }
 
   let(:domain_xml) { File.read(File.join(File.dirname(__FILE__), File.basename(__FILE__, '.rb'), domain_xml_file)) }
   let(:storage_pool_xml) { File.read(File.join(File.dirname(__FILE__), File.basename(__FILE__, '.rb'), storage_pool_xml_file)) }
@@ -29,9 +31,15 @@ describe VagrantPlugins::ProviderLibvirt::Action::CreateDomain do
 
       allow(connection).to receive(:servers).and_return(servers)
       allow(connection).to receive(:volumes).and_return(volumes)
+      allow(volumes).to receive(:all).and_return([domain_volume])
+      allow(domain_volume).to receive(:pool_name).and_return('default')
+      allow(domain_volume).to receive(:[]).with('name').and_return('vagrant-test_default.img')
+      allow(domain_volume).to receive(:path).and_return('/var/lib/libvirt/images/vagrant-test_default.img')
+      allow(machine).to receive_message_chain("box.name") { 'vagrant-libvirt/test' }
 
       allow(logger).to receive(:info)
       allow(logger).to receive(:debug)
+      allow(ui).to receive(:info)
 
       env[:domain_name] = "vagrant-test_default"
 
@@ -39,7 +47,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::CreateDomain do
       env[:box_volumes].push({
         :path=>"/test/box.img",
         :name=>"test_vagrant_box_image_1.1.1_0.img", 
-        :virtual_size=>5
+        :virtual_size=> ByteNumber.new(5),
       })
       # should be ignored for system session and used for user session
       allow(Process).to receive(:uid).and_return(9999)
@@ -47,35 +55,44 @@ describe VagrantPlugins::ProviderLibvirt::Action::CreateDomain do
     end
 
     context 'connection => qemu:///system' do
-      context 'default pool' do
-        let(:domain_xml_file) { 'default_domain.xml' }
-        let(:storage_pool_xml_file) { 'default_system_storage_pool.xml' }
+      let(:domain_xml_file) { 'default_domain.xml' }
 
+      context 'default pool' do
         it 'should execute correctly' do
-          expect(libvirt_client).to receive(:lookup_storage_pool_by_name).and_return(libvirt_storage_pool)
-          expect(libvirt_storage_pool).to receive(:xml_desc).and_return(storage_pool_xml)
           expect(servers).to receive(:create).with(xml: domain_xml).and_return(machine)
           expect(volumes).to_not receive(:create) # additional disks only
 
           expect(subject.call(env)).to be_nil
         end
 
-        context 'additional disks' do
+        context 'with no box' do
+          let(:storage_pool_xml_file) { 'default_system_storage_pool.xml' }
           let(:vagrantfile) do
             <<-EOF
             Vagrant.configure('2') do |config|
               config.vm.define :test
-              config.vm.provider :libvirt do |libvirt|
-                libvirt.storage :file, :size => '20G'
-              end
             end
             EOF
           end
 
+          it 'should query for the storage pool path' do
+            expect(libvirt_client).to receive(:lookup_storage_pool_by_name).and_return(libvirt_storage_pool)
+            expect(libvirt_storage_pool).to receive(:xml_desc).and_return(storage_pool_xml)
+            expect(servers).to receive(:create).and_return(machine)
+
+            expect(subject.call(env)).to be_nil
+          end
+        end
+
+        context 'additional disks' do
+          let(:vagrantfile_providerconfig) do
+            <<-EOF
+            libvirt.storage :file, :size => '20G'
+            EOF
+          end
+
           context 'volume create failed' do
             it 'should raise an exception' do
-              expect(libvirt_client).to receive(:lookup_storage_pool_by_name).and_return(libvirt_storage_pool)
-              expect(libvirt_storage_pool).to receive(:xml_desc).and_return(storage_pool_xml)
               expect(volumes).to receive(:create).and_raise(Libvirt::Error)
 
               expect{ subject.call(env) }.to raise_error(VagrantPlugins::ProviderLibvirt::Errors::FogCreateDomainVolumeError)
@@ -86,8 +103,6 @@ describe VagrantPlugins::ProviderLibvirt::Action::CreateDomain do
             let(:domain_xml_file) { 'additional_disks_domain.xml' }
 
             it 'should complete' do
-              expect(libvirt_client).to receive(:lookup_storage_pool_by_name).and_return(libvirt_storage_pool)
-              expect(libvirt_storage_pool).to receive(:xml_desc).and_return(storage_pool_xml)
               expect(volumes).to receive(:create).with(
                 hash_including(
                   :path        => "/var/lib/libvirt/images/vagrant-test_default-vdb.qcow2",
@@ -105,6 +120,14 @@ describe VagrantPlugins::ProviderLibvirt::Action::CreateDomain do
       end
 
       context 'no default pool' do
+        let(:vagrantfile) do
+          <<-EOF
+          Vagrant.configure('2') do |config|
+            config.vm.define :test
+          end
+          EOF
+        end
+
         it 'should raise an exception' do
           expect(libvirt_client).to receive(:lookup_storage_pool_by_name).and_return(nil)
 
@@ -114,45 +137,51 @@ describe VagrantPlugins::ProviderLibvirt::Action::CreateDomain do
     end
 
     context 'connection => qemu:///session' do
-      let(:vagrantfile) do
+      let(:vagrantfile_providerconfig) do
         <<-EOF
-        Vagrant.configure('2') do |config|
-          config.vm.define :test
-          config.vm.provider :libvirt do |libvirt|
-            libvirt.qemu_use_session = true
-          end
-        end
+        libvirt.qemu_use_session = true
         EOF
       end
 
       context 'default pool' do
-        let(:storage_pool_xml_file) { 'default_user_storage_pool.xml' }
-
         it 'should execute correctly' do
-          expect(libvirt_client).to receive(:lookup_storage_pool_by_name).and_return(libvirt_storage_pool)
-          expect(libvirt_storage_pool).to receive(:xml_desc).and_return(storage_pool_xml)
           expect(servers).to receive(:create).and_return(machine)
 
           expect(subject.call(env)).to be_nil
         end
 
-        context 'additional disks' do
+        context 'with no box' do
+          let(:storage_pool_xml_file) { 'default_user_storage_pool.xml' }
           let(:vagrantfile) do
             <<-EOF
             Vagrant.configure('2') do |config|
               config.vm.define :test
               config.vm.provider :libvirt do |libvirt|
-                libvirt.qemu_use_session = true
-                libvirt.storage :file, :size => '20G'
+                #{vagrantfile_providerconfig}
               end
             end
             EOF
           end
 
+          it 'should query for the storage pool path' do
+            expect(libvirt_client).to receive(:lookup_storage_pool_by_name).and_return(libvirt_storage_pool)
+            expect(libvirt_storage_pool).to receive(:xml_desc).and_return(storage_pool_xml)
+            expect(servers).to receive(:create).and_return(machine)
+
+            expect(subject.call(env)).to be_nil
+          end
+        end
+
+        context 'additional disks' do
+          let(:vagrantfile_providerconfig) do
+            <<-EOF
+            libvirt.qemu_use_session = true
+            libvirt.storage :file, :size => '20G'
+            EOF
+          end
+
           context 'volume create succeeded' do
             it 'should complete' do
-              expect(libvirt_client).to receive(:lookup_storage_pool_by_name).and_return(libvirt_storage_pool)
-              expect(libvirt_storage_pool).to receive(:xml_desc).and_return(storage_pool_xml)
               expect(volumes).to receive(:create).with(
                 hash_including(
                   :path        => "/var/lib/libvirt/images/vagrant-test_default-vdb.qcow2",

spec/unit/action/create_domain_spec/additional_disks_domain.xml

@@ -26,6 +26,11 @@
   <clock offset='utc'>
   </clock>
   <devices>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2' cache='default'/>
+      <source file='/var/lib/libvirt/images/vagrant-test_default.img'/>
+      <target dev='vda' bus='virtio'/>
+    </disk>
     <disk type='file' device='disk'>
       <driver name='qemu' type='qcow2' cache='default'/>
       <source file='/var/lib/libvirt/images/vagrant-test_default-vdb.qcow2'/>

spec/unit/action/create_domain_spec/default_domain.xml

@@ -26,6 +26,11 @@
   <clock offset='utc'>
   </clock>
   <devices>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2' cache='default'/>
+      <source file='/var/lib/libvirt/images/vagrant-test_default.img'/>
+      <target dev='vda' bus='virtio'/>
+    </disk>
 
 
     <serial type='pty'>