theforeman / foreman-ansible-modules

Ansible modules for interacting with the Foreman API and various plugin APIs such as Katello
GNU General Public License v3.0
149 stars 166 forks source link

Feature: Add description to location module #1724

Closed PfurtschellerP closed 4 months ago

PfurtschellerP commented 7 months ago

Currently - even though the REST API and python utils would allow it - the location module does not implement the parameter description. However, this can be helpful especially if a company uses e.g. computed site codes as locations in Foreman or Satellite that are not easily understandable at first glance (but help with automation).

evgeni commented 7 months ago

On a first glance, your changes look good (and you have a picky editor 😉). You've added tests, but did not re-record the fixtures, which is why things are now failing in CI. Do you need any help with that step?

PfurtschellerP commented 7 months ago

On a first glance, your changes look good (and you have a picky editor 😉). You've added tests, but did not re-record the fixtures, which is why things are now failing in CI. Do you need any help with that step?

Hi, yes I would definetly need help with that. I tried to replicate it like it was done for the other tests, but I'm not familiar with creating fixtures.

evgeni commented 7 months ago

I'll try to ping you with some pointers next week when back in the office.

evgeni commented 7 months ago

Okay, here I am. Let's see if we can get this going for you.

There is some basic documentation at https://github.com/theforeman/foreman-ansible-modules/blob/develop/docs/testing.md#writing-tests but I'll try to re-phrase it (and you tell me which version was better understandable ;) )

You'll need:

Once you have that, you should be ready to go. You can verify the overall setup by calling an existing test like make test_bookmark, this won't touch your installation yet, but run the same tests that we run on GitHub here.

As a counter-test, you can run make test_location and that will fail, as the playbooks do not (yet) match the recorded fixtures.

You can now run make record_location which will delete all existing fixtures and re-record them. As you've added new tests, you'll see new files in tests/test_playbooks/fixtures/location-*.yml (and the old ones change a bit, as you re-record everything). If make record_location passes, you can add the new (and changed) files to git, and commit them. Once pushed here I'd expect the CI to turn green. If it doesn't pass, it might be a bug in the changes you made, or something else. Ideally just paste the whole output here and we'll look over it together.