Bugfix/unittests #451
Bugfix/unittests #451
Conversation
Mark-Powers
commented
Sep 23, 2024
•
Mark-Powers
commented
- Removes outdated unit tests (no longer part of portal)
- Adds balance service tests for API functions and helps
- Fixes migrations that could not be ran (required external values)
- Uses sqlite3 for testing DB for ease of running tests
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -70,6 +70,10 @@ def _str_to_localtime(utctimestr): | |||
|
|
|||
|
|
|||
| def init_charge_from_openstack_db(apps, schema_editor): | |||
| # No initial charges in test DB | |||
There was a problem hiding this comment.
Is this just to skip migrations when running in CI?
Do we know why this needs to access an external resource to run a migration anyway?
There was a problem hiding this comment.
settings.TESTING is added in this PR, and is true when the code is run with ./manage.py test.
This code initially loaded existing reservations at openstack sites into the charges database table.
3a6d518
There was a problem hiding this comment.
Absolutely better than before, and I'm really happy to see where we explicitly need to mock calls to keystone and keycloak in order for it to work.
comments so far are mostly pointing at potential tech debt, but this seems good to me.
Are there any particularly annoying test cases where we should add comments describing the intended behavior?
|
|
||
| if not include_update: | ||
| return { | ||
| "context": { | ||
| "user_id": "c631173e-dec0-4bb7-a0c3-f7711153c06c", | ||
| "project_id": "a0b86a98-b0d3-43cb-948e-00689182efd4", | ||
| "auth_url": "https://api.example.com:5000/v3", | ||
| "region_name": "RegionOne", | ||
| }, | ||
| "lease": { | ||
| "start_date": lease_start, | ||
| "end_date": lease_end, | ||
| "project_id": "a0b86a98-b0d3-43cb-948e-00689182efd4", | ||
| "user_id": "c631173e-dec0-4bb7-a0c3-f7711153c06c", | ||
| "name": "my_lease", | ||
| "reservations": [ | ||
| { | ||
| "resource_type": "physical:host", | ||
| "min": allocations_per_res, | ||
| "max": allocations_per_res, | ||
| "hypervisor_properties": "[]", | ||
| "resource_properties": '["==", "$availability_zone", "az1"]', | ||
| "id": str(j), | ||
| "allocations": [ | ||
| { | ||
| "id": str(i), | ||
| "hypervisor_hostname": "32af5a7a-e7a3-4883-a643-828e3f63bf54", | ||
| "extra": {"availability_zone": "az1"}, | ||
| "su_factor": su_factor, | ||
| } | ||
| for i in range(allocations_per_res) | ||
| ], | ||
| } | ||
| for j in range(reservations) | ||
| ], | ||
| }, | ||
| } | ||
| else: | ||
| return { | ||
| "context": { | ||
| "user_id": "c631173e-dec0-4bb7-a0c3-f7711153c06c", | ||
| "project_id": "a0b86a98-b0d3-43cb-948e-00689182efd4", | ||
| "auth_url": "https://api.example.com:5000/v3", | ||
| "region_name": "RegionOne", | ||
| }, | ||
| "current_lease": { | ||
| "start_date": lease_start, | ||
| "end_date": lease_end, | ||
| "project_id": "a0b86a98-b0d3-43cb-948e-00689182efd4", | ||
| "user_id": "c631173e-dec0-4bb7-a0c3-f7711153c06c", | ||
| "name": "my_lease", | ||
| "reservations": [ | ||
| { | ||
| "resource_type": "physical:host", | ||
| "min": allocations_per_res, | ||
| "max": allocations_per_res, | ||
| "hypervisor_properties": "[]", | ||
| "resource_properties": '["==", "$availability_zone", "az1"]', | ||
| "id": str(j), | ||
| "allocations": [ | ||
| { | ||
| "id": str(i), | ||
| "hypervisor_hostname": "32af5a7a-e7a3-4883-a643-828e3f63bf54", | ||
| "extra": {"availability_zone": "az1"}, | ||
| "su_factor": su_factor, | ||
| } | ||
| for i in range(allocations_per_res) | ||
| ], | ||
| } | ||
| for j in range(reservations) | ||
| ], | ||
| }, | ||
| "lease": { | ||
| "start_date": update_start, | ||
| "end_date": update_end, | ||
| "project_id": "a0b86a98-b0d3-43cb-948e-00689182efd4", | ||
| "user_id": "c631173e-dec0-4bb7-a0c3-f7711153c06c", | ||
| "name": "my_lease", | ||
| "reservations": [ | ||
| { | ||
| "resource_type": "physical:host", | ||
| "min": update_allocations_per_res, | ||
| "max": update_allocations_per_res, | ||
| "hypervisor_properties": "[]", | ||
| "resource_properties": '["==", "$availability_zone", "az1"]', | ||
| "id": str(j), | ||
| "allocations": [ | ||
| { | ||
| "id": str(i), | ||
| "hypervisor_hostname": "32af5a7a-e7a3-4883-a643-828e3f63bf54", | ||
| "extra": {"availability_zone": "az1"}, | ||
| "su_factor": update_su_factor, | ||
| } | ||
| for i in range(update_allocations_per_res) | ||
| ], | ||
| } | ||
| for j in range(update_reservations) | ||
| ], | ||
| }, | ||
| } |
There was a problem hiding this comment.
to comment, at a glance it's not clear to me which values are actually important, and which are just placeholders. Is this basically generating test data to help parametrize other tests?
There was a problem hiding this comment.
This is generating blazar API data to test the methods that our API view calls. https://docs.openstack.org/blazar/2024.1/admin/usage-enforcement.html
The most relevant parts are # of reservations, # of allocations in each reservation, and su factor, along with the lease duration.
|
Also calling out this functional change: portal/balance_service/enforcement/usage_enforcement.py Lines 206 to 215 in d98dcec Before, it always used a temp ID, as the blazar enforcement API does not give reservation IDs on create. Now, this first looks for a real ID, and falls back on a temp ID. A celery task renames the temp IDs. I'm working to add a real ID to this API call in blazar. The temp IDs are not reliable, and cause errors when a user extends their lease. |
