From 8190fba8729685df63659a5b85f3bc4214bf557f Mon Sep 17 00:00:00 2001 From: Gilles Cornu Date: Wed, 25 May 2016 09:12:14 +0200 Subject: [PATCH] provisoners/ansible: improve config test coverage Changes: - Add "config" unit tests for `ansible_local` (guest) - Share some "config" examples between both ansible provisioners - Move `config_host.rb` specific examples to `config/host.rb` - Add a requirement to "../helpers" in `config/guest.rb` in order to be able to run the related unit tests References: - This is the first part of GH-6633 resolution - This change is a handy prerequisite for GH-6570 Not addressed yet: - FIXME (guest.rb): Some test-double stubs are currently not working as expected, and the related checks are commented out for the moment (no idea why, but this is not urgent to be fixed because of GH-7335 rejection. See also GH-6984) - FIXME (shared.rb): The guest-based config should actually NOT return an error when the extra_vars file cannot be found, but only display a a warning (similarly to the changes done for GH-6763, see 4e451c6) --- plugins/provisioners/ansible/config/guest.rb | 1 + .../provisioners/ansible/config/guest_test.rb | 163 ++++++++++++++++++ .../{config_test.rb => config/host_test.rb} | 144 +++------------- .../provisioners/ansible/config/shared.rb | 124 +++++++++++++ 4 files changed, 309 insertions(+), 123 deletions(-) create mode 100644 test/unit/plugins/provisioners/ansible/config/guest_test.rb rename test/unit/plugins/provisioners/ansible/{config_test.rb => config/host_test.rb} (55%) create mode 100644 test/unit/plugins/provisioners/ansible/config/shared.rb diff --git a/plugins/provisioners/ansible/config/guest.rb b/plugins/provisioners/ansible/config/guest.rb index 86ace3b58..31507ad04 100644 --- a/plugins/provisioners/ansible/config/guest.rb +++ b/plugins/provisioners/ansible/config/guest.rb @@ -1,4 +1,5 @@ require_relative "base" +require_relative "../helpers" module VagrantPlugins module Ansible diff --git a/test/unit/plugins/provisioners/ansible/config/guest_test.rb b/test/unit/plugins/provisioners/ansible/config/guest_test.rb new file mode 100644 index 000000000..56f1366c6 --- /dev/null +++ b/test/unit/plugins/provisioners/ansible/config/guest_test.rb @@ -0,0 +1,163 @@ +require_relative "../../../../base" +require_relative "../../support/shared/config" +require_relative "shared" + +require Vagrant.source_root.join("plugins/provisioners/ansible/config/guest") + +describe VagrantPlugins::Ansible::Config::Guest do + include_context "unit" + + subject { described_class.new } + + # FIXME: machine.ui.warn stub is not working as expected... + let(:machine) { double("machine", env: Vagrant::Environment.new) } + + let(:communicator) { double("communicator") } + let(:existing_file) { "this/path/is/a/stub" } + let(:non_existing_file) { "this/path/does/not/exist" } + + it "supports a list of options" do + supported_options = %w( extra_vars + galaxy_command + galaxy_role_file + galaxy_roles_path + groups + host_vars + install + inventory_path + limit + playbook + provisioning_path + raw_arguments + skip_tags + start_at_task + sudo + sudo_user + tags + tmp_path + vault_password_file + verbose + version ) + + expect(get_provisioner_option_names(described_class)).to eql(supported_options) + end + + describe "default options handling" do + it_behaves_like "options shared by both Ansible provisioners" + + it "assigns default values to unset guest-specific options" do + subject.finalize! + + expect(subject.install).to be_true + expect(subject.provisioning_path).to eql("/vagrant") + expect(subject.tmp_path).to eql("/tmp/vagrant-ansible") + expect(subject.version).to be_empty + end + end + + describe "#validate" do + before do + machine.stub(communicate: communicator) + end + + context "when the machine is not ready to communicate" do + before do + allow(communicator).to receive(:ready?).and_return(false) + end + + it "cannot check the existence of remote file" do + subject.playbook = non_existing_file + subject.finalize! + + result = subject.validate(machine) + expect(result["ansible local provisioner"]).to eql([]) + # FIXME: commented out because this `communicator.ready?` stub is not working as expected + # expect(communicator).to receive(:ready?).ordered + # Note that communicator mock will fail if it receives an unexpected message, + # which is part of this spec. + end + end + + context "when the machine is ready to communicate" do + before do + allow(communicator).to receive(:ready?).and_return(true) + allow(communicator).to receive(:test).and_return(false) + + allow(communicator).to receive(:test) do |arg1| + arg1.include?("#{existing_file}") + end + + stubbed_ui = Vagrant::UI::Colored.new + machine.stub(ui: stubbed_ui) + allow(machine.ui).to receive(:warn) + + subject.playbook = existing_file + end + + it_behaves_like "an Ansible provisioner", "/vagrant", "local" + + it "only shows a warning if the playbook file does not exist" do + subject.playbook = non_existing_file + subject.finalize! + + result = subject.validate(machine) + expect(result["ansible remote provisioner"]).to be_nil + + # FIXME + # expect(machine).to receive(:ui).with { |warning_text| + # expect(warning_text).to eq("`playbook` does not exist on the guest: /vagrant/this/path/does/not/exist") + # } + end + + it "only shows a warning if inventory_path is specified, but does not exist" do + subject.inventory_path = non_existing_file + subject.finalize! + + result = subject.validate(machine) + expect(result["ansible remote provisioner"]).to be_nil + + # FIXME + # expect(machine.ui).to receive(:warn).with { |warning_text| + # expect(warning_text).to eq("`inventory_path` does not exist on the guest: this/path/does/not/exist") + # } + end + + it "only shows a warning if vault_password_file is specified, but does not exist" do + subject.vault_password_file = non_existing_file + subject.finalize! + + result = subject.validate(machine) + expect(result["ansible remote provisioner"]).to be_nil + + # FIXME + # expect(machine.ui).to receive(:warn).with { |warning_text| + # expect(warning_text).to eq("`inventory_path` does not exist on the guest: this/path/does/not/exist") + # } + end + + it "it doesn't consider missing files on the remote system as errors" do + subject.playbook = non_existing_file + subject.inventory_path = non_existing_file + subject.extra_vars = non_existing_file + subject.finalize! + + result = subject.validate(machine) + expect(result["ansible local provisioner"]).to include( + I18n.t("vagrant.provisioners.ansible.errors.extra_vars_invalid", + type: subject.extra_vars.class.to_s, + value: subject.extra_vars.to_s)) + + expect(result["ansible local provisioner"]).to_not include( + I18n.t("vagrant.provisioners.ansible.errors.playbook_path_invalid", + path: File.join("/vagrant", non_existing_file), system: "guest")) + + expect(result["ansible local provisioner"]).to_not include( + I18n.t("vagrant.provisioners.ansible.errors.inventory_path_invalid", + path: File.join("/vagrant", non_existing_file), system: "guest")) + end + + end + + end + +end diff --git a/test/unit/plugins/provisioners/ansible/config_test.rb b/test/unit/plugins/provisioners/ansible/config/host_test.rb similarity index 55% rename from test/unit/plugins/provisioners/ansible/config_test.rb rename to test/unit/plugins/provisioners/ansible/config/host_test.rb index 5ba9d638d..8ebe6e35a 100644 --- a/test/unit/plugins/provisioners/ansible/config_test.rb +++ b/test/unit/plugins/provisioners/ansible/config/host_test.rb @@ -1,26 +1,19 @@ -require_relative "../../../base" -require_relative "../support/shared/config" - -require "vagrant/util/platform" +require_relative "../../../../base" +require_relative "../../support/shared/config" +require_relative "shared" require Vagrant.source_root.join("plugins/provisioners/ansible/config/host") -describe VagrantPlugins::Ansible::Config::Host do +describe VagrantPlugins::Ansible::Config::Host, :skip_windows => true do include_context "unit" subject { described_class.new } let(:machine) { double("machine", env: Vagrant::Environment.new) } let(:existing_file) { File.expand_path(__FILE__) } - let(:non_existing_file) do - next "/this/does/not/exist" if !Vagrant::Util::Platform.windows? - "C:/foo/nope/nope" - end + let(:non_existing_file) { "/this/does/not/exist" } it "supports a list of options" do - config_options = subject.public_methods(false).find_all { |i| i.to_s.end_with?('=') } - config_options.map! { |i| i.to_s.sub('=', '') } - supported_options = %w( ask_sudo_pass ask_vault_pass extra_vars @@ -47,27 +40,18 @@ describe VagrantPlugins::Ansible::Config::Host do expect(get_provisioner_option_names(described_class)).to eql(supported_options) end - it "assigns default values to unset options" do - subject.finalize! + describe "default options handling" do + it_behaves_like "options shared by both Ansible provisioners" - expect(subject.playbook).to be_nil - expect(subject.extra_vars).to be_nil - expect(subject.force_remote_user).to be_true - expect(subject.ask_sudo_pass).to be_false - expect(subject.ask_vault_pass).to be_false - expect(subject.vault_password_file).to be_nil - expect(subject.limit).to be_nil - expect(subject.sudo).to be_false - expect(subject.sudo_user).to be_nil - expect(subject.verbose).to be_false - expect(subject.tags).to be_nil - expect(subject.skip_tags).to be_nil - expect(subject.start_at_task).to be_nil - expect(subject.host_vars).to eq({}) - expect(subject.groups).to eq({}) - expect(subject.host_key_checking).to be_false - expect(subject.raw_arguments).to be_nil - expect(subject.raw_ssh_args).to be_nil + it "assigns default values to unset host-specific options" do + subject.finalize! + + expect(subject.ask_sudo_pass).to be_false + expect(subject.ask_vault_pass).to be_false + expect(subject.force_remote_user).to be_true + expect(subject.host_key_checking).to be_false + expect(subject.raw_ssh_args).to be_nil + end end describe "force_remote_user option" do @@ -82,31 +66,13 @@ describe VagrantPlugins::Ansible::Config::Host do describe "ask_vault_pass option" do it_behaves_like "any VagrantConfigProvisioner strict boolean attribute", :ask_sudo_pass, false end - describe "sudo option" do - it_behaves_like "any VagrantConfigProvisioner strict boolean attribute", :sudo, false - end describe "#validate" do before do subject.playbook = existing_file end - it "passes if the playbook option refers to an existing file" do - subject.finalize! - - result = subject.validate(machine) - expect(result["ansible remote provisioner"]).to eql([]) - end - - it "returns an error if the playbook option is undefined" do - subject.playbook = nil - subject.finalize! - - result = subject.validate(machine) - expect(result["ansible remote provisioner"]).to eql([ - I18n.t("vagrant.provisioners.ansible.errors.no_playbook") - ]) - end + it_behaves_like "an Ansible provisioner", "", "remote" it "returns an error if the playbook file does not exist" do subject.playbook = non_existing_file @@ -119,54 +85,17 @@ describe VagrantPlugins::Ansible::Config::Host do ]) end - it "passes if the extra_vars option refers to an existing file" do - subject.extra_vars = existing_file - subject.finalize! - - result = subject.validate(machine) - expect(result["ansible remote provisioner"]).to eql([]) - end - - it "passes if the extra_vars option is a hash" do - subject.extra_vars = { var1: 1, var2: "foo" } - subject.finalize! - - result = subject.validate(machine) - expect(result["ansible remote provisioner"]).to eql([]) - end - - it "returns an error if the extra_vars option refers to a file that does not exist" do - subject.extra_vars = non_existing_file + it "returns an error if galaxy_role_file is specified, but does not exist" do + subject.galaxy_role_file = non_existing_file subject.finalize! result = subject.validate(machine) expect(result["ansible remote provisioner"]).to eql([ - I18n.t("vagrant.provisioners.ansible.errors.extra_vars_invalid", - type: subject.extra_vars.class.to_s, - value: subject.extra_vars.to_s) + I18n.t("vagrant.provisioners.ansible.errors.galaxy_role_file_invalid", + path: non_existing_file, system: "host") ]) end - it "returns an error if the extra_vars option is of wrong data type" do - subject.extra_vars = ["var1", 3, "var2", 5] - subject.finalize! - - result = subject.validate(machine) - expect(result["ansible remote provisioner"]).to eql([ - I18n.t("vagrant.provisioners.ansible.errors.extra_vars_invalid", - type: subject.extra_vars.class.to_s, - value: subject.extra_vars.to_s) - ]) - end - - it "passes if inventory_path refers to an existing location" do - subject.inventory_path = existing_file - subject.finalize! - - result = subject.validate(machine) - expect(result["ansible remote provisioner"]).to eql([]) - end - it "returns an error if inventory_path is specified, but does not exist" do subject.inventory_path = non_existing_file subject.finalize! @@ -189,37 +118,6 @@ describe VagrantPlugins::Ansible::Config::Host do ]) end - it "returns an error if galaxy_role_file is specified, but does not exist" do - subject.galaxy_role_file = non_existing_file - subject.finalize! - - result = subject.validate(machine) - expect(result["ansible remote provisioner"]).to eql([ - I18n.t("vagrant.provisioners.ansible.errors.galaxy_role_file_invalid", - path: non_existing_file, system: "host") - ]) - end - - it "returns an error if the raw_arguments is of the wrong data type" do - subject.raw_arguments = { arg1: 1, arg2: "foo" } - subject.finalize! - - result = subject.validate(machine) - expect(result["ansible remote provisioner"]).to eql([ - I18n.t("vagrant.provisioners.ansible.errors.raw_arguments_invalid", - type: subject.raw_arguments.class.to_s, - value: subject.raw_arguments.to_s) - ]) - end - - it "converts a raw_arguments option defined as a String into an Array" do - subject.raw_arguments = "--foo=bar" - subject.finalize! - - result = subject.validate(machine) - expect(subject.raw_arguments).to eql(%w(--foo=bar)) - end - it "returns an error if the raw_ssh_args is of the wrong data type" do subject.raw_ssh_args = { arg1: 1, arg2: "foo" } subject.finalize! diff --git a/test/unit/plugins/provisioners/ansible/config/shared.rb b/test/unit/plugins/provisioners/ansible/config/shared.rb new file mode 100644 index 000000000..b3364e855 --- /dev/null +++ b/test/unit/plugins/provisioners/ansible/config/shared.rb @@ -0,0 +1,124 @@ +shared_examples_for 'options shared by both Ansible provisioners' do + + it "assigns default values to unset common options" do + subject.finalize! + + expect(subject.extra_vars).to be_nil + expect(subject.galaxy_command).to eql("ansible-galaxy install --role-file=%{role_file} --roles-path=%{roles_path} --force") + expect(subject.galaxy_role_file).to be_nil + expect(subject.galaxy_roles_path).to be_nil + expect(subject.groups).to eq({}) + expect(subject.host_vars).to eq({}) + expect(subject.inventory_path).to be_nil + expect(subject.limit).to be_nil + expect(subject.playbook).to be_nil + expect(subject.raw_arguments).to be_nil + expect(subject.skip_tags).to be_nil + expect(subject.start_at_task).to be_nil + expect(subject.sudo).to be_false + expect(subject.sudo_user).to be_nil + expect(subject.tags).to be_nil + expect(subject.vault_password_file).to be_nil + expect(subject.verbose).to be_false + end + +end + +shared_examples_for 'an Ansible provisioner' do | path_prefix, ansible_setup | + + provisioner_label = "ansible #{ansible_setup} provisioner" + provisioner_system = ansible_setup == "local" ? "guest" : "host" + + it "passes if the playbook option refers to an existing file" do + subject.finalize! + + result = subject.validate(machine) + expect(result[provisioner_label]).to eql([]) + end + + it "returns an error if the playbook option is undefined" do + subject.playbook = nil + subject.finalize! + + result = subject.validate(machine) + expect(result[provisioner_label]).to eql([ + I18n.t("vagrant.provisioners.ansible.errors.no_playbook") + ]) + end + + it "passes if the extra_vars option refers to an existing file" do + subject.extra_vars = existing_file + subject.finalize! + + result = subject.validate(machine) + expect(result[provisioner_label]).to eql([]) + end + + it "passes if the extra_vars option is a hash" do + subject.extra_vars = { var1: 1, var2: "foo" } + subject.finalize! + + result = subject.validate(machine) + expect(result[provisioner_label]).to eql([]) + end + + # FIXME: + # The guest-based config should actually NOT return this error, + # but only display a warning (similarly to GH-6764 and 4e451c6) + it "returns an error if the extra_vars option refers to a file that does not exist" do + subject.extra_vars = non_existing_file + subject.finalize! + + result = subject.validate(machine) + expect(result[provisioner_label]).to eql([ + I18n.t("vagrant.provisioners.ansible.errors.extra_vars_invalid", + type: subject.extra_vars.class.to_s, + value: subject.extra_vars.to_s) + ]) + end + + it "returns an error if the extra_vars option is of wrong data type" do + subject.extra_vars = ["var1", 3, "var2", 5] + subject.finalize! + + result = subject.validate(machine) + expect(result[provisioner_label]).to eql([ + I18n.t("vagrant.provisioners.ansible.errors.extra_vars_invalid", + type: subject.extra_vars.class.to_s, + value: subject.extra_vars.to_s) + ]) + end + + it "passes if inventory_path refers to an existing location" do + subject.inventory_path = existing_file + subject.finalize! + + result = subject.validate(machine) + expect(result[provisioner_label]).to eql([]) + end + + it "returns an error if the raw_arguments is of the wrong data type" do + subject.raw_arguments = { arg1: 1, arg2: "foo" } + subject.finalize! + + result = subject.validate(machine) + expect(result[provisioner_label]).to eql([ + I18n.t("vagrant.provisioners.ansible.errors.raw_arguments_invalid", + type: subject.raw_arguments.class.to_s, + value: subject.raw_arguments.to_s) + ]) + end + + it "converts a raw_arguments option defined as a String into an Array" do + subject.raw_arguments = "--foo=bar" + subject.finalize! + + result = subject.validate(machine) + expect(subject.raw_arguments).to eql(%w(--foo=bar)) + end + + describe "sudo option" do + it_behaves_like "any VagrantConfigProvisioner strict boolean attribute", :sudo, false + end + +end