Improve error handling and add comprehensive client tests#2007
Merged
Improve error handling and add comprehensive client tests#2007
Conversation
- Update `get_current_execution` method to return None for non-existent executions. - Add new exceptions: NoSuchDeviceError and NoSuchActionGroupError. - Introduce error handling probes in `test_error_handling.py` for various invalid inputs. - Add test cases for empty responses and specific error scenarios in `test_client.py`. - Create JSON fixtures for various error responses to improve testing coverage.
Contributor
There was a problem hiding this comment.
Pull request overview
Improves robustness of Overkiz client error/edge-case handling (notably executions and missing resources), aligns execution parsing with real API payloads, and adds broad unit-test coverage plus a real-API probing script.
Changes:
- Make
get_current_execution()resilient to empty/invalid “not found” payloads by returningNone. - Extend/align models and error mapping:
Executiongains timing/type fields and usesExecutionState; addNoSuchDeviceError/NoSuchActionGroupError; allow inlineActionGroupobjects without identifiers. - Add many new unit tests + JSON fixtures for cloud/local differences, plus a probe script to validate error handling against real endpoints.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pyoverkiz/client.py |
Returns None for empty/non-dict exec/current/{id} responses to prevent crashes. |
pyoverkiz/models.py |
Aligns Execution/ActionGroup parsing with observed API payloads; adds execution metadata fields and enum state. |
pyoverkiz/response_handler.py |
Maps NO_SUCH_DEVICE / NO_SUCH_ACTION_GROUP to new dedicated exceptions. |
pyoverkiz/exceptions.py |
Introduces NoSuchDeviceError and NoSuchActionGroupError. |
tests/test_client.py |
Adds comprehensive tests for executions, history, places, state parsing, action execution, and local-gateway error variants. |
tests/fixtures/** |
Adds endpoint and exception JSON fixtures used by new tests. |
scripts/test_error_handling.py |
Adds a probe tool to validate error handling behavior against real cloud/local APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
get_current_executioncrashing on empty responses (cloud returns{}, local returns[]ornull)ActionGroup.idandActionGroup.oidoptional (str | None) — inline action groups fromexec/currentdon't carry identifiersstart_time,execution_type, andexecution_sub_typefields toExecutionmodel (matching real API responses)Execution.statefromstrtoExecutionStateenum (consistent withHistoryExecution)NoSuchDeviceErrorandNoSuchActionGroupErrorexception types with response handler mappingsBreaking changes
Execution.stateis nowExecutionStateinstead ofstr,ActionGroup.idandActionGroup.oidare nowstr | Noneinstead ofstr,get_current_executionnow returnsExecution | Noneinstead ofExecutionTest plan