From eadb0ac83113f0c4d388d8e9d152df00e425ff13 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 26 May 2017 15:07:39 -0700 Subject: [PATCH 1/2] Raise exception if provider doesn't have snapshot capability Prior to this commit, if a user attempted to use the `vagrant snapshot save` or `vagrant snapshot list` commands on a vm whose provider did not support snapshots, it would simply print a warning. This commit changes that behavior by instead raising an error. --- lib/vagrant/errors.rb | 4 ++++ plugins/commands/snapshot/command/list.rb | 3 +-- plugins/commands/snapshot/command/save.rb | 3 +-- templates/locales/en.yml | 6 ++++++ .../commands/snapshot/command/save_test.rb | 15 +++++++++++++++ 5 files changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index f1c2cb3e4..b3255e534 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -656,6 +656,10 @@ module Vagrant error_key(:snapshot_force) end + class SnapshotNotSupported < VagrantError + error_key(:snapshot_not_supported) + end + class SSHAuthenticationFailed < VagrantError error_key(:ssh_authentication_failed) end diff --git a/plugins/commands/snapshot/command/list.rb b/plugins/commands/snapshot/command/list.rb index d0c433b94..1ee77aac4 100644 --- a/plugins/commands/snapshot/command/list.rb +++ b/plugins/commands/snapshot/command/list.rb @@ -22,8 +22,7 @@ module VagrantPlugins end if !vm.provider.capability?(:snapshot_list) - vm.ui.info(I18n.t("vagrant.commands.snapshot.not_supported")) - next + raise Vagrant::Errors::SnapshotNotSupported end snapshots = vm.provider.capability(:snapshot_list) diff --git a/plugins/commands/snapshot/command/save.rb b/plugins/commands/snapshot/command/save.rb index 28d9afe0c..e380c228e 100644 --- a/plugins/commands/snapshot/command/save.rb +++ b/plugins/commands/snapshot/command/save.rb @@ -34,8 +34,7 @@ module VagrantPlugins name = argv.pop with_target_vms(argv) do |vm| if !vm.provider.capability?(:snapshot_list) - vm.ui.info(I18n.t("vagrant.commands.snapshot.not_supported")) - next + raise Vagrant::Errors::SnapshotNotSupported end snapshot_list = vm.provider.capability(:snapshot_list) diff --git a/templates/locales/en.yml b/templates/locales/en.yml index f4e182a31..505a8f48a 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1158,6 +1158,12 @@ en: guest system. Please report a bug with your Vagrantfile and debug log. snapshot_force: |- You must include the `--force` option to replace an existing snapshot. + snapshot_not_supported: |- + This provider doesn't support snapshots. + + This may be intentional or this may be a bug. If this provider + should support snapshots, then please report this as a bug to the + maintainer of the provider. ssh_authentication_failed: |- SSH authentication failed! This is typically caused by the public/private keypair for the SSH user not being properly set on the guest VM. Please diff --git a/test/unit/plugins/commands/snapshot/command/save_test.rb b/test/unit/plugins/commands/snapshot/command/save_test.rb index 2dbdf4145..3ed2f1ffb 100644 --- a/test/unit/plugins/commands/snapshot/command/save_test.rb +++ b/test/unit/plugins/commands/snapshot/command/save_test.rb @@ -38,6 +38,21 @@ describe VagrantPlugins::CommandSnapshot::Command::Save do end end + context "with an unsupported provider" do + let(:argv) { ["test"] } + + before do + allow(machine.provider).to receive(:capability?).with(:snapshot_list). + and_return(false) + end + + it "raises an exception" do + machine.id = "foo" + expect { subject.execute }. + to raise_error(Vagrant::Errors::SnapshotNotSupported) + end + end + context "with a snapshot name given" do let(:argv) { ["test"] } it "calls snapshot_save with a snapshot name" do From b349d664e49de70a90bb2174ba8e122ad89a8e1b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 30 May 2017 09:52:57 -0700 Subject: [PATCH 2/2] Include snapshot list unit test This commit introduces a new set of unit tests for the snapshot list command. --- .../commands/snapshot/command/list_test.rb | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 test/unit/plugins/commands/snapshot/command/list_test.rb diff --git a/test/unit/plugins/commands/snapshot/command/list_test.rb b/test/unit/plugins/commands/snapshot/command/list_test.rb new file mode 100644 index 000000000..a812aa40f --- /dev/null +++ b/test/unit/plugins/commands/snapshot/command/list_test.rb @@ -0,0 +1,83 @@ +require File.expand_path("../../../../../base", __FILE__) + +require Vagrant.source_root.join("plugins/commands/snapshot/command/list") + +describe VagrantPlugins::CommandSnapshot::Command::List do + include_context "unit" + + 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 + + let(:guest) { double("guest") } + let(:host) { double("host") } + let(:machine) { iso_env.machine(iso_env.machine_names[0], :dummy) } + + let(:argv) { [] } + + subject { described_class.new(argv, iso_env) } + + before do + allow(machine.provider).to receive(:capability?).with(:snapshot_list). + and_return(true) + + allow(machine.provider).to receive(:capability).with(:snapshot_list). + and_return([]) + + allow(subject).to receive(:with_target_vms) { |&block| block.call machine } + end + + describe "execute" do + context "with an unsupported provider" do + let(:argv) { ["foo"] } + + before do + allow(machine.provider).to receive(:capability?).with(:snapshot_list). + and_return(false) + end + + it "raises an exception" do + machine.id = "foo" + expect { subject.execute }. + to raise_error(Vagrant::Errors::SnapshotNotSupported) + end + end + + context "with a vm given" do + let(:argv) { ["foo"] } + + it "prints a message if the vm does not exist" do + machine.id = nil + + expect(iso_env.ui).to receive(:info).with { |message, _| + expect(message).to include("VM not created") + } + expect(machine).to_not receive(:action) + expect(subject.execute).to eq(0) + end + + it "prints a message if no snapshots have been taken" do + machine.id = "foo" + + expect(iso_env.ui).to receive(:output) + .with(/No snapshots have been taken yet!/, anything) + expect(subject.execute).to eq(0) + end + + it "prints a list of snapshots" do + machine.id = "foo" + + allow(machine.provider).to receive(:capability).with(:snapshot_list). + and_return(["foo", "bar", "baz"]) + + expect(iso_env.ui).to receive(:output).with(/foo/, anything) + expect(iso_env.ui).to receive(:output).with(/bar/, anything) + expect(iso_env.ui).to receive(:output).with(/baz/, anything) + expect(subject.execute).to eq(0) + end + end + end +end