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: boxes: Add type annotations. #1102

Merged
merged 5 commits into from
Aug 17, 2021
Merged

Conversation

prah23
Copy link
Member

@prah23 prah23 commented Jul 27, 2021

What does this PR do?
Adds type annotations to test_boxes.py and adds the file to the list in run-mypy. Also has a prior refactor commit to remove unnecessary mocker parameters.

Tested?

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

Commit flow

  • refactor: tests: boxes: Remove mocker as parameter where unnecessary.
    This commit removes the mocker fixture as a parameter from tests
    where it isn't used.

  • refactor: tests: boxes: Add type annotations.
    This commit adds parameter and return type annotations or hints to the
    test_boxes.py file, that contains tests for it's counterpart
    boxes.py from the zulipterminal module, to make mypy checks
    consistent and improve code readability.

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

@zulipbot zulipbot added the size: XL [Automatic label added by zulipbot] label Jul 27, 2021
@prah23 prah23 added PR needs review PR requires feedback to proceed area: refactoring area: tests labels Jul 27, 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!

@prah23 prah23 force-pushed the test_annotation_boxes branch from e9e3a7b to cce1d98 Compare August 2, 2021 11:30
@prah23 prah23 force-pushed the test_annotation_boxes branch 3 times, most recently from 74150b5 to b00f4e6 Compare August 2, 2021 12:00
@neiljp neiljp force-pushed the test_annotation_boxes branch from b00f4e6 to ce4d74b Compare August 6, 2021 01:29
@neiljp
Copy link
Collaborator

neiljp commented Aug 11, 2021

As I mentioned in #1096, I think that is best done before this, to finalize the type annotations here.

@neiljp
Copy link
Collaborator

neiljp commented Aug 17, 2021

@prah23 I was just going to look at this again, but there may have been boxes merges since, as well as the other tests merge - could you do a quick rebase?

@prah23
Copy link
Member Author

prah23 commented Aug 17, 2021

@neiljp sure, I'll just review the types again once, since you wanted the conftest PR to be merged before this one to have a concrete reference for what the fixture types should be and rebase.

@prah23 prah23 force-pushed the test_annotation_boxes branch from ce4d74b to 66821ec Compare August 17, 2021 13:32
@prah23
Copy link
Member Author

prah23 commented Aug 17, 2021

PR updated.

@prah23 prah23 force-pushed the test_annotation_boxes branch from 66821ec to f4da11e Compare August 17, 2021 17:09
prah23 added 5 commits August 17, 2021 10:31
This commit patches the `write_box` fixture's
`_set_stream_box_write_style` using `mocker.patch` instead of assigning
a `mocker.Mock` to it to maintain type consistency.
This commit removes the `mocker` fixture as a parameter from tests
where it isn't used.
This commit adds assert statements within multiple tests to assert
that the value of WriteBox's `to_write_box` is None after a
`private_box_view` function call, since it initiates `to_write_box`
with a ReadlineEdit instance.

This is also necessary for supporting mypy, since the attribute type
depends on whether it is a private box or not.
This commit adds parameter and return type annotations or hints to the
`test_boxes.py` file, that contains tests for it's counterpart
`boxes.py` from  the `zulipterminal` module, to make mypy checks
consistent and improve code readability.
This commit adds `test_boxes.py` to the `type_consistent_testfiles`
list to check for type consistency with mypy.
@neiljp neiljp force-pushed the test_annotation_boxes branch from f4da11e to cb73646 Compare August 17, 2021 18:08
@neiljp neiljp merged commit c9a59fa into zulip:main Aug 17, 2021
@neiljp
Copy link
Collaborator

neiljp commented Aug 17, 2021

@prah23 Thanks for keeping up with this 🎉

@neiljp neiljp removed the PR needs review PR requires feedback to proceed label Aug 17, 2021
@neiljp neiljp added this to the Next Release milestone Aug 17, 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