-
Notifications
You must be signed in to change notification settings - Fork 153
Add tests for cli commands: monitor token-get, alerts help, alerts definition-view #832
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 tests for cli commands: monitor token-get, alerts help, alerts definition-view #832
Conversation
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.
Pull Request Overview
This PR adds integration tests for CLI monitoring and alert commands to improve test coverage. The changes focus on testing token retrieval, alert help output, and alert definition viewing.
- Renames variables from
dashboard_idtoservice_typefor clarity - Adds test for token-get command error handling
- Adds tests for alerts help command and definition-view command
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/integration/monitor/test_metrics.py | Renames variables for clarity and adds test for token-get command with invalid entity IDs |
| tests/integration/monitor/test_alerts.py | Adds help command test and definition-view test (currently skipped) |
| tests/integration/helpers.py | Adds helper function to validate help command action lists |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
| @pytest.mark.skip | ||
| def test_list_alert_definitions_for_service_type(get_service_type): |
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.
It is under investigation: now command does not work - https://techdocs.akamai.com/linode-api/reference/get-alert-definitions-for-service-type
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.
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.
Okay, techdocs PR was merged and I can remove skip notation for this test. It should work
zliang-akamai
left a comment
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.
Looks good and works well!
vshanthe
left a comment
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
📝 Description
Adds extra integration tests for Metrics & Alerts
✔️ How to Test
make test-int TEST_CASE=test_metrics
make test-int TEST_CASE=test_alerts