From 39663f6f11a1923ef183c544da9e15308c32c043 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Sep 2010 07:13:37 -0700 Subject: [PATCH] Finish replacement of "env.error!" with exceptions in VM actions --- lib/vagrant/action/vm/persist.rb | 4 ++-- lib/vagrant/action/vm/provision.rb | 6 ++--- lib/vagrant/action/vm/share_folders.rb | 8 ++----- lib/vagrant/errors.rb | 15 ++++++++++++ templates/locales/en.yml | 20 +++++++++++++--- templates/strings.yml | 16 ------------- test/vagrant/action/vm/persist_test.rb | 6 ++--- test/vagrant/action/vm/provision_test.rb | 25 +++++++------------- test/vagrant/action/vm/share_folders_test.rb | 11 --------- 9 files changed, 51 insertions(+), 60 deletions(-) diff --git a/lib/vagrant/action/vm/persist.rb b/lib/vagrant/action/vm/persist.rb index 29a3e3929..1255148b5 100644 --- a/lib/vagrant/action/vm/persist.rb +++ b/lib/vagrant/action/vm/persist.rb @@ -6,8 +6,8 @@ module Vagrant @app = app # Error the environment if the dotfile is not valid - env.error!(:dotfile_error, :env => env.env) if File.exist?(env.env.dotfile_path) && - !File.file?(env.env.dotfile_path) + raise Errors::PersistDotfileExists.new(:dotfile_path => env.env.dotfile_path) if File.exist?(env.env.dotfile_path) && + !File.file?(env.env.dotfile_path) end def call(env) diff --git a/lib/vagrant/action/vm/provision.rb b/lib/vagrant/action/vm/provision.rb index 9fe3d76a3..c2f810cc4 100644 --- a/lib/vagrant/action/vm/provision.rb +++ b/lib/vagrant/action/vm/provision.rb @@ -12,7 +12,7 @@ module Vagrant def call(env) @app.call(env) - if !env.error? && provisioning_enabled? + if provisioning_enabled? @env.ui.info "vagrant.actions.vm.provision.beginning" @provisioner.provision! end @@ -27,7 +27,7 @@ module Vagrant if provisioner.is_a?(Class) @provisioner = provisioner.new(@env) - return @env.error!(:provisioner_invalid_class) unless @provisioner.is_a?(Provisioners::Base) + raise Errors::ProvisionInvalidClass.new if !@provisioner.is_a?(Provisioners::Base) elsif provisioner.is_a?(Symbol) # We have a few hard coded provisioners for built-ins mapping = { @@ -36,7 +36,7 @@ module Vagrant } provisioner_klass = mapping[provisioner] - return @env.error!(:provisioner_unknown_type, :provisioner => provisioner.to_s) if provisioner_klass.nil? + raise Errors::ProvisionUnknownType.new(:provisioner => provisioner.to_s) if provisioner_klass.nil? @provisioner = provisioner_klass.new(@env) end diff --git a/lib/vagrant/action/vm/share_folders.rb b/lib/vagrant/action/vm/share_folders.rb index 0898b6f10..68dcbe9e5 100644 --- a/lib/vagrant/action/vm/share_folders.rb +++ b/lib/vagrant/action/vm/share_folders.rb @@ -16,12 +16,8 @@ module Vagrant @app.call(env) - if !env.error? - catch_action_exception(env) do - # Only mount and setup shared folders in the absense of an - # error - mount_shared_folders - end + catch_action_exception(env) do + mount_shared_folders end end diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 6e176c1c1..ffe550812 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -149,6 +149,21 @@ module Vagrant error_key(:requires_directory, "vagrant.actions.general.package") end + class PersistDotfileExists < VagrantError + status_code(34) + error_key(:dotfile_error, "vagrant.actions.vm.persist") + end + + class ProvisionInvalidClass < VagrantError + status_code(35) + error_key(:invalid_class, "vagrant.actions.vm.provision") + end + + class ProvisionUnknownType < VagrantError + status_code(36) + error_key(:unknown_type, "vagrant.actions.vm.provision") + end + class SSHAuthenticationFailed < VagrantError status_code(11) error_key(:ssh_authentication_failed) diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 405ed595d..a001c6c54 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -216,10 +216,24 @@ en: exporting: Exporting NFS shared folders... mounting: Mounting NFS shared folders... persist: - persisting: Persisting the VM UUID (%{uuid})... + dotfile_error: |- + The dotfile which Vagrant uses to store the UUID of the project's + virtual machine already exists and is not a file! The dotfile is + currently configured to be '%{dotfile_path}' + + To change this value, please see `config.vagrant.dotfile_name` + + Are you trying to use Vagrant from your home directory? This is the + leading cause of this error message. To resolve this, simply use a + different directory. Or, if you really want to run Vagrant from your + home directory, modify the `config.vagrant.dotfile_name` configuration + key. + persisting: "Persisting the VM UUID (%{uuid})..." provision: - beginning: Beginning provisioning process... - enabled: Provisioning enabled with %{provisioner}... + beginning: "Beginning provisioning process..." + enabled: "Provisioning enabled with %{provisioner}..." + invalid_class: "Provisioners must be an instance of Vagrant::Provisioners::Base" + unknown_type: "Unknown provisioner type: %{provisioner}" resume: resuming: Resuming suspended VM... share_folders: diff --git a/templates/strings.yml b/templates/strings.yml index 2574dadaf..bcdc737bf 100644 --- a/templates/strings.yml +++ b/templates/strings.yml @@ -52,28 +52,12 @@ vagrant box add name uri vagrant box remove name vagrant box list -:dotfile_error: |- - The dotfile which Vagrant uses to store the UUID of the project's - virtual machine already exists and is not a file! The dotfile is - currently configured to be `<%= env.dotfile_path %>` - - To change this value, please see `config.vagrant.dotfile_name` - - Are you trying to use Vagrant from your home directory? This is the - leading cause of this error message. To resolve this, simply use a - different directory. Or, if you really want to run Vagrant from your - home directory, modify the `config.vagrant.dotfile_name` configuration - key. :downloader_file_doesnt_exist: |- The given box does not exist on the file system: <%= source_url %> :package_requires_export: |- Package must be used in conjunction with export. -:provisioner_invalid_class: |- - Provisioners must be an instance of Vagrant::Provisioners::Base -:provisioner_unknown_type: |- - Unknown provisioner type: <%= provisioner %> :ssh_bad_exit_status: |- The following SSH command responded with a non-zero exit status. Vagrant assumes that this means the command failed! diff --git a/test/vagrant/action/vm/persist_test.rb b/test/vagrant/action/vm/persist_test.rb index c8a666c51..2c5196cc3 100644 --- a/test/vagrant/action/vm/persist_test.rb +++ b/test/vagrant/action/vm/persist_test.rb @@ -20,9 +20,9 @@ class PersistVMActionTest < Test::Unit::TestCase should "error environment if dotfile exists but is not a file" do File.expects(:file?).with(@env.env.dotfile_path).returns(false) - @klass.new(@app, @env) - assert @env.error? - assert_equal :dotfile_error, @env.error.first + assert_raises(Vagrant::Errors::PersistDotfileExists) { + @klass.new(@app, @env) + } end should "initialize properly if dotfiles doesn't exist" do diff --git a/test/vagrant/action/vm/provision_test.rb b/test/vagrant/action/vm/provision_test.rb index ef5f62500..a43a9fbfe 100644 --- a/test/vagrant/action/vm/provision_test.rb +++ b/test/vagrant/action/vm/provision_test.rb @@ -65,9 +65,10 @@ class ProvisionVMActionTest < Test::Unit::TestCase should "error environment if the class is not a subclass of the provisioner base" do @prov.expects(:is_a?).with(Vagrant::Provisioners::Base).returns(false) - @instance.load_provisioner - assert @env.error? - assert_equal :provisioner_invalid_class, @env.error.first + + assert_raises(Vagrant::Errors::ProvisionInvalidClass) { + @instance.load_provisioner + } end end @@ -81,11 +82,12 @@ class ProvisionVMActionTest < Test::Unit::TestCase assert_equal instance, @instance.load_provisioner end - should "raise an ActionException if its an unknown symbol" do + should "raise an error if its an unknown symbol" do @env["config"].vm.provisioner = :this_will_never_exist - @instance.load_provisioner - assert @env.error? - assert_equal :provisioner_unknown_type, @env.error.first + + assert_raises(Vagrant::Errors::ProvisionUnknownType) { + @instance.load_provisioner + } end should "set :chef_solo to the ChefSolo provisioner" do @@ -120,15 +122,6 @@ class ProvisionVMActionTest < Test::Unit::TestCase @instance.call(@env) end - - should "not provision if erroneous environment" do - @env.error!(:foo) - - @prov.expects(:provision!).never - @app.expects(:call).with(@env).once - - @instance.call(@env) - end end end end diff --git a/test/vagrant/action/vm/share_folders_test.rb b/test/vagrant/action/vm/share_folders_test.rb index 2bb5df986..1e570a354 100644 --- a/test/vagrant/action/vm/share_folders_test.rb +++ b/test/vagrant/action/vm/share_folders_test.rb @@ -44,17 +44,6 @@ class ShareFoldersVMActionTest < Test::Unit::TestCase @instance.call(@env) end - - should "run only the metadata actions if erroneous environment" do - @env.error!(:foo) - - before_seq = sequence("before") - @instance.expects(:create_metadata).once.in_sequence(before_seq) - @app.expects(:call).with(@env).in_sequence(before_seq) - @instance.expects(:mount_shared_folders).never - - @instance.call(@env) - end end context "collecting shared folders" do