-
-
Notifications
You must be signed in to change notification settings - Fork 273
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
refactor: tests: buttons: Add type annotations. #1105
Conversation
5b5a1ec
to
7426970
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.
@prah23 This looks broadly good with only a few issues I could see from first review 👍
expected_prefix: List[str], | ||
text_color: Optional[str], | ||
count_text: Tuple[Optional[str], str], | ||
expected_suffix: List[Any], |
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.
Improving this is pending on the urwid markup PR, or something like it.
7426970
to
3bb184c
Compare
Thanks for taking a look, @neiljp! I've accommodated the changes suggested, only the second point on the review is left, which is pending on other urwid-markup typing refactor work (I've also updated the PR description accordingly). |
Hello @zulip/server-refactoring members, this pull request was labeled with the "area: refactoring" label, so you may want to check it out! |
90a79b2
to
aaac0d4
Compare
This commit refactors certain test parameters that are of the `ParsedNarrowLink` type to `ParsedNarrowLink` instances and parameters of the type `DecodedStream` to `DecodeStream` instances from the current dict instances to ensure type consistency.
This commit assigns fixture method attribute `mocker.patch`s to variables, so that the corresponding asserts can be called on the mocked nature, as was the original intention.
This commit adds parameter and return type annotations or hints to the `test_buttons.py` file, that contains tests for it's counterpart `buttons.py` from the `zulipterminal` module, to make mypy checks consistent and improve code readability.
This commit adds `test_buttons.py` to the `type_consistent_testfiles` list to check for type consistency with mypy.
aaac0d4
to
28d3ade
Compare
@prah23 Thanks for this and the updates 👍 Merging with slight commit message tweaks shortly! 🎉 |
What does this PR do?
Adds type annotations to test_buttons.py and adds the file to the list in run-mypy. Also has a prior refactor commit to assign
mocker.patch
s to variables.Tested?
Commit flow
refactor: tests: buttons: Assign method mocker.patchs to variables.
This commit assigns fixture method attribute
mocker.patch
s tovariables, so that the corresponding asserts can be called on the
mocked nature, as was the original intention.
refactor: tests: buttons: Add type annotations.
This commit adds parameter and return type annotations or hints to the
test_buttons.py
file, that contains tests for it's counterpartbuttons.py
from thezulipterminal
module, to make mypy checksconsistent and improve code readability.
tools: Include test_buttons.py to be checked by mypy.
This commit adds
test_buttons.py
to thetype_consistent_testfiles
list to check for type consistency with mypy.
Waiting on:
Refactor work to improve type annotations for urwid markup.