-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add unit and integration tests for kati-ui charm #143
base: main
Are you sure you want to change the base?
Conversation
Unit tests cover basic pebble and leader implementation. Integration test checks if charm builds and deploys and that the katib-ui server is coming up. Relates to #5
6bfe6fc
to
31187b3
Compare
Currently CI is failing due to pypa/setuptools_scm#918 |
harness.cleanup() | ||
|
||
|
||
def test_leader(mocker: MockerFixture, harness: Harness[KatibUIOperator]): |
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.
I would add docstring to each test describing what is being tested.
|
||
|
||
@pytest_asyncio.fixture | ||
async def ws_addresses(ops_test: OpsTest) -> List[str]: |
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.
I would add doc string that described the purpose of this helper function.
"""Verify that all katib-ui units report ready.""" | ||
for addr in ws_addresses: | ||
url = f"http://{addr}:8080/katib" | ||
home = requests.get(url) |
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.
Should we wrap this code in try-except
? This way, we can assert on any failure in fetching URL.
Unit tests cover basic pebble and leader implementation. Integration test checks if charm builds and deploys and that the katib-ui server is coming up.
Relates to #5
This is a clone of #141 to be able to see the CI running.