NotificationsYou must be signed in to change notification settings
Fork674
Star2.4k

Commitbab6802

committed

fix(cli): support binary files with@ notation

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 PermissionErrorexceptions.Remove the bare exception handling. We would rather have the fulltraceback of any exceptions that we don't know about and add themlater if needed.Closes:#2752

1 parentb8824a6 commitbab6802Copy full SHA for bab6802

File tree

4 files changed

+40

-16

lines changed

.github/workflows
- test.yml
gitlab
- cli.py
tests
- functional/cli
  - test_cli_v4.py
- unit
  - test_cli.py

4 files changed

+40

-16

lines changed

`‎.github/workflows/test.yml‎`

Lines changed: 1 addition & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ jobs:`
`56`	`56`	`-name:Run tests`
`57`	`57`	`env:`
`58`	`58`	`TOXENV:${{ matrix.python.toxenv }}`
`59`		`-run:tox --skip-missing-interpreters false`
	`59`	`+run:tox --skip-missing-interpreters false -- -vv`
`60`	`60`
`61`	`61`	`functional:`
`62`	`62`	`runs-on:ubuntu-22.04`

`‎gitlab/cli.py‎`

Lines changed: 8 additions & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`importargparse`
`2`	`2`	`importfunctools`
`3`	`3`	`importos`
	`4`	`+importpathlib`
`4`	`5`	`importre`
`5`	`6`	`importsys`
`6`	`7`	`importtextwrap`
`@@ -299,11 +300,15 @@ def _parse_value(v: Any) -> Any:`
`299`	`300`	`ifisinstance(v,str)andv.startswith("@"):`
`300`	`301`	`# If the user-provided value starts with @, we try to read the file`
`301`	`302`	`# path provided after @ as the real value. Exit on any error.`
	`303`	`+filepath=pathlib.Path(v[1:]).expanduser().resolve()`
`302`	`304`	`try:`
`303`		`-withopen(v[1:],encoding="utf-8")asf:`
	`305`	`+withopen(filepath,encoding="utf-8")asf:`
`304`	`306`	`returnf.read()`
`305`		`-exceptExceptionase:`
`306`		`-sys.stderr.write(f"{e}\n")`
	`307`	`+exceptUnicodeDecodeError:`
	`308`	`+withopen(filepath,"rb")asf:`
	`309`	`+returnf.read()`
	`310`	`+except (FileNotFoundError,PermissionError)asexc:`
	`311`	`+sys.stderr.write(f"{exc}\n")`
`307`	`312`	`sys.exit(1)`
`308`	`313`
`309`	`314`	`returnv`

`‎tests/functional/cli/test_cli_v4.py‎`

Lines changed: 6 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -540,12 +540,15 @@ def test_update_application_settings(gitlab_cli):`
`540`	`540`	`assertret.success`
`541`	`541`
`542`	`542`
`543`		`-deftest_create_project_with_values_from_file(gitlab_cli,tmpdir):`
	`543`	`+deftest_create_project_with_values_from_file(gitlab_cli,fixture_dir,tmpdir):`
`544`	`544`	`name="gitlab-project-from-file"`
`545`	`545`	`description="Multiline\n\nData\n"`
`546`	`546`	`from_file=tmpdir.join(name)`
`547`	`547`	`from_file.write(description)`
`548`	`548`	`from_file_path=f"@{str(from_file)}"`
	`549`	`+avatar_file=fixture_dir/"avatar.png"`
	`550`	`+assertavatar_file.exists()`
	`551`	`+avatar_file_path=f"@{avatar_file}"`
`549`	`552`
`550`	`553`	`cmd= [`
`551`	`554`	`"-v",`
`@@ -555,6 +558,8 @@ def test_create_project_with_values_from_file(gitlab_cli, tmpdir):`
`555`	`558`	`name,`
`556`	`559`	`"--description",`
`557`	`560`	`from_file_path,`
	`561`	`+"--avatar",`
	`562`	`+avatar_file_path,`
`558`	`563`	`]`
`559`	`564`	`ret=gitlab_cli(cmd)`
`560`	`565`

`‎tests/unit/test_cli.py‎`

Lines changed: 25 additions & 11 deletions

Original file line number	Diff line number	Diff line change
`@@ -1,9 +1,10 @@`
`1`	`1`	`importargparse`
	`2`	`+importcontextlib`
`2`	`3`	`importio`
`3`	`4`	`importos`
	`5`	`+importre`
`4`	`6`	`importsys`
`5`	`7`	`importtempfile`
`6`		`-fromcontextlibimportredirect_stderr# noqa: H302`
`7`	`8`	`fromunittestimportmock`
`8`	`9`
`9`	`10`	`importpytest`
`@@ -62,7 +63,7 @@ def test_cls_to_gitlab_resource(class_name, expected_gitlab_resource):`
`62`	`63`	`)`
`63`	`64`	`deftest_die(message,error,expected):`
`64`	`65`	`fl=io.StringIO()`
`65`		`-withredirect_stderr(fl):`
	`66`	`+withcontextlib.redirect_stderr(fl):`
`66`	`67`	`withpytest.raises(SystemExit)astest:`
`67`	`68`	`cli.die(message,error)`
`68`	`69`	`assertfl.getvalue()==expected`
`@@ -89,15 +90,28 @@ def test_parse_value():`
`89`	`90`	`assertret=="content"`
`90`	`91`	`os.unlink(temp_path)`
`91`	`92`
`92`		`-fl=io.StringIO()`
`93`		`-withredirect_stderr(fl):`
`94`		`-withpytest.raises(SystemExit)asexc:`
`95`		`-cli._parse_value("@/thisfileprobablydoesntexist")`
`96`		`-assert (`
`97`		`-fl.getvalue()=="[Errno 2] No such file or directory:"`
`98`		`-" '/thisfileprobablydoesntexist'\n"`
`99`		`- )`
`100`		`-assertexc.value.code==1`
	`93`	`+# fl = io.StringIO()`
	`94`	`+`
	`95`	`+# with contextlib.redirect_stderr(fl):`
	`96`	`+# with pytest.raises(SystemExit) as exc:`
	`97`	`+# cli._parse_value("@/thisfileprobablydoesntexist")`
	`98`	`+# non_existent_file = os.path.abspath("/thisfileprobablydoesntexist")`
	`99`	`+# assert (`
	`100`	`+# fl.getvalue() == f"[Errno 2] No such file or directory:"`
	`101`	`+# f" '{str(non_existent_file)}'\n"`
	`102`	`+# )`
	`103`	`+# assert exc.value.code == 1`
	`104`	`+`
	`105`	`+deftest_parse_value_file(capsys):`
	`106`	`+withpytest.raises(SystemExit)asexc:`
	`107`	`+cli._parse_value("@/thisfileprobablydoesntexist")`
	`108`	`+non_existent_file=re.escape(str(os.path.abspath("/thisfileprobablydoesntexist")))`
	`109`	`+captured=capsys.readouterr()`
	`110`	`+assert (`
	`111`	`+captured.err==f"[Errno 2] No such file or directory:"`
	`112`	`+f" '{non_existent_file}'\n"`
	`113`	`+ )`
	`114`	`+assertexc.value.code==1`
`101`	`115`
`102`	`116`
`103`	`117`	`deftest_base_parser():`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commitbab6802

File tree

4 files changed

4 files changed

`‎.github/workflows/test.yml‎`

`‎gitlab/cli.py‎`

`‎tests/functional/cli/test_cli_v4.py‎`

`‎tests/unit/test_cli.py‎`

0 commit comments