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.

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

messages: Add tests for transform_content. #1507

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

messages: Add tests for transform_content. #1507

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
110 changes: 109 additions & 1 deletion tests/ui_tools/test_messages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
from collections import OrderedDict, defaultdict
from datetime import date
from typing import Any, Dict, List, Tuple

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

from zulipterminal.config.keys import keys_for_command
Expand Down Expand Up @@ -1585,14 +1587,120 @@ def test_keypress_EDIT_MESSAGE(
# fmt: on
],
)
def test_transform_content(self, mocker, raw_html, expected_content):
def test_transform_content(self, raw_html: str, expected_content: Any) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure if there's a need for this change as we don't usually define data type for the function arguments or the function return variable as we decide what to pass.
You may not keep them in the newly created test function as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is not currently type-checked, though it would be good to do so (there's an issue for it). Adding these right now are more like comments, which is fine but won't be tested or maintained, which is the downside - though if they keep passing manual review then they should help when we do apply mypy to these files.

Given the introduction of the similar transform_content tests, it would make sense to rename this test to define what set of cases it handles, so I'd consider it reasonable to update nearby lines like the types.

expected_content = expected_content.replace("{}", QUOTED_TEXT_MARKER)

content, *_ = MessageBox.transform_content(raw_html, SERVER_URL)

rendered_text = Text(content)
assert rendered_text.text == expected_content

@pytest.mark.parametrize(
"raw_html, expected_message_links",
[
case(
"""
<p><a href="https://github.com/zulip/zulip-terminal/pull/1">https://github.com/zulip/zulip-terminal/pull/1</a></p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we have some link where the text is not the same as the link?
One of the first two?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done :)

""",
{
"https://github.com/zulip/zulip-terminal/pull/1": (
"github.com",
1,
True,
)
},
id="github_pr_link_matches_text",
),
case(
"""
<p><a href="https://foo.com">https://foo.com</a></p>
""",
{"https://foo.com": ("https://foo.com", 1, False)},
id="external_link_no_footlink",
),
case(
"""
<p><a href="#narrow/stream/206-zulip-terminal/topic/announce">https://chat.zulip.zulip/#narrow/stream/206-zulip-terminal/topic/announce</a></p>
neiljp marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these test cases all from real example messages, to indicate this is what we actually receive?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I printed in debug mode and got them.

""",
{
"https://chat.zulip.zulip#narrow/stream/206-zulip-terminal/topic/announce": ( # noqa: E501
"https://chat.zulip.zulip/#narrow/stream/206-zulip-terminal/topic/announce",
1,
True,
)
},
id="internal_link",
),
case(
"""
<p><a href="https://github.com/zulip/zulip-terminal">test</a></p>
""",
{"https://github.com/zulip/zulip-terminal": ("test", 1, True)},
id="github_link_with_text",
),
],
)
def test_transform_content_message_links(
self, raw_html: str, expected_message_links: Dict[str, Tuple[str, int, bool]]
) -> None:
_, message_links, *_ = MessageBox.transform_content(raw_html, SERVER_URL)

assert message_links == expected_message_links

@pytest.mark.parametrize(
"raw_html, expected_time_mentions",
[
case(
"""
<p><time datetime="2024-05-29T11:30:00Z">2024-05-29T17:00:00+05:30</time></p>
""", # noqa: E501
[
(
"Wed, May 29 2024, 17:00 (IST)",
"Original text was 2024-05-29T17:00:00+05:30",
)
],
id="test_date",
),
case(
"""
<p><time datetime="3000-06-01T14:00:00Z">3000-06-01T19:30:00+05:30</time></p>
""", # noqa: E501
[
(
"Sun, Jun 1 3000, 19:30 (IST)",
"Original text was 3000-06-01T19:30:00+05:30",
)
],
id="distant_future_date",
),
case(
"""
<p><time datetime="1947-08-14T19:47:00Z">1947-08-15T01:17:00+05:30</time></p>
""", # noqa: E501
[
(
"Fri, Aug 15 1947, 1:17 (IST)",
"Original text was 1947-08-15T01:17:00+05:30",
)
],
id="past_date",
),
],
)
def test_transform_content_time_mentions(
self,
mocker: MockerFixture,
raw_html: str,
expected_time_mentions: List[Tuple[str, str]],
) -> None:
mocker.patch(
MODULE + ".get_localzone", return_value=pytz.timezone("Asia/Kolkata")
)
_, _, time_mentions, *_ = MessageBox.transform_content(raw_html, SERVER_URL)

assert time_mentions == expected_time_mentions

# FIXME This is the same parametrize as MsgInfoView:test_height_reactions
@pytest.mark.parametrize(
"to_vary_in_each_message, expected_text, expected_attributes",
Expand Down
Loading