Skip to content

Add type hints to test_message.py #1407

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add type hints to test_message.py #1407

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
175 changes: 117 additions & 58 deletions tests/ui_tools/test_messages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
from collections import OrderedDict, defaultdict
from datetime import date
from typing import List, Tuple, Any, Optional, Dict, Callable
from pytest_mock import MockerFixture
from zulipterminal.api_types import MessageType, Message
from zulipterminal.urwid_types import urwid_Size

import pytest
import pytz
from bs4 import BeautifulSoup
from pytest import param as case
from urwid import Columns, Divider, Padding, Text
from urwid import Columns, Divider, Padding, Text, Widget

from zulipterminal.config.keys import keys_for_command
from zulipterminal.config.symbols import (
Expand All @@ -25,7 +29,7 @@

class TestMessageBox:
@pytest.fixture(autouse=True)
def mock_external_classes(self, mocker, initial_index):
def mock_external_classes(self, mocker: MockerFixture, initial_index: int) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here initial_index is a fixture, not an int. Please see #1337 for some details on fixtures and where they're often defined. If it's not obvious where a parameter comes from, it's often a fixture!

self.model = mocker.MagicMock()
self.model.index = initial_index

Expand All @@ -36,7 +40,12 @@ def mock_external_classes(self, mocker, initial_index):
("private", [("email", ""), ("user_id", None)]),
],
)
def test_init(self, mocker, message_type, set_fields):
def test_init(
self,
mocker: MockerFixture,
message_type: str,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For message_type, we likely want the new MessageType in api_types.py; that's also a string, but a more constrained type.

set_fields: List[Tuple[str, Any]]
) -> None:
mocker.patch.object(MessageBox, "main_view")
message = dict(
display_recipient=[
Expand All @@ -59,13 +68,13 @@ def test_init(self, mocker, message_type, set_fields):
assert msg_box.message_links == OrderedDict()
assert msg_box.time_mentions == list()

def test_init_fails_with_bad_message_type(self):
def test_init_fails_with_bad_message_type(self) -> None:
message = dict(type="BLAH")

with pytest.raises(RuntimeError):
MessageBox(message, self.model, None)

def test_private_message_to_self(self, mocker):
def test_private_message_to_self(self, mocker: MockerFixture) -> None:
message = dict(
type="private",
display_recipient=[
Expand Down Expand Up @@ -660,7 +669,11 @@ def test_private_message_to_self(self, mocker):
),
],
)
def test_soup2markup(self, content, expected_markup, mocker):
def test_soup2markup(
self, content: str,
expected_markup: str,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ensure you understand how parametrize works, so that you can determine a good type for expected_markup.

mocker: MockerFixture
) -> None:
mocker.patch(
MODULE + ".get_localzone", return_value=pytz.timezone("Asia/Kolkata")
)
Expand Down Expand Up @@ -742,7 +755,12 @@ def test_soup2markup(self, content, expected_markup, mocker):
),
],
)
def test_main_view(self, mocker, message, last_message):
def test_main_view(
self,
mocker: MockerFixture,
message: Dict[str, Any],
last_message: Optional[Dict[str, Any]]
Comment on lines +761 to +762
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These dicts, and those in other changes, should be replaced by Message, again in api_types.py.

) -> None:
self.model.stream_dict = {
5: {
"color": "#bd6",
Expand Down Expand Up @@ -777,7 +795,13 @@ def test_main_view(self, mocker, message, last_message):
("<p>/me is excited!</p>", False),
],
)
def test_main_view_renders_slash_me(self, mocker, message, content, is_me_message):
def test_main_view_renders_slash_me(
self,
mocker: MockerFixture,
message: Dict[str, Any],
content: str,
is_me_message: bool
) -> None:
mocker.patch(MODULE + ".urwid.Text")
message["content"] = content
message["is_me_message"] = is_me_message
Expand Down Expand Up @@ -820,8 +844,11 @@ def test_main_view_renders_slash_me(self, mocker, message, content, is_me_messag
],
)
def test_main_view_generates_stream_header(
self, mocker, message, to_vary_in_last_message
):
self,
mocker: MockerFixture,
message: Dict[str, Any],
to_vary_in_last_message: bool
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are another case of you needing to understand parametrize first.

) -> None:
self.model.stream_dict = {
5: {
"color": "#bd6",
Expand Down Expand Up @@ -877,8 +904,11 @@ def test_main_view_generates_stream_header(
],
)
def test_main_view_generates_PM_header(
self, mocker, message, to_vary_in_last_message
):
self,
mocker: MockerFixture,
message: Dict[str, Any],
to_vary_in_last_message: bool
) -> None:
last_message = dict(message, **to_vary_in_last_message)
msg_box = MessageBox(message, self.model, last_message)
view_components = msg_box.main_view()
Expand Down Expand Up @@ -945,13 +975,13 @@ def test_main_view_generates_PM_header(
)
def test_msg_generates_search_and_header_bar(
self,
mocker,
messages_successful_response,
msg_type,
msg_narrow,
assert_header_bar,
assert_search_bar,
):
mocker: MockerFixture,
messages_successful_response: Dict[str, Any],
msg_type: MessageType,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see you used MessageType here - but note that the variable type is different here, which I can tell from reading the parametrize (as per other comments).

