Use command return codes and return 1 for errors.

Use the return code from executed user commands instead of returning 0
in all cases.

Return 1 instead of -1 for Protontricks errors per Unix conventions.

Fixes #126

CHANGELOG.md

@@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [1.6.1] - 2021-10-18
+### Changed
+ - Exit status is now returned from the executed commands
+ - Return code `1` is returned for most Protontricks errors instead of `-1`
+
 ### Fixed
  - Fix duplicate Steam application entries
  - Fix crash on Python 3.5

src/protontricks/cli/main.py

@@ -281,16 +281,17 @@ def exit_(error):
         )
 
     if args.winetricks_command:
-        run_command(
+        returncode = run_command(
             winetricks_path=winetricks_path,
             proton_app=proton_app,
             steam_app=steam_app,
             use_steam_runtime=use_steam_runtime,
             legacy_steam_runtime_path=legacy_steam_runtime_path,
             use_bwrap=use_bwrap,
-            command=[str(winetricks_path)] + args.winetricks_command)
+            command=[str(winetricks_path)] + args.winetricks_command
+        )
     elif args.command:
-        run_command(
+        returncode = run_command(
             winetricks_path=winetricks_path,
             proton_app=proton_app,
             steam_app=steam_app,
@@ -301,7 +302,10 @@ def exit_(error):
             # Pass the command directly into the shell *without*
             # escaping it
             cwd=str(steam_app.install_path),
-            shell=True)
+            shell=True
+        )
+
+    sys.exit(returncode)
 
 
 if __name__ == "__main__":

src/protontricks/cli/util.py

@@ -102,7 +102,7 @@ def _get_zenity_args():
 
     if not desktop:
         print(error)
-        sys.exit(-1)
+        sys.exit(1)
 
     try:
         log_messages = _get_log_file_path().read_text()
@@ -125,7 +125,7 @@ def _get_zenity_args():
         args = _get_zenity_args()
 
     run(args, input=message.encode("utf-8"), check=False)
-    sys.exit(-1)
+    sys.exit(1)
 
 
 def cli_error_handler(cli_func):

src/protontricks/gui.py

@@ -202,7 +202,7 @@ def run_gui(args, input_=None, strip_nonascii=False):
 
     if choice in (b"", b" \n"):
         print("No game was selected. Quitting...")
-        sys.exit(0)
+        sys.exit(1)
 
     appid = str(choice).rsplit(':')[-1]
     appid = ''.join(x for x in appid if x.isdigit())

src/protontricks/util.py

