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

tests: popups: Add type annotations. #1125

Merged
merged 6 commits into from
Aug 25, 2021
Merged

Conversation

prah23
Copy link
Member

@prah23 prah23 commented Aug 21, 2021

What does this PR do?
Adds type annotations to test_popups.py and adds the file to the list in run-mypy. Also has a few related prior refactors to ensure type consistency.

Tested?

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

Commit flow

  • refactor: tests: popups: Remove unnecessary parameters.
    This commit removes the passing of parameters in test definitions
    for parameters that are unused within the body of the test.

  • refactor: tests: popups: Use Message instances to pass into views.
    This commit refactors the multiple dict assignments to View classes'
    message attribute to Message instances instead to maintain type
    consistency.

  • refactor: tests: popups: Assign mocked keypress to a variable.
    This commit assigns the mocked keypress method of the
    emoji_picker_view within test_mouse_event of the TestEmojiPickerView
    class to a variable, so that the corresponding assert method calls
    are type consistent.

  • refactor: tests: popups: Return a PopUpView instance from pop_up_view.
    This commit refactors the pop_up_view fixture to return a PopUpView
    instance for usage in further tests, instead of assigning the instance
    to an attribute of the TestPopUpView class, to maintain type
    consistency.

  • refactor: tests: popups: Add type annotations.
    This commit adds parameter and return type annotations or hints
    to the test_popups.py file, that contains tests for the
    corresponding classes present in views.py from the
    zulipterminal module, to make mypy checks consistent and
    improve code readability.

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

prah23 added 3 commits August 21, 2021 19:49
This commit removes the passing of parameters in test definitions
for parameters that are unused within the body of the test.
This commit refactors the multiple `dict` assignments to View classes'
message attribute to Message instances instead to maintain type
consistency.
This commit assigns the mocked keypress method of the
emoji_picker_view within `test_mouse_event` of the TestEmojiPickerView
class to a variable, so that the corresponding assert method calls
are type consistent.
@zulipbot zulipbot added the size: XL [Automatic label added by zulipbot] label Aug 21, 2021
@prah23 prah23 force-pushed the test_annotation_popups branch from 9c94d07 to 432819c Compare August 21, 2021 15:56
@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!

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 almost good to go - just a few inline queries 👍

tests/ui_tools/test_popups.py Outdated Show resolved Hide resolved
tests/ui_tools/test_popups.py Show resolved Hide resolved
@neiljp neiljp added the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Aug 23, 2021
@prah23 prah23 removed the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Aug 23, 2021
neiljp and others added 3 commits August 24, 2021 18:11
This was originally identified by prah23 <[email protected]>.

This commit resolves the name conflict via a rename, rather than
adjusting the autouse fixture to return an object which is explicitly
used on demand (rather than being assigned to the fixture name).
This commit adds parameter and return type annotations or hints
to the `test_popups.py` file, that contains tests for the
corresponding classes present in `views.py` from  the
`zulipterminal` module, to make mypy checks consistent and
improve code readability.
This commit adds `test_popups.py` to the `type_consistent_testfiles`
list to check for type consistency with mypy.
@neiljp neiljp force-pushed the test_annotation_popups branch from 432819c to eeecb67 Compare August 25, 2021 01:17
@neiljp
Copy link
Collaborator

neiljp commented Aug 25, 2021

@prah23 I think you actually found a bug with the test - compare the previous name of the autouse fixture and the attribute we assign! I made a simpler change than the fixture change you had previously and am going to merge that with your other commits shortly 🎉

I'm looking forward to run-mypy just having tests/ rather than all the file-names :)

@neiljp neiljp merged commit 22d576b into zulip:main Aug 25, 2021
@neiljp neiljp added this to the Next Release milestone Aug 25, 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