tests: conftest: Add type annotations. #1096
Merged
+160
−129
Conversation
This file contains 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
prah23
force-pushed
the
test_annotation_conftest
branch
2 times, most recently
from
5e24e15 to
305e864
Compare
prah23
force-pushed
the
test_annotation_conftest
branch
2 times, most recently
from
1cb93f7 to
d3acd1e
Compare
neiljp
reviewed
Aug 6, 2021
tests/conftest.py
Outdated
| @@ -231,7 +236,7 @@ def logged_on_user(): | |||
|
|
|||
|
|
|||
| @pytest.fixture | |||
| def streams_fixture(): | |||
| def streams_fixture() -> List[Dict[str, Any]]: | |||
There was a problem hiding this comment.
This should match a type, but again we may have entries missing and be ZFL-dependent :/
There was a problem hiding this comment.
Though there is the ZFL issue, I do think an API type for streams would be appropriate, since a "Stream" is one of our major types.
| @@ -289,7 +294,7 @@ def realm_emojis(): | |||
|
|
|||
|
|
|||
| @pytest.fixture | |||
| def realm_emojis_data(): | |||
| def realm_emojis_data() -> "OrderedDict[str, Dict[str, Any]]": | |||
There was a problem hiding this comment.
We may want to check with Zeeshan, but a specific (internal) type might be good to define for this, given how much we're using it :)
There was a problem hiding this comment.
Agreed, there's also an instance of this in test_emoji_data.py.
neiljp
added
PR awaiting update
3 tasks
prah23
force-pushed
the
test_annotation_conftest
branch
4 times, most recently
from
6d4b03c to
a00d17b
Compare
prah23
added
PR needs review
This commit changes the returned object of the `msg_template_factory` from a `Dict` to a `Message` instance, which is the appropriate type for a message template. `stream_id` is also included in the `Message` instance only when it is a stream message. Places where the factory function is used are also typed accordingly.
This commit returns an `Index` instance for the `empty_index` fixture which previously returned a deepcopy of a Dict, as Index is the `empty_index`'s appropriate type. Other fixtures using this fixture are updated to this type at this point too.
This commit returns `Index` instances instead of `dict`s from all the various index fixtures present, being the more appropriate type. The return type for these fixtures is added accordingly.
This commit fixes the `else` case of the `_widget_size` method of the `widget_size` fixture method. Previously, the else case did not return anything, which has been modified to appropriately return an empty tuple now.
This commit modifies the `bot_type` attribute of the RealmUser class to an `Optional[int]` from the previous `int` data type. This change is motivated by the server not assigning an integer to the user when the user isn't a bot, making None the ideal value in that case. To maintain consistency the `bot_type` attribute of the TidiedUserInfo class becomes an `Optional[int]` from the previous `int` data type.
This commit modifies the default `bot_type` value from the previous value of 0 to None, since a user who isn't a bot has a null value for this attribute.
This commit adds parameter and return type annotations or hints to the `conftest.py` file, that contains custom pytest fixtures meant to be used in the tests to make mypy checks consistent and improve code readability.
This commit adds `conftest.py` to the `type_consistent_testfiles` list to check for type consistency with mypy.
neiljp
force-pushed
the
test_annotation_conftest
branch
from
a00d17b to
f9e7a8e
Compare
|
@prah23 Thanks for getting this done - this is quite large and will also be a good guide to typing other tests 🎉 |
neiljp
added
area: refactoring
area: tests
and removed
PR needs review
|
Hello @zulip/server-refactoring members, this pull request was labeled with the "area: refactoring" label, so you may want to check it out! |
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.
What does this PR do?
Adds type annotations to
conftest.pyand some relevant prior refactors. Also includesconftest.pyunder the list inrun-mypy.Tested?
Commit flow
refactor: tests: conftest: Return a Message from template factory.
This commit changes the returned object of the
msg_template_factoryfrom a
Dictto aMessageinstance, which is the appropriate typefor a message template.
refactor: tests: conftest: Return Index instance from empty_index.
This commit returns an Index instance for the
empty_indexfixturewhich previously returned a deepcopy of a Dict, as Index is the
empty_index's appropriate type.refactor: test: conftest: Return Index instances for index_* fixtures.
This commit returns Index instances instead of
dicts from all thevarious index fixtures present, since the appropriate type for these
fixtures is Index.
api_types/helper/model/server_url: Replace stream_id type in Message.
This commit replaces the Message class' stream_id attribute's type
from int to Optional[int], since this attribute only has an integer
value when the message is a stream message and None when it is a
private message.
bugfix: tests: conftest: Return empty tuple when widget isn't box/flow.
This commit fixes the
elsecase of the_widget_sizemethod ofthe
widget_sizefixture method. Previously, the `else case did notreturn anything, which has been modified to appropriately return an
empty tuple now.
refactor: tests: conftest: Add type annotations.
This commit adds parameter and return type annotations or hints to
the
conftest.pyfile, that contains custom pytest fixtures meant tobe used in the tests to make mypy checks consistent and improve code
readability.
tools: Include conftest.py to be checked by mypy.
This commit adds
conftest.pyto thetype_consistent_testfileslist to check for type consistency with mypy.
Notes & Questions
The
requestpytest fixture has no exported type, so I've typed it asAnyfor now. The only way to type it currently would be a private import otherwise.