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,4 +1,5 @@
 require_relative "base"
+require_relative "../helpers"
 
 module VagrantPlugins
   module Ansible

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

test/unit/plugins/provisioners/ansible/config/host_test.rb

@@ -1,26 +1,19 @@
-require_relative "../../../base"
+require_relative "../../../../base"
-require_relative "../support/shared/config"
+require_relative "../../support/shared/config"
-
+require_relative "shared"
-require "vagrant/util/platform"
 
 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
+  let(:non_existing_file) { "/this/does/not/exist" }
-    next "/this/does/not/exist" if !Vagrant::Util::Platform.windows?
-    "C:/foo/nope/nope"
-  end
 
   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
+  describe "default options handling" do
-    subject.finalize!
+    it_behaves_like "options shared by both Ansible provisioners"
 
-    expect(subject.playbook).to be_nil
+    it "assigns default values to unset host-specific options" do
-    expect(subject.extra_vars).to be_nil
+      subject.finalize!
-    expect(subject.force_remote_user).to be_true
+
-    expect(subject.ask_sudo_pass).to be_false
+      expect(subject.ask_sudo_pass).to be_false
-    expect(subject.ask_vault_pass).to be_false
+      expect(subject.ask_vault_pass).to be_false
-    expect(subject.vault_password_file).to be_nil
+      expect(subject.force_remote_user).to be_true
-    expect(subject.limit).to be_nil
+      expect(subject.host_key_checking).to be_false
-    expect(subject.sudo).to be_false
+      expect(subject.raw_ssh_args).to be_nil
-    expect(subject.sudo_user).to be_nil
+    end
-    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
   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
+    it_behaves_like "an Ansible provisioner", "", "remote"
-      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 "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
+    it "returns an error if galaxy_role_file is specified, but does not exist" do
-      subject.extra_vars = existing_file
+      subject.galaxy_role_file = non_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
       subject.finalize!
 
       result = subject.validate(machine)
       expect(result["ansible remote provisioner"]).to eql([
-        I18n.t("vagrant.provisioners.ansible.errors.extra_vars_invalid",
+        I18n.t("vagrant.provisioners.ansible.errors.galaxy_role_file_invalid",
-               type:  subject.extra_vars.class.to_s,
+               path: non_existing_file, system: "host")
-               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["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!

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