use the api to create a set of reusable fixtures #43
Conversation
evgeni
force-pushed
the
apifixtures
branch
8 times, most recently
from
bae103c to
510ad5b
Compare
| def yum_repository(product, organization, foremanapi): | ||
| repo = foremanapi.create('repositories', {'name': str(uuid.uuid4()), 'product_id': product['id'], 'content_type': 'yum', 'url': 'https://fixtures.pulpproject.org/rpm-no-comps/'}) |
There was a problem hiding this comment.
I don't know how far you want to take this, but I think you can even make the URL a parameter so it can be overridden
| def yum_repository(product, organization, foremanapi): | |
| repo = foremanapi.create('repositories', {'name': str(uuid.uuid4()), 'product_id': product['id'], 'content_type': 'yum', 'url': 'https://fixtures.pulpproject.org/rpm-no-comps/'}) | |
| def yum_repository(product, organization, foremanapi, url='https://fixtures.pulpproject.org/rpm-no-comps/'): | |
| repo = foremanapi.create('repositories', {'name': str(uuid.uuid4()), 'product_id': product['id'], 'content_type': 'yum', 'url': url}) |
There was a problem hiding this comment.
I don't think fixtures take regular parameters like this (there are fixture factories, but those are more complicated)
There was a problem hiding this comment.
https://docs.pytest.org/en/7.1.x/example/parametrize.html#indirect-parametrization says you can do it, but you need to use request.param. Feel free to leave it out for now, but let's keep it in mind for the future.
There was a problem hiding this comment.
Oh via indirect… Mhh, interesting. Maybe. I'll think about it.
There was a problem hiding this comment.
#60 is to track this, merging as is for now
| def lifecycle_environment(organization, foremanapi): | ||
| library = foremanapi.list('lifecycle_environments', 'name=Library', {'organization_id': organization['id']})[0] |
There was a problem hiding this comment.
Same here perhaps?
| def lifecycle_environment(organization, foremanapi): | |
| library = foremanapi.list('lifecycle_environments', 'name=Library', {'organization_id': organization['id']})[0] | |
| def lifecycle_environment(organization, foremanapi, prior='Library'): | |
| library = foremanapi.list('lifecycle_environments', f'name={prior}', {'organization_id': organization['id']})[0] |
Though perhaps it should just accept an instance where None is automatically resolve to the library:
| def lifecycle_environment(organization, foremanapi): | |
| library = foremanapi.list('lifecycle_environments', 'name=Library', {'organization_id': organization['id']})[0] | |
| def lifecycle_environment(organization, foremanapi, prior=None): | |
| if not prior: | |
| prior = foremanapi.list('lifecycle_environments', 'name=Library', {'organization_id': organization['id']})[0] |
Which also makes me question what the Katello API does if no prior ID is given. It's not listed as required and mandating this, but would it make sense to find the library server side and make it an optional parameter?
There was a problem hiding this comment.
It does not find "Library" automatically and errors out:
https://github.com/Katello/katello/blob/db89c59031e34994805faaf3c6314b0e412b56c3/app/controllers/katello/api/v2/environments_controller.rb#L39
https://github.com/Katello/katello/blob/db89c59031e34994805faaf3c6314b0e412b56c3/app/controllers/katello/api/v2/environments_controller.rb#L172-L177
| ) | ||
|
|
||
| @pytest.fixture | ||
| def organization(foremanapi): |
There was a problem hiding this comment.
Right now all these tests have an (implicit) scope=function (https://docs.pytest.org/en/stable/how-to/fixtures.html#fixture-scopes) which means that they are destroyed at the end of every test.
Conceptually, this is good, of course. But on the flip side this means that every repository test also first creates a new org (also in Candlepin), a new product, etc. Those times quickly add up, and make the test tiresome to execute.
We can speed this up with scope=module, but this means that any failure in a test might have cascading effects on later tests (which is the same behaviour as we have today with bats, FWIW).
WDYT?
There was a problem hiding this comment.
For now I'd lean to the isolation given we still have very few tests, but I agree in the future it will add up. Later we can add a "persistent" fixture that can be reused.
Given we have isolation, it's probably also easy to run things in parallel. Those failures may be harder to diagnose, but it can also uncover real work concurrency issues which large deployments will see at some point.
| vagrant up quadlet | ||
| vagrant up client |
There was a problem hiding this comment.
Do you intentionally start them up sequentially instead of parallel? Is that the issue with fetching the same box concurrently giving issues?
There was a problem hiding this comment.
I honestly haven't tried 😅
evgeni
force-pushed
the
apifixtures
branch
4 times, most recently
from
31c26f3 to
d926760
Compare
|
Generally looks good.. what's needed to get it out of draft? |
|
| hosts: all | ||
| become: true | ||
| roles: | ||
| - theforeman.forklift.etc_hosts |
No description provided.