Skip to content
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

Merged
merged 4 commits into from
Jul 30, 2021

Conversation

prah23
Copy link
Member

@prah23 prah23 commented Jul 28, 2021

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.patchs to variables.

Tested?

  • Manually
  • Existing tests (adapted, if necessary)
  • Passed linting & tests (each commit)

Commit flow

  • refactor: tests: buttons: Assign method mocker.patchs to variables.
    This commit assigns fixture method attribute mocker.patchs to
    variables, 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 counterpart
    buttons.py from the zulipterminal module, to make mypy checks
    consistent and improve code readability.

  • tools: Include test_buttons.py to be checked by mypy.
    This commit adds test_buttons.py to the type_consistent_testfiles
    list to check for type consistency with mypy.

Waiting on:
Refactor work to improve type annotations for urwid markup.

@zulipbot zulipbot added the size: XL [Automatic label added by zulipbot] label Jul 28, 2021
@prah23 prah23 force-pushed the test_annotation_buttons branch from 5b5a1ec to 7426970 Compare July 28, 2021 21:27
Copy link
Collaborator

@neiljp neiljp left a 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],
Copy link
Collaborator

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.

@neiljp neiljp added the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Jul 29, 2021
@prah23 prah23 force-pushed the test_annotation_buttons branch from 7426970 to 3bb184c Compare July 30, 2021 07:43
@prah23
Copy link
Member Author

prah23 commented Jul 30, 2021

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).

@prah23 prah23 removed the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Jul 30, 2021
@zulipbot
Copy link
Member

Hello @zulip/server-refactoring members, this pull request was labeled with the "area: refactoring" label, so you may want to check it out!

@neiljp neiljp added this to the Next Release milestone Jul 30, 2021
@prah23 prah23 force-pushed the test_annotation_buttons branch 2 times, most recently from 90a79b2 to aaac0d4 Compare July 30, 2021 17:38
prah23 added 4 commits July 30, 2021 11:18
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.
@neiljp neiljp force-pushed the test_annotation_buttons branch from aaac0d4 to 28d3ade Compare July 30, 2021 18:32
@neiljp
Copy link
Collaborator

neiljp commented Jul 30, 2021

@prah23 Thanks for this and the updates 👍 Merging with slight commit message tweaks shortly! 🎉

@neiljp neiljp merged commit 4c939a6 into zulip:main Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: refactoring area: tests size: XL [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants