docs: improve unit tests of httpbin demo charm #2254
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the unit tests for the httpbin demo charm by making them more comprehensive and maintainable. The changes include refactoring tests to use pytest's parametrization feature, adding new test cases for edge conditions, and reorganizing the code for better clarity.
- Introduced pytest parametrization to reduce test duplication and improve coverage of log level validation
- Added new test cases for service status conditions (inactive service, missing service, unavailable container)
- Refactored charm code to use else block for clearer control flow in service status checking
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| examples/httpbin-demo/tests/unit/test_charm.py | Refactored existing tests to use parametrization, added new edge case tests for service status, and improved test naming and documentation |
| examples/httpbin-demo/src/charm.py | Refactored service status checking logic to use else block instead of early return pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| if not self.container.get_service(SERVICE_NAME).is_running(): | ||
| # We can connect to Pebble in the container, but the service hasn't started yet. | ||
| event.add_status(ops.MaintenanceStatus("waiting for workload")) | ||
| service = self.container.get_service(SERVICE_NAME) | ||
| except ops.ModelError: | ||
| # We can connect to Pebble in the container, but the service doesn't exist. This is | ||
| # most likely because we haven't added a layer yet. | ||
| event.add_status(ops.MaintenanceStatus("waiting for workload container")) | ||
| except ops.pebble.ConnectionError: | ||
| # We can't connect to Pebble in the container. This is most likely because the | ||
| # container hasn't started yet. | ||
| event.add_status(ops.MaintenanceStatus("waiting for workload container")) | ||
| except ops.pebble.APIError: | ||
| # It's technically possible (but unlikely) for Pebble to have an internal error. | ||
| logger.error("Unable to fetch service info from Pebble") | ||
| raise | ||
| else: | ||
| if not service.is_running(): | ||
| # We can connect to Pebble in the container, but the service hasn't started yet. | ||
| event.add_status(ops.MaintenanceStatus("waiting for workload")) |
There was a problem hiding this comment.
I refactored this part of the charm code after James's comment on similar code elsewhere: #2062 (comment)
1 participant
This PR makes several improvements to the unit tests of our httpbin demo charm:
log-levelI also made a couple of small changes elsewhere: