test: add type hints to test_framework

[tonyandrewmeyer]

• 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

[benhoyt]

Looks good, thanks. A couple of questions and a few small changes requested.

[benhoyt]

Unless I'm missing something, I'm a bit nervous that we're changing / removing these test cases. If anything I think we should keep the existing ones (changing the typing shouldn't change them) and add a couple of new ones testing the additional cases.

[tonyandrewmeyer]

The Handle.__init__ code is like this:

    def __init__(self, parent: Optional[Union['Handle', 'Object']], kind: str, key: str):
        ...
        self._key = key
        ...
            if key:
                self._path = f"{parent}/{kind}[{key}]"
            else:
                self._path = f"{parent}/{kind}"

The docs say (my emphasis):

The handle key is a string uniquely identifying the object.

The key is also used for hashing and equality checks (None is fine in either of those situations, but so are other non-string things). It's also a public member, and I don't have the experience to know how that's generally used, however, it does claim to be a string as well:

    @property
    def key(self) -> str:

This seems messy to me. Shouldn't we either have the type signature and documentation match the behaviour and accept str|None (the docs could say that this should not be done), or have the implementation match the type signature and documentation and reject None as an invalid value for key (maybe this is tricky with backwards compatibility?)?

[benhoyt]

Hmm, yeah. From looking at the code, I think key should have been an Optional[str] all along -- the fact that is was typed as str was I think an oversight. The if key would handle empty string as well, but I think None would be the intention.

I've found one charm that passes None as the key argument to Handle.

So I think we should change Handle's key arg to Optional[str], and same for nest's key argument. Those should be perfectly backwards-compatible changes, as we're expanding the type.

I also think we should change the key property's type to be Optional[str]. That isn't quite a backward-compatible change (typing-wise), because code that accesses the property will then have to assert key is not None to type check. But I've done a quick search and I can't see any charms using that property (it's a bit advanced/arcane to use Handle directly in the first place, let alone access handle.key).

So I think it's safe to change and we should do it. If you want, we could start a file CHANGES.txt that we use to record notes and breaking changes for the upcoming release, so we remember.

[tonyandrewmeyer]

Done - lmk if you'd prefer a particular format for the changes file.

[benhoyt]

All looks good, thanks!