tests: core: Add type annotations. #1077
Merged
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
neiljp
force-pushed
the
test_annotation_core
branch
from
1b01b3f to
cf5a1cb
Compare
neiljp
reviewed
Jul 12, 2021
There was a problem hiding this comment.
@prah23 I've rearranged these commits slightly to be clearer:
- fixes to the tests
- combined applying mocker.patch (prep for mypy)
- combined index-messages add+use
- type annotation adding (including original first commit, as it was another type annotation correction that we weren't checking)
There is at least one case that I would expect to be caught, so I'm looking into configuration for that to catch it automatically.
prah23
commented
Jul 12, 2021
neiljp
added
the
PR awaiting update
prah23
force-pushed
the
test_annotation_core
branch
from
cf5a1cb to
80e9129
Compare
|
I've removed the need for |
prah23
added
PR needs review
prah23
force-pushed
the
test_annotation_core
branch
from
80e9129 to
d25e2ef
Compare
This commit replaces the previous assignment of a `str` value to the theme attribute of the `controller` pytest fixture with a ThemeSpec instance generated by the `generate_theme` helper method.
This commits changes the previous incorrect assignment of an empty list to model's muted_streams to an empty set which is the appropriate data structure.
This commit refactors mocking for model.is_muted_topic and controller.report_* methods from direct mocker.Mock assignment to using mocker.patch instead to simplify mypy compliance. (mypy prevents assigning mocker.Mock() to an attribute method)
The `index_search_messages` fixture returns an initial index with the "search" key of the index set with a `set` containing a single `message_id`, 500. The fixture is then used to specify the model's index in `test_search_message()`, as opposed to the previous dict consisting of only the "search" key.
This commit avoids using the `user_button` fixture in `test_narrow_to_user` and uses the parameters `user_email` and `user_id` instead, because the `narrow_to_user` method requires only the list of recipient emails, and the button is not relevant to the abstracted method's test.
This commit avoids using the `stream_button` fixture in `test_narrow_to_stream` and uses the parameters `button_stream_id` and `button_stream_name` instead, because the `narrow_to_stream` method requires only the stream name, and the button is not relevant to the abstracted method's test.
This commit avoids using the `msg_box` fixture in `test_narrow_to_topic` and uses the parameters `msg_box_stream_id`, `msg_box_stream_name` and `msg_box_topic_name` instead, because the `narrow_to_topic` method requires only the stream name and topic name and the MessageBox instance is not relevant to the abstracted method's test.
This commit adds parameter and return type annotations or hints to the `test_core.py` file, that contains tests for its counterpart `core.py` from the `zulipterminal` module, to make mypy checks consistent and improve code readability.
This commit adds `test_core.py` to the `type_consistent_testfiles` list to check for type consistency with mypy.
neiljp
force-pushed
the
test_annotation_core
branch
from
d25e2ef to
bd9a00d
Compare
|
@prah23 Thanks for this second test typing PR :) I pushed a mypy config improvement first, and am now merging your PR with only a few minor name adjustments in the narrowing test improvements 🎉 A good related follow-up, other than the other typing work, would be to decouple that last case of streambutton you noticed in the code and fix the tests accordingly 👍 |
|
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 the test_core.py file. I've also added a few refactor/bugfix commits.
Tested?
Commit flow
report_*methods.Interactions
run-mypytool.