Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new log_metric fixture that allows users to record performance metrics or numerical data during tests in the Prometheus/OpenMetrics text-based format.
Key changes:
- New
--metric-pathCLI option to specify the output file for metrics - New
log_metricfixture that writes metrics in OpenMetrics format with labels - Comprehensive test coverage for both with and without metric path scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pytest-embedded/pytest_embedded/plugin.py | Adds --metric-path CLI option and implements metric_path and log_metric fixtures with file locking for thread-safe writes |
| pytest-embedded/tests/test_base.py | Adds test coverage for log_metric functionality with and without metric path |
| docs/usages/help_functions.rst | New documentation file explaining the usage and format of the log_metric fixture |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| elif os.path.dirname(metric_path): | ||
| os.makedirs(os.path.dirname(metric_path), exist_ok=True) |
There was a problem hiding this comment.
The condition elif os.path.dirname(metric_path) will be False for paths in the current directory (e.g., 'metrics.txt'), which causes the directory creation to be skipped. This logic should handle the case where the file path is relative and has a non-empty directory component. Consider checking if os.path.dirname(metric_path) is not empty and that directory does not already exist, or use a simpler approach like unconditionally creating the directory if it's not empty.
| elif os.path.dirname(metric_path): | |
| os.makedirs(os.path.dirname(metric_path), exist_ok=True) | |
| else: | |
| dir_name = os.path.dirname(metric_path) | |
| if dir_name: | |
| os.makedirs(dir_name, exist_ok=True) |
No description provided.