Adapt functional testing to recent lint updates

[tehsmyers]

Starting in ansible-lint 4.0, the role metadata is now being linted
to ensure the metadata file has been updated by the role author prior
to submission to ansible galaxy. This is a "good thing", but
unfortunately breaks molecule CI.

This solution attempts to retain the behavior of reporting these lint
errors to users, but adds behavior to functional tests to independently
lint and fix this file prior to testing it with molecule.

I have also updated references to the cookiecutter template to use the
new location in the github 'ansible' namespace.

re #1747
I think the lint errors coming out of both the embedded cookiecutter templates used by molecule init and the molecule-cookiecutter repo used in testing, are correct, and should be preserved. Leaving these in place will prevent users from forgetting to update meta/main.yml in a role when developing a role created using molecule init. This change attempts to ensure that those lint errors continue to be raised on a default role, and then patches them out to allow CI to proceed.

PR Type

• Bugfix Pull Request

Testing info

Limited functional testing was done using ansible 2.7 on py2.7 and py3.7:

$ tox -e py27-ansible27-functional,py37-ansible27-functional -- -k docker
...
  py27-ansible27-functional: commands succeeded
  py37-ansible27-functional: commands succeeded
  congratulations :)

Linting and formatting were also checked:

$ tox -e format-check -e lint
...
  format-check: commands succeeded
  lint: commands succeeded
  congratulations :)

No unit tests were harmed in the making of this PR:

$ tox -e py27-ansible27-unit,py37-ansible27-unit
...
  py27-ansible27-unit: commands succeeded
  py37-ansible27-unit: commands succeeded
  congratulations :)

[tehsmyers]

Test failure is the same lint errors we've been seeing, but only on the py27-ansible25-functional target, and only for one of the tests that would trigger it (test_command_init_role_inspec). I'm unable to reproduce this locally:

$ tox -e py27-ansible25-functional -- -k 'test_command_init_role_inspec'
...
  py27-ansible25-functional: commands succeeded
  congratulations :)

[decentral1se]

Awesome @seandst! Thanks for taking this on 🚀 🚀 🚀

So, I took a review pass on it and I did enjoy that well documented code! The further I went, the more my spider sense started to tingle. We may be going down a path that we might not want to in this change. I think there are quite a few parts of this change that are brittle and tied to too many things that can change outside of the control of Molecule.

So, what about this as an alternative strategy: we create a custom ansible-lint configuration file for the testing and ignore only these warnings (and if we can be more specific, for these files? Not sure)? This would allow us to skip things we know are going wrong because we leave it to the user to resolve and use the standard ansible-lint configuration API we know and love. It would avoid any tight coupling which could potentially bring us maintenance headaches.

WDYT?

[gundalow]

Changing my review to -1 till we address the comments

[tehsmyers]

create a custom ansible-lint configuration file for the testing and ignore only these warnings (and if we can be more specific, for these files? Not sure)? This would allow us to skip things we know are going wrong because we leave it to the user to resolve and use the standard ansible-lint configuration API we know and love. It would avoid any tight coupling which could potentially bring us maintenance headaches.

The main goal of centralizing all of this stuff is to get an early warning system in place, so that the failures happen in a way that's more obvious (and more easily fixed) in a failing CI test, and I think that using a custom ansible-lint configuration would still satisfy that goal and, as you suggest, reduce the brittleness.

I mocked this up real quick, continuing to include the "paranoid lint" at the end. Here's a diff of what that might look like:
https://gist.github.com/seandst/52cc61a3f4350ddf52aa9151ff060f5d

Targeted functional testing seems to like it, but I wanted to hold off on amending the commit if this isn't the direction you were expecting here.

An aside: most of the time spent putting this together was just figuring out how to get the functional tests working well enough to actually prove that the fixes fix stuff. Now that I'm over that hurdle, this little change was pretty quick.

[ssbarnea]

I like @lwm idea of having a well controlled testing environment for linting where we pin ansible-lint version for the purpose of testing compatibility of molecule with it.

At the same time molecule itself should not point to a pinned version of ansible-lint in its own requirements because its users should be allowed to adapt/evolve their linting at their own speed.

ansible-lint will have more and more often releases and it will break things for sure, that is a guarantee and for this reason users are manually bumping it. Shortly, please decouple them.

[decentral1se]

I mocked this up real quick, continuing to include the "paranoid lint" at the end. Here's a diff of what that might look like: https://gist.github.com/seandst/52cc61a3f4350ddf52aa9151ff060f5d

Nice! One point on this approach:

Could we just create a test/functional/.ansible-lint with the config we want (skip_list ...) and then we would not have to generate anything? We could create a fixture in test/functional/conftest.py which spits out the path to the configuration file (using plain 'ol join(abspath(__file__), '.ansible-lint') or something) and then you can pass that to the sh.ansible_lint.bake invocation?

LGTM otherwise!

An aside: most of the time spent putting this together was just figuring out how to get the functional tests working well enough to actually prove that the fixes fix stuff. Now that I'm over that hurdle, this little change was pretty quick.

This is good to hear 💯

[tehsmyers]

Updated, but I forgot to format-check before I pushed so travis is gonna be sad. Will push again after I fix that. :)

Edit: Fixed.

[decentral1se]

Great! This should do the job 💯