fix(cli): support binary files with @ notation

JohnVillalovos · JohnVillalovos · commit bab680246f53 · 2023-12-28T15:33:52.000-08:00
Support binary files being used in the CLI with arguments using the `@` notation. For example `--avatar @/path/to/avatar.png` Also explicity catch the common FileNotFoundError and PermissionError exceptions. Remove the bare exception handling. We would rather have the full traceback of any exceptions that we don't know about and add them later if needed. Closes: #2752
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -56,7 +56,7 @@ jobs:
       - name: Run tests
         env:
           TOXENV: ${{ matrix.python.toxenv }}
-        run: tox --skip-missing-interpreters false
+        run: tox --skip-missing-interpreters false -- -vv
 
   functional:
     runs-on: ubuntu-22.04
diff --git a/gitlab/cli.py b/gitlab/cli.py
@@ -1,6 +1,7 @@
 import argparse
 import functools
 import os
+import pathlib
 import re
 import sys
 import textwrap
@@ -299,11 +300,15 @@ def _parse_value(v: Any) -> Any:
     if isinstance(v, str) and v.startswith("@"):
         # If the user-provided value starts with @, we try to read the file
         # path provided after @ as the real value. Exit on any error.
+        filepath = pathlib.Path(v[1:]).expanduser().resolve()
         try:
-            with open(v[1:], encoding="utf-8") as f:
+            with open(filepath, encoding="utf-8") as f:
                 return f.read()
-        except Exception as e:
-            sys.stderr.write(f"{e}\n")
+        except UnicodeDecodeError:
+            with open(filepath, "rb") as f:
+                return f.read()
+        except (FileNotFoundError, PermissionError) as exc:
+            sys.stderr.write(f"{exc}\n")
             sys.exit(1)
 
     return v
diff --git a/tests/functional/cli/test_cli_v4.py b/tests/functional/cli/test_cli_v4.py
@@ -540,12 +540,15 @@ def test_update_application_settings(gitlab_cli):
     assert ret.success
 
 
-def test_create_project_with_values_from_file(gitlab_cli, tmpdir):
+def test_create_project_with_values_from_file(gitlab_cli, fixture_dir, tmpdir):
     name = "gitlab-project-from-file"
     description = "Multiline\n\nData\n"
     from_file = tmpdir.join(name)
     from_file.write(description)
     from_file_path = f"@{str(from_file)}"
+    avatar_file = fixture_dir / "avatar.png"
+    assert avatar_file.exists()
+    avatar_file_path = f"@{avatar_file}"
 
     cmd = [
         "-v",
@@ -555,6 +558,8 @@ def test_create_project_with_values_from_file(gitlab_cli, tmpdir):
         name,
         "--description",
         from_file_path,
+        "--avatar",
+        avatar_file_path,
     ]
     ret = gitlab_cli(cmd)
 
diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py
@@ -1,9 +1,10 @@
 import argparse
+import contextlib
 import io
 import os
+import re
 import sys
 import tempfile
-from contextlib import redirect_stderr  # noqa: H302
 from unittest import mock
 
 import pytest
@@ -62,7 +63,7 @@ def test_cls_to_gitlab_resource(class_name, expected_gitlab_resource):
 )
 def test_die(message, error, expected):
     fl = io.StringIO()
-    with redirect_stderr(fl):
+    with contextlib.redirect_stderr(fl):
         with pytest.raises(SystemExit) as test:
             cli.die(message, error)
     assert fl.getvalue() == expected
@@ -89,15 +90,28 @@ def test_parse_value():
     assert ret == "content"
     os.unlink(temp_path)
 
-    fl = io.StringIO()
-    with redirect_stderr(fl):
-        with pytest.raises(SystemExit) as exc:
-            cli._parse_value("@/thisfileprobablydoesntexist")
-        assert (
-            fl.getvalue() == "[Errno 2] No such file or directory:"
-            " '/thisfileprobablydoesntexist'\n"
-        )
-        assert exc.value.code == 1
+    # fl = io.StringIO()
+
+    # with contextlib.redirect_stderr(fl):
+    #     with pytest.raises(SystemExit) as exc:
+    #         cli._parse_value("@/thisfileprobablydoesntexist")
+    #     non_existent_file = os.path.abspath("/thisfileprobablydoesntexist")
+    #     assert (
+    #         fl.getvalue() == f"[Errno 2] No such file or directory:"
+    #         f" '{str(non_existent_file)}'\n"
+    #     )
+    #     assert exc.value.code == 1
+
+def test_parse_value_file(capsys):
+    with pytest.raises(SystemExit) as exc:
+        cli._parse_value("@/thisfileprobablydoesntexist")
+    non_existent_file = re.escape(str(os.path.abspath("/thisfileprobablydoesntexist")))
+    captured = capsys.readouterr()
+    assert (
+        captured.err == f"[Errno 2] No such file or directory:"
+        f" '{non_existent_file}'\n"
+    )
+    assert exc.value.code == 1
 
 
 def test_base_parser():