@@ -289,6 +289,8 @@ def run_command(
 
     If 'use_bwrap' is True, run newer Steam Runtime installations using
     bwrap based containerization.
+
+    :returns: Return code of the executed command
     """
     # Check for incomplete Steam Runtime installation
     runtime_install_incomplete = \
@@ -401,7 +403,8 @@ def run_command(
     logger.info("Attempting to run command %s", command)
 
     try:
-        run(command, **kwargs)
+        result = run(command, check=False, **kwargs)
+        return result.returncode
     finally:
         # Restore original env vars
         os.environ.clear()

tests/cli/test_launch.py

@@ -58,7 +58,7 @@ def test_run_executable_no_selection(
         # Fake the user closing the form
         gui_provider.mock_stdout = ""
 
-        result = launch_cli(["test.exe"], expect_exit=True)
+        result = launch_cli(["test.exe"], expect_returncode=1)
 
         assert "No game was selected." in result
 
@@ -67,7 +67,7 @@ def test_run_executable_no_apps(self, launch_cli):
         Try running an EXE file when no Proton enabled Steam apps are installed
         or ready
         """
-        result = launch_cli(["test.exe"], expect_exit=True)
+        result = launch_cli(["test.exe"], expect_returncode=1)
 
         assert "No Proton enabled Steam apps were found" in result
 
@@ -77,7 +77,7 @@ def test_run_executable_no_apps_from_desktop(
         Try running an EXE file when no Proton enabled Steam apps are installed
         or ready, and ensure an error dialog is opened using `gui_provider`.
         """
-        launch_cli(["--no-term", "test.exe"], expect_exit=True)
+        launch_cli(["--no-term", "test.exe"], expect_returncode=1)
 
         assert gui_provider.args[0] == "yad"
         assert gui_provider.args[1] == "--text-info"
@@ -136,7 +136,7 @@ def _mock_from_appmanifest(*args, **kwargs):
         )
 
         launch_cli(
-            ["--no-term", "--appid", "10", "test.exe"], expect_exit=True
+            ["--no-term", "--appid", "10", "test.exe"], expect_returncode=1
         )
 
         assert gui_provider.args[0] == "yad"

tests/cli/test_main.py

@@ -269,7 +269,7 @@ def test_run_winetricks_game_not_found(
         """
         Try running a Protontricks command for a non-existing app
         """
-        result = cli(["100", "winecfg"], expect_exit=True)
+        result = cli(["100", "winecfg"], expect_returncode=1)
 
         assert "Steam app with the given app ID could not be found" in result
 
@@ -282,6 +282,15 @@ def test_run_no_command(self, cli):
         # Help will be printed if no specific command is given
         assert result.startswith("usage: ")
 
+    @pytest.mark.usefixtures("default_proton")
+    def test_run_returncode_passed(self, cli, steam_app_factory):
+        """
+        Run a command that returns a specific exit code and ensure it is
+        returned
+        """
+        steam_app_factory(name="Fake game", appid=10)
+        cli(["-c", "exit 5", "10"], expect_returncode=5)
+
     def test_run_multiple_commands(self, cli):
         """
         Try performing multiple commands at once
@@ -296,7 +305,7 @@ def test_run_steam_not_found(self, cli, steam_dir):
         """
         shutil.rmtree(str(steam_dir))
 
-        result = cli(["10", "winecfg"], expect_exit=True)
+        result = cli(["10", "winecfg"], expect_returncode=1)
 
         assert "Steam installation directory could not be found" in result
 
@@ -308,7 +317,7 @@ def test_run_winetricks_not_found(
         steam_app_factory(name="Fake game 1", appid=10)
         (home_dir / ".local" / "bin" / "winetricks").unlink()
 
-        result = cli(["10", "winecfg"], expect_exit=True)
+        result = cli(["10", "winecfg"], expect_returncode=1)
 
         assert "Winetricks isn't installed" in result
 
@@ -324,7 +333,7 @@ def test_run_winetricks_from_desktop(
         steam_app_factory(name="Fake game 1", appid=10)
         (home_dir / ".local" / "bin" / "winetricks").unlink()
 
-        cli(["--no-term", "10", "winecfg"], expect_exit=True)
+        cli(["--no-term", "10", "winecfg"], expect_returncode=1)
 
         assert gui_provider.args[0] == "yad"
         assert gui_provider.args[1] == "--text-info"
@@ -345,7 +354,7 @@ def test_run_gui_provider_not_found(self, cli, home_dir, steam_app_factory):
         (home_dir / ".local" / "bin" / "yad").unlink()
         (home_dir / ".local" / "bin" / "zenity").unlink()
 
-        result = cli(["--gui"], expect_exit=True)
+        result = cli(["--gui"], expect_returncode=1)
 
         assert "YAD or Zenity is not installed" in result
 
@@ -358,14 +367,14 @@ def test_run_steam_runtime_not_found(
         steam_app_factory(name="Fake game 1", appid=10)
         result = cli(
             ["10", "winecfg"], env={"STEAM_RUNTIME": "invalid/path"},
-            expect_exit=True
+            expect_returncode=1
         )
 
         assert "Steam Runtime was enabled but couldn't be found" in result
 
     def test_run_proton_not_found(self, cli, steam_dir, steam_app_factory):
         steam_app_factory(name="Fake game 1", appid=10)
-        result = cli(["10", "winecfg"], expect_exit=True)
+        result = cli(["10", "winecfg"], expect_returncode=1)
 
         assert "Proton installation could not be found" in result
 
@@ -386,7 +395,7 @@ def test_run_compat_tool_not_proton(
             name="Fake game", appid=10, compat_tool_name="Not Proton"
         )
 
-        result = cli(["10", "winecfg"], expect_exit=True)
+        result = cli(["10", "winecfg"], expect_returncode=1)
 
         assert "Proton installation could not be found" in result
 
@@ -474,7 +483,7 @@ def _mock_from_appmanifest(*args, **kwargs):
             _mock_from_appmanifest
         )
 
-        cli(["--no-term", "-s", "Fake"], expect_exit=True)
+        cli(["--no-term", "-s", "Fake"], expect_returncode=1)
 
         assert gui_provider.args[0] == "yad"
         assert gui_provider.args[1] == "--text-info"
@@ -524,7 +533,7 @@ def test_run_gui_no_games(self, cli, default_proton):
         """
         Try starting the GUI when no games are installed
         """
-        result = cli(["--gui"], expect_exit=True)
+        result = cli(["--gui"], expect_returncode=1)
 
         assert "Found no games" in result
 
@@ -673,7 +682,11 @@ def test_cli_error_help(cli):
     Ensure that the full help message is printed when an incorrect argument
     is provided
     """
-    _, stderr = cli(["--nothing"], expect_exit=True, include_stderr=True)
+    _, stderr = cli(
+        ["--nothing"],
+        expect_returncode=2,  # Returned for CLI syntax error
+        include_stderr=True
+    )
 
     # Usage message
     assert "[-h] [--verbose]" in stderr

tests/cli/test_util.py

@@ -24,7 +24,7 @@ def test_cli_error_handler_uncaught_exception(
     """
     steam_app_factory(name="Fake game", appid=10)
 
-    cli(["--no-term", "-s", "Fake"], expect_exit=True)
+    cli(["--no-term", "-s", "Fake"], expect_returncode=1)
 
     assert gui_provider.args[0] == "yad"
     assert gui_provider.args[1] == "--text-info"
@@ -47,7 +47,7 @@ def test_cli_error_handler_gui_provider_env(
 
     steam_app_factory(name="Fake game", appid=10)
 
-    cli(["--no-term", "-s", "Fake"], expect_exit=True)
+    cli(["--no-term", "-s", "Fake"], expect_returncode=1)
 
     message = gui_provider.kwargs["input"]
 

tests/conftest.py

@@ -587,7 +587,8 @@ def func(name, new_struct=False):
 
 
 class MockSubprocess:
-    def __init__(self, args=None, kwargs=None, mock_stdout=None, env=None):
+    def __init__(
+            self, args=None, kwargs=None, mock_stdout=None, env=None):
         self.args = args
         self.kwargs = kwargs
 
@@ -600,8 +601,9 @@ def __init__(self, args=None, kwargs=None, mock_stdout=None, env=None):
 
 
 class MockResult:
-    def __init__(self, stdout):
+    def __init__(self, stdout, returncode=0):
         self.stdout = stdout
+        self.returncode = returncode
 
 
 @pytest.fixture(scope="function")
@@ -630,7 +632,7 @@ def mock_subprocess_run(args, **kwargs):
     yield mock_gui_provider
 
 
-@pytest.fixture(scope="function", autouse=True)
+@pytest.fixture(scope="function")
 def command(monkeypatch):
     """
     Monkeypatch the subprocess.run to store the args and environment
@@ -666,12 +668,10 @@ def _run_cli(monkeypatch, capsys, cli_func):
     Run protontricks with the given arguments and environment variables
     and return the output
     """
-    def func(args, env=None, include_stderr=False, expect_exit=False):
+    def func(args, env=None, include_stderr=False, expect_returncode=0):
         if not env:
             env = {}
 
-        system_exit = False
-
         with monkeypatch.context() as monkeypatch_ctx:
             # Monkeypatch environments values for the duration
             # of the CLI call
@@ -680,16 +680,10 @@ def func(args, env=None, include_stderr=False, expect_exit=False):
 
             try:
                 cli_func(args)
-            except SystemExit:
-                if expect_exit:
-                    system_exit = True
-                else:
+            except SystemExit as exc:
+                if expect_returncode != exc.code:
                     raise
 
-        if expect_exit:
-            assert system_exit, \
-                "Expected command to exit, but command succeeded instead"
-
         stdout, stderr = capsys.readouterr()
         if include_stderr:
             return stdout, stderr

tests/test_gui.py

@@ -125,7 +125,7 @@ def test_select_game_no_choice(
                 steam_apps=steam_apps, steam_path=steam_dir
             )
 
-        assert exc.value.code == 0
+        assert exc.value.code == 1
 
     def test_select_game_broken_zenity(
             self, broken_zenity, monkeypatch, steam_app_factory, steam_dir):

tests/test_util.py

@@ -110,8 +110,8 @@ def test_unknown_steam_runtime_detected(
             proton_app=proton_app,
             steam_app=steam_app,
             command=["echo", "nothing"],
-            use_steam_runtime=True,
-            steam_runtime_path=steam_runtime_medic.install_path
+            shell=True,
+            use_steam_runtime=True
         )
 
         # Warning will be logged since Protontricks does not recognize