-
Notifications
You must be signed in to change notification settings - Fork 138
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
type annotate the tests (using monkeytype and manual fixes) #301
type annotate the tests (using monkeytype and manual fixes) #301
Conversation
8a2cb90
to
a7b5518
Compare
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.
this will need a history with the user facing type changes
Do we really need that many annotation changes in the library itself? Did the typed tests reveal that many issues? |
Technically the backends didn't need changes to _SignalReciever because the type information gets destroyed by get_asynclib. Also fileio only needed But other than that everything came from mypy errors |
@@ -483,9 +527,9 @@ async def test_extra_attributes(self, server_sock, socket_path): | |||
pytest.raises(TypedAttributeLookupError, stream.extra, SocketAttribute.local_port) | |||
pytest.raises(TypedAttributeLookupError, stream.extra, SocketAttribute.remote_port) | |||
|
|||
@pytest.mark.parametrize('as_path', [False, True], ids=['str', 'path']) | |||
async def test_send_receive(self, server_sock, socket_path, as_path): |
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.
this as_path was unused
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.
Then perhaps we should add this?
if not as_path:
socket_path = str(socket_path)
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.
I fixed it at runtime by making a Union[str, Path] fixture
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.
Because you can't reassign a Path to a str, in refactoring it to a fixture I noticed the bug in the test
585d6fc
to
f6cce95
Compare
db07b35
to
4de0399
Compare
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.
Comments, questions and a few change requests.
Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>
Thanks! |
Fixed the type annotations ofAsyncFile.__aiter__
,readline
,write
to also accept/returnstr
and also fixed
AsyncFile.writelines
to take anIterable[str|bytes]
rather thanbytes
.DeprecatedAwaitable(|List|Float).__await__
to match thetyping.Awaitable
protocol