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

Add unit and integration tests for kati-ui charm #143

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DnPlas
Copy link
Contributor

@DnPlas DnPlas commented Sep 21, 2023

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.

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
@i-chvets
Copy link
Contributor

Currently CI is failing due to pypa/setuptools_scm#918

harness.cleanup()


def test_leader(mocker: MockerFixture, harness: Harness[KatibUIOperator]):
Copy link
Contributor

@i-chvets i-chvets Sep 22, 2023

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]:
Copy link
Contributor

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)
Copy link
Contributor

@i-chvets i-chvets Sep 22, 2023

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.

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

Successfully merging this pull request may close these issues.

None yet

3 participants