Skip to content

Commit

Permalink
test: add type hints to test_framework (#1025)
Browse files Browse the repository at this point in the history
* Change the type of `Handle`'s `key` (in `__init__` and `nest()` and the property) to explicitly allow `None
* Add a CHANGES.md file to document changes (backfilled only to 2.7)
* Tidy up the type hinting of `BoundStoredState.__getattr__` - multiple overloads need to be provided, and once that's done the ignores can be removed (and a bunch of type issues in the tests get resolved)
* Ignore type where creating an object that's only partially complete, but the test only requires that much and fully creating it would be a lot of extra code for no real gain
* Where's it's simple to do, pass the required attributes rather than `None`
* Ignore types where we're checking handling of invalid types
* Add casts around the very dynamic snapshot/restore functionality
* For a couple of the "list of test cases" situations, move the documentation of what each element in the test case is to a new type definition that covers both what was there before and the (rough) expected type
* Sprinkle type hints where required for pyright to be happy

Partially addresses #1007
  • Loading branch information
tonyandrewmeyer authored Oct 3, 2023
1 parent d8827e0 commit 2794449
Show file tree
Hide file tree
Showing 4 changed files with 220 additions and 150 deletions.
9 changes: 9 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# 2.8.0

* The type of a `Handle`'s `key` was expanded from `str` to `str|None`

# 2.7.0

* Added Unit.set_ports()
* Type checks now allow comparing a `JujuVersion` to a `str`
* Renamed `OpenPort` to `Port` (`OpenPort` remains as an alias)
14 changes: 9 additions & 5 deletions ops/framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class Handle:
under the same parent and kind may have the same key.
"""

def __init__(self, parent: Optional[Union['Handle', 'Object']], kind: str, key: str):
def __init__(self, parent: Optional[Union['Handle', 'Object']], kind: str, key: Optional[str]):
if isinstance(parent, Object):
# if it's not an Object, it will be either a Handle (good) or None (no parent)
parent = parent.handle
Expand All @@ -119,7 +119,7 @@ def __init__(self, parent: Optional[Union['Handle', 'Object']], kind: str, key:
else:
self._path = f"{kind}" # don't need f-string, but consistent with above

def nest(self, kind: str, key: str) -> 'Handle':
def nest(self, kind: str, key: Optional[str]) -> 'Handle':
"""Create a new handle as child of the current one."""
return Handle(self, kind, key)

Expand All @@ -143,7 +143,7 @@ def kind(self) -> str:
return self._kind

@property
def key(self) -> str:
def key(self) -> Optional[str]:
"""Return the handle's key."""
return self._key

Expand Down Expand Up @@ -1061,13 +1061,17 @@ def __init__(self, parent: Object, attr_name: str):

if TYPE_CHECKING:
@typing.overload
def __getattr__(self, key: Literal['on']) -> ObjectEvents: # type: ignore
def __getattr__(self, key: Literal['on']) -> ObjectEvents:
pass

@typing.overload
def __getattr__(self, key: str) -> Any:
pass

def __getattr__(self, key: str) -> Any:
# "on" is the only reserved key that can't be used in the data map.
if key == "on":
return self._data.on # type: ignore # casting won't work for some reason
return self._data.on
if key not in self._data:
raise AttributeError(f"attribute '{key}' is not stored")
return _wrap_stored(self._data, self._data[key])
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ include = ["ops/*.py", "ops/_private/*.py",
"test/test_testing.py",
"test/test_storage.py",
"test/test_charm.py",
"test/test_framework.py",
]
pythonVersion = "3.8" # check no python > 3.8 features are used
pythonPlatform = "All"
Expand Down
Loading

0 comments on commit 2794449

Please sign in to comment.