diff --git a/tests/helper/test_helper.py b/tests/helper/test_helper.py index 216b03e16a..118cd9b609 100644 --- a/tests/helper/test_helper.py +++ b/tests/helper/test_helper.py @@ -603,28 +603,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() diff --git a/tests/platform_code/test_platform_code.py b/tests/platform_code/test_platform_code.py index 21669da025..d0c94018d0 100644 --- a/tests/platform_code/test_platform_code.py +++ b/tests/platform_code/test_platform_code.py @@ -6,7 +6,7 @@ SupportedPlatforms, normalized_file_path, notify, - successful_GUI_return_code, + validate_GUI_exit_status, ) @@ -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( diff --git a/zulipterminal/helper.py b/zulipterminal/helper.py index a71d055b74..d978cd4ad7 100644 --- a/zulipterminal/helper.py +++ b/zulipterminal/helper.py @@ -42,7 +42,7 @@ from zulipterminal.platform_code import ( PLATFORM, normalized_file_path, - successful_GUI_return_code, + validate_GUI_exit_status, ) @@ -841,7 +841,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), diff --git a/zulipterminal/platform_code.py b/zulipterminal/platform_code.py index 2741e8ec38..cb652a61d3 100644 --- a/zulipterminal/platform_code.py +++ b/zulipterminal/platform_code.py @@ -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: