WIP: Add synced_folder_opts to puppet provisioner #10345

[calbrecht]

Provide the possibility to configure the synced folders
created by the puppet and chef provisioner with options
supported by any of the existing synced folder plugins.

Deprecate synced_folder_type and synced_folder_args.

See #10345

@todo test provisioner/puppet.rb

[briancain]

Thanks for your contribution to Vagrant! So far so good! I know it's a WIP pull request but I left some feedback for you. Let me know if you have any questions about what I wrote. Thanks! 🎉

[calbrecht]

@briancain thanks for your feedback, i appreciate it. :) Nevertheless some points i disagree, but will comment on them separtely. 🎉

[calbrecht]

These calls i really would like to test with some sort of integration test, at least against a mocked root_config.vm.synced_folder method, but did not find any good copy-paste template for it so to understand what i have to do here. Maybe you know of some place i can peak a solution to test this?

[briancain]

You should be able to mock this up with Rspec. Unfortunately the puppet provisioner is very sparse on tests :( You'll want to create a new describe context for this classes rspec tests around the #configure method. Ideally here I would want to mock up that a given double called vm received the method :synced_folder with the specific config options. That will be good enough locally for unit tests.

Something like

describe "#configure" do
  let(:vm) { double("vm") }
  let(:root_config) { double("root_config" } # This should actually be real when writing this test, not just pseudo code
...
  it "configures the provisioner" do
    # other stuff probably goes here!!
    expect(vm).to receive(:synced_folder).with("/path/to/the/manifest", {type=>"nfs", other=>"keys"})

    subject.configure(root_config)
  end
end

This is super basic and missing a lot of what would probably be required 😅 But that is basically how I would test that the right thing is being passed to the #synced_folder function to be stored as a config.

[briancain]

You will also probably need to ensure that the double vm is the right class and not just a double? Since it's apart of root_config, you'll likely need to do a bit of mocking to ensure that it is a VagrantPlugins::Kernel_V2::VMConfig type. But anyway, hopefully that gives you a good idea 👀

[calbrecht]

Yes, exactly :) Thanks very much, because it would have been tedious to search this together without knowing what to do in a new language :) Very much appreciated 👍

[calbrecht]

I know it's a WIP pull request but I left some feedback for you.

No need to apologize, in fact i was hoping to get some feedback soon that's why i created this PR early. Maybe next time i'll have to pull feedback more actively :)

[briancain]

@calbrecht - Hey there! I was just curious if you needed anymore help finishing this up, or if you needed to hand it off. Thanks! :)

[hashicorp-cla]

All committers have signed the CLA.

[calbrecht]

@briancain thank you very much for getting back on me.

To be honest, i got distracted by the fact that, for me, the isolated_environment is not really isolated and is pulling the plugins from my actual installation home folder (which makes running the "integration" tests that are left to implement so tedious) and eventually lost interest in writing the remaining tests after trying and failing to fix the isolation for good.