Skip to content

Commit

Permalink
bugfix: tests: Fix WSL GUI exit status related tests.
Browse files Browse the repository at this point in the history
WSL always returns a non-zero exit code. The `successful_GUI_return_code()`
function has been refactored to directly compare exit statuses and return
"success" or "failure" accordingly.

Fix open_media test to validate both the tool and the error code across
various platforms.

Refactor process_media to shift platform dependent code to platform_code.py.
  • Loading branch information
rsashank committed Sep 30, 2024
1 parent 3b52fcd commit efa23bb
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 38 deletions.
41 changes: 33 additions & 8 deletions tests/helper/test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ def test_download_media(
("Linux", True, True, "xdg-open", "/path/to/media"),
("MacOS", True, True, "open", "/path/to/media"),
("WSL", True, True, "explorer.exe", "\\path\\to\\media"),
("UnknownOS", True, False, "unknown-tool", "/path/to/media"),
("UnknownOS", True, False, "invalid", "/path/to/media"),
],
ids=[
"Linux_os_user",
Expand All @@ -572,9 +572,13 @@ def test_process_media(
MODULE + ".download_media", return_value=media_path
)
mocked_open_media = mocker.patch(MODULE + ".open_media")
mocker.patch(MODULE + ".PLATFORM", platform)
mocker.patch("zulipterminal.platform_code.PLATFORM", platform)
mocker.patch("zulipterminal.core.Controller.show_media_confirmation_popup")

# Mocking the PLATFORM variable in both platform_code and the main module
# to ensure consistency during tests that rely on platform detection.
mocker.patch("zulipterminal.platform_code" + ".PLATFORM", platform)
mocker.patch(
"zulipterminal.platform_code" + ".process_media_tool", return_value=tool
)

process_media(controller, link)

Expand Down Expand Up @@ -603,28 +607,49 @@ def test_process_media_empty_url(


@pytest.mark.parametrize(
"returncode, error",
"platform, returncode, tool, error",
[
(0, []),
(
case("Linux", 0, "xdg-open", [], id="Linux:success"),
case("MacOS", 0, "open", [], id="MacOS:success"),
case("WSL", 1, "explorer.exe", [], id="WSL:success"),
case(
"Linux",
1,
"xdg-open",
[
" The tool ",
("footer_contrast", "xdg-open"),
" did not run successfully" ". Exited with ",
("footer_contrast", "1"),
],
id="Linux:error",
),
case(
"MacOS",
1,
"open",
[
" The tool ",
("footer_contrast", "open"),
" did not run successfully" ". Exited with ",
("footer_contrast", "1"),
],
id="Mac:error",
),
# NOTE: explorer.exe (WSL) always returns a non-zero exit code (1)
# so we do not test for it.
],
)
def test_open_media(
mocker: MockerFixture,
platform: str,
returncode: int,
tool: str,
error: List[Any],
tool: str = "xdg-open",
media_path: str = "/tmp/zt-somerandomtext-image.png",
) -> None:
mocked_run = mocker.patch(MODULE + ".subprocess.run")
mocker.patch("zulipterminal.platform_code" + ".PLATFORM", platform)
mocked_run.return_value.returncode = returncode
controller = mocker.Mock()

Expand Down
21 changes: 13 additions & 8 deletions tests/platform_code/test_platform_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
SupportedPlatforms,
normalized_file_path,
notify,
successful_GUI_return_code,
validate_GUI_exit_status,
)


Expand Down Expand Up @@ -76,20 +76,25 @@ def test_notify_quotes(


@pytest.mark.parametrize(
"platform, expected_return_code",
"platform, return_code, expected_return_value",
[
("Linux", 0),
("MacOS", 0),
("WSL", 1),
("Linux", 0, "success"),
("MacOS", 0, "success"),
("WSL", 1, "success"),
("Linux", 1, "failure"),
("MacOS", 1, "failure"),
# NOTE: explorer.exe (WSL) always returns a non-zero exit code (1),
# so we do not test for it.
],
)
def test_successful_GUI_return_code(
def test_validate_GUI_exit_status(
mocker: MockerFixture,
platform: SupportedPlatforms,
expected_return_code: int,
return_code: int,
expected_return_value: str,
) -> None:
mocker.patch(MODULE + ".PLATFORM", platform)
assert successful_GUI_return_code() == expected_return_code
assert validate_GUI_exit_status(return_code) == expected_return_value


@pytest.mark.parametrize(
Expand Down
22 changes: 7 additions & 15 deletions zulipterminal/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@
REGEX_QUOTED_FENCE_LENGTH,
)
from zulipterminal.platform_code import (
PLATFORM,
process_media_tool,
normalized_file_path,
successful_GUI_return_code,
validate_GUI_exit_status,
)


Expand Down Expand Up @@ -785,18 +785,10 @@ def process_media(controller: Any, link: str) -> None:
)
media_path = download_media(controller, link, show_download_status)
media_path = normalized_file_path(media_path)
tool = ""

# TODO: Add support for other platforms as well.
if PLATFORM == "WSL":
tool = "explorer.exe"
# Modifying path to backward slashes instead of forward slashes
media_path = media_path.replace("/", "\\")
elif PLATFORM == "Linux":
tool = "xdg-open"
elif PLATFORM == "MacOS":
tool = "open"
else:

tool = process_media_tool()

if tool == "invalid":
controller.report_error("Media not supported for this platform")
return

Expand Down Expand Up @@ -841,7 +833,7 @@ def open_media(controller: Any, tool: str, media_path: str) -> None:
command, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL
)
exit_status = process.returncode
if exit_status != successful_GUI_return_code():
if validate_GUI_exit_status(exit_status) == "failure":
error = [
" The tool ",
("footer_contrast", tool),
Expand Down
15 changes: 8 additions & 7 deletions zulipterminal/platform_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,19 @@ def notify(title: str, text: str) -> str:
return ""


def successful_GUI_return_code() -> int: # noqa: N802 (allow upper case)
def validate_GUI_exit_status( # noqa: N802 (allow upper case)
exit_status: int,
) -> Literal["success", "failure"]:
"""
Returns success return code for GUI commands, which are OS specific.
"""
# WSL uses GUI return code as 1. Refer below link to know more:
# https://stackoverflow.com/questions/52423031/
# why-does-opening-an-explorer-window-and-selecting-a-file-through-pythons-subpro/
# 52423798#52423798
# NOTE: WSL (explorer.exe) always returns a non-zero exit code (1) for GUI commands.
# This is a known issue. Therefore, we consider it a success if the exit code is 1.
# For more information, see: https://github.com/microsoft/WSL/issues/6565
if PLATFORM == "WSL":
return 1
return "success"

return 0
return "success" if exit_status == 0 else "failure"


def normalized_file_path(path: str) -> str:
Expand Down

0 comments on commit efa23bb

Please sign in to comment.