allowing merge_dict calls to correctly update the main dict

[smbambling]

This fixes a bug where the params for config_options and provider_options were not being merged in the _get_vagrant_config_dict function

Manually tested this with my remote Libvirt setup.

[smbambling]

I did a force push to fix a small typo during the tests...is there a way to kick these off again ?

[ssbarnea]

It will run again, not need to worry but if ever need to force a rebuild, you have two options: add a comment with recheck as body or close and reopen the PR. You may need to wait a little bit after that, but don't worry it will rebuild.

Also, alwats run tox -e lint before any push.

[ssbarnea]

Please include a test that will prevent a future regression.

[smbambling]

Pushed an update to fix the linting with Black.

My pytesting foo is so-so, not sure how to correctly setup the testing so that a scenario is called with test values for config/provider options. I don't know if you were thinking along the lines for a test that just looked to make sure .update( is prefixed before molecule.util.merge_dicts or a deeper test that would look at the molecule cache (~/.cache/moleulce/{role}/{scenario}/vagrant.yml) and verify the output

[apatard]

This is probably a silly question, but how did you notice the problem ? maybe this would help writing a test ?

[ssbarnea]

Not silly at all. In fact that is the best way to add a test, verifying that the expected outcome is met. Without a test I am not going to approve it, mainly because every time I did this I soon regretted it.

[ssbarnea]

I did not approve this yet due to the lack of tests (and other reviews). Anyone is welcomed to step-in and help writing tests.

[apatard]

It turns out that on my quest for writing a test case for #20 not involving virtualbox (I'm trying to add vbox to the test suite but having to build a kernel module is a nightmare), I managed to write tests cases for theses two configuration options.

I'll create a pull request for them, which will fail and hopefully, by triggering again the test suite on this pull request, theses two tests will be fixed.

[apatard]

I've merged this tree with my tests for this commit in my branch https://github.com/apatard/molecule-vagrant/tree/mergedicts. I think it'll be easier to handle this way. I'll send a pull request once my vagrant-root branch is merged.

[apatard]

It's fixing real bugs and I've tested it locally. So, it's fine for me.