msg_narrow: list,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think mypy will complain if you use a list with no typing, even if that typing is Any (which we may need here) - and with our current setup we likely need List too.

assert_header_bar: str,
assert_search_bar: str,
) -> None:
self.model.stream_dict = {
205: {
"color": "#bd6",
Expand Down Expand Up @@ -1012,13 +1042,13 @@ def test_msg_generates_search_and_header_bar(
)
def test_main_view_content_header_without_header(
self,
mocker,
message,
expected_header,
current_year,
starred_msg,
to_vary_in_last_message,
):
mocker: MockerFixture,
message: Dict[str, Any],
expected_header: List[str],
current_year: int,
starred_msg: str,
to_vary_in_last_message: Dict[str, Any],
) -> None:
mocked_date = mocker.patch(MODULE + ".date")
mocked_date.today.return_value = date(current_year, 1, 1)
mocked_date.side_effect = lambda *args, **kw: date(*args, **kw)
Expand Down Expand Up @@ -1073,8 +1103,11 @@ def test_main_view_content_header_without_header(
],
)
def test_main_view_compact_output(
self, mocker, message_fixture, to_vary_in_each_message
):
self,
mocker: MockerFixture,
message_fixture: Message,
to_vary_in_each_message: dict
) -> None:
message_fixture.update({"id": 4})
varied_message = dict(message_fixture, **to_vary_in_each_message)
msg_box = MessageBox(varied_message, self.model, varied_message)
Expand All @@ -1083,8 +1116,10 @@ def test_main_view_compact_output(
assert isinstance(view_components[0], Padding)

def test_main_view_generates_EDITED_label(
self, mocker, messages_successful_response
):
self,
mocker: MockerFixture,
messages_successful_response: Dict[str, Any]
) -> None:
messages = messages_successful_response["messages"]
for message in messages:
self.model.index["edited_messages"].add(message["id"])
Expand All @@ -1108,10 +1143,10 @@ def test_main_view_generates_EDITED_label(
)
def test_update_message_author_status(
self,
message_fixture,
update_required,
to_vary_in_last_message,
):
message_fixture: Message,
update_required: bool,
to_vary_in_last_message: Dict[str, Any],
)-> None:
message = message_fixture
last_msg = dict(message, **to_vary_in_last_message)

