Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Add description to location module #1724

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

PfurtschellerP
Copy link

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).

Signed-off-by: Patrick Pfurtscheller <patrick@pfurtscheller.org>
@evgeni
Copy link
Member

evgeni commented Apr 3, 2024

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?

Signed-off-by: Patrick Pfurtscheller <patrick@pfurtscheller.org>
@PfurtschellerP
Copy link
Author

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
Copy link
Member

evgeni commented Apr 3, 2024

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

@evgeni
Copy link
Member

evgeni commented Apr 10, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants