-
Notifications
You must be signed in to change notification settings - Fork 8
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
Improve k8s charm test coverage #1199
Improve k8s charm test coverage #1199
Conversation
- Added tests for create-auth-model action. - Added tests for Postgres relation. - Updated database lib to the latest maintained.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a suggestion; if we add this to pyproject.toml
then we can see the uncovered branches in the final report:
[tool.coverage.run]
branch = true
def mocked_requests_post(*args, **kwargs): | ||
class MockResponse: | ||
def __init__(self, json_data, status_code): | ||
self.json_data = json_data | ||
self.status_code = status_code | ||
self.ok = True | ||
|
||
def json(self): | ||
return self.json_data | ||
|
||
return MockResponse({"authorization_model_id": 123}, 200) | ||
|
||
@mock.patch("src.charm.requests.post") | ||
def test_create_auth_model_action(self, mock_post): | ||
mock_post.side_effect = self.mocked_requests_post | ||
self.harness.enable_hooks() | ||
self.add_openfga_relation() | ||
self.harness.run_action("create-authorization-model", {"model": "null"}) | ||
self.assertEqual(self.harness.charm._state.openfga_auth_model_id, 123) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If mocked_requests_post
is only used in this test (test_create_auth_model_action
), I think it's better to move it's definition to the body of the test (instead of defining it at the class scope).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, can do.
Description
Improve the JIMM k8s charm unit test coverage.
Adds tests for:
create-authorization-model
action.Updated the postgres relation library to the latest one as the previously used one was deprecated. Also removed some unused code.
Partially addresses CSS-8235
Engineering checklist
Check only items that apply