Expand Down Expand Up @@ -1142,8 +1177,14 @@ def test_update_message_author_status(
],
)
def test_keypress_STREAM_MESSAGE(
self, mocker, msg_box, widget_size, narrow, expect_to_prefill, key
):
self,
mocker: MockerFixture,
msg_box: MessageBox,
widget_size: Callable[[Widget], urwid_Size],
narrow: Dict[str, Any],
expect_to_prefill: bool,
key: str
) -> None:
write_box = msg_box.model.controller.view.write_box
msg_box.model.narrow = narrow
size = widget_size(msg_box)
Expand Down Expand Up @@ -1284,17 +1325,17 @@ def test_keypress_STREAM_MESSAGE(
)
def test_keypress_EDIT_MESSAGE(
self,
mocker,
message_fixture,
widget_size,
to_vary_in_each_message,
realm_editing_allowed,
msg_body_edit_limit,
expect_msg_body_edit_enabled,
expect_editing_to_succeed,
expect_footer_text,
key,
):
mocker: MockerFixture,
message_fixture: Message,
widget_size: Callable[[Widget], urwid_Size],
to_vary_in_each_message: bool,
realm_editing_allowed: bool,
msg_body_edit_limit: int,
expect_msg_body_edit_enabled: bool,
expect_editing_to_succeed: bool,
expect_footer_text: Optional[str],
key: str,
) -> None:
if message_fixture["type"] == "private":
to_vary_in_each_message["subject"] = ""
varied_message = dict(message_fixture, **to_vary_in_each_message)
Expand Down Expand Up @@ -1493,7 +1534,12 @@ def test_keypress_EDIT_MESSAGE(
# fmt: on
],
)
def test_transform_content(self, mocker, raw_html, expected_content):
def test_transform_content(
self,
mocker: MockerFixture,
raw_html: str,
expected_content: str
) -> None:
expected_content = expected_content.replace("{}", QUOTED_TEXT_MARKER)

content, *_ = MessageBox.transform_content(raw_html, SERVER_URL)
Expand Down Expand Up @@ -1719,11 +1765,11 @@ def test_transform_content(self, mocker, raw_html, expected_content):
)
def test_reactions_view(
self,
message_fixture,
to_vary_in_each_message,
expected_text,
expected_attributes,
):
message_fixture: Message,
to_vary_in_each_message: bool,
expected_text: str,
expected_attributes: Dict[str, Any],
) -> None:
self.model.user_id = 1
varied_message = dict(message_fixture, **to_vary_in_each_message)
msg_box = MessageBox(varied_message, self.model, None)
Expand Down Expand Up @@ -1827,8 +1873,12 @@ def test_reactions_view(
],
)
def test_footlinks_view(
self, message_links, expected_text, expected_attrib, expected_footlinks_width
):
self,
message_links: List[str],
expected_text: Optional[str],
expected_attrib: Optional[Dict[str, Any]],
expected_footlinks_width: Optional[int]
Comment on lines +1877 to +1880
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These need you to understand parametrize.

) -> None:
footlinks, footlinks_width = MessageBox.footlinks_view(
message_links,
maximum_footlinks=3,
Expand All @@ -1852,7 +1902,11 @@ def test_footlinks_view(
(3, Padding),
],
)
def test_footlinks_limit(self, maximum_footlinks, expected_instance):
def test_footlinks_limit(
self,
maximum_footlinks: int,
expected_instance: Any
) -> None:
message_links = OrderedDict(
[
("https://github.com/zulip/zulip-terminal", ("ZT", 1, True)),
Expand All @@ -1872,8 +1926,13 @@ def test_footlinks_limit(self, maximum_footlinks, expected_instance):
"key", keys_for_command("ENTER"), ids=lambda param: f"left_click-key:{param}"
)
def test_mouse_event_left_click(
self, mocker, msg_box, key, widget_size, compose_box_is_open
):
self,
mocker: MockerFixture,
msg_box: MessageBox,
key: str,
widget_size: Callable[[Widget], urwid_Size],
compose_box_is_open: bool
) -> None:
size = widget_size(msg_box)
col = 1
row = 1
Expand Down