Skip to content

Commit

Permalink
convert: Present exceptions more nicely
Browse files Browse the repository at this point in the history
Wrap them in ClickException, so tracebacks aren’t shown to users.

Fixes: #172

Signed-off-by: Nils Philippsen <nils@redhat.com>
  • Loading branch information
nphilipp committed Aug 30, 2024
1 parent 2d68a29 commit 0e9cf55
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 18 deletions.
27 changes: 20 additions & 7 deletions rpmautospec/subcommands/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,17 +254,30 @@ def convert(
) -> None:
"""Convert a package repository to use rpmautospec"""
if commit and message == "":
raise ValueError("Commit message cannot be empty")
raise click.UsageError("Commit message cannot be empty")

if not changelog and not release:
raise ValueError("All changes are disabled")
raise click.UsageError("All changes are disabled")

try:
pkg = PkgConverter(spec_or_path)
except (
ValueError,
FileNotFoundError,
SpecialFileError,
FileUntrackedError,
FileModifiedError,
) as exc:
raise click.ClickException(*exc.args) from exc

pkg = PkgConverter(spec_or_path)
pkg.load()
if changelog:
pkg.convert_to_autochangelog()
if release:
pkg.convert_to_autorelease()
try:
if changelog:
pkg.convert_to_autochangelog()
if release:
pkg.convert_to_autorelease()
except SpecParseFailure as exc:
raise click.ClickException(*exc.args) from exc
pkg.save()
if commit:
pkg.commit(message=message, signoff=signoff)
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,4 +208,4 @@ def repo(repopath, specfile, specfile_content, _repo_config):

@pytest.fixture
def cli_runner() -> CliRunner:
return CliRunner()
return CliRunner(mix_stderr=False)
47 changes: 37 additions & 10 deletions tests/rpmautospec/subcommands/test_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,20 +158,22 @@ def test_commit(self, with_message, signoff, release, changelog, specfile, repo,

@mock.patch.object(convert, "PkgConverter")
def test_convert_empty_commit_message(PkgConverter, cli_runner, specfile):
with pytest.raises(ValueError, match="Commit message cannot be empty"):
cli_runner.invoke(
convert.convert, ["--commit", "--message=", str(specfile)], catch_exceptions=False
)
# with pytest.raises(click.UsageError, match="Commit message cannot be empty"):
result = cli_runner.invoke(
convert.convert, ["--commit", "--message=", str(specfile)], catch_exceptions=False
)
assert result.exit_code != 0
assert "Error: Commit message cannot be empty" in result.stderr


@mock.patch.object(convert, "PkgConverter")
def test_convert_no_changes(PkgConverter, cli_runner, specfile):
with pytest.raises(ValueError, match="All changes are disabled"):
cli_runner.invoke(
convert.convert,
["--no-changelog", "--no-release", str(specfile)],
catch_exceptions=False,
)
# with pytest.raises(ValueError, match="All changes are disabled"):
result = cli_runner.invoke(
convert.convert, ["--no-changelog", "--no-release", str(specfile)], catch_exceptions=False
)
assert result.exit_code != 0
assert "Error: All changes are disabled" in result.stderr


@mock.patch("rpmautospec.subcommands.convert.PkgConverter")
Expand Down Expand Up @@ -389,3 +391,28 @@ def test_commit(specfile, release, changelog, repo):
assert head.message == expected_message
assert specfile.name in fileschanged
assert ("changelog" in fileschanged) == changelog_should_change


@pytest.mark.parametrize(
"method, exception",
(
("__init__", ValueError),
("__init__", FileNotFoundError),
("__init__", SpecialFileError),
("__init__", convert.FileUntrackedError),
("__init__", convert.FileModifiedError),
("convert_to_autorelease", SpecParseFailure),
("convert_to_autochangelog", SpecParseFailure),
),
)
@mock.patch.object(convert, "PkgConverter")
def test_rewrap_exceptions(PkgConverter, method, exception, cli_runner, specfile):
if method == "__init__":
PkgConverter.side_effect = exception("BOOP")
else:
obj = PkgConverter.return_value
getattr(obj, method).side_effect = exception("BOOP")

result = cli_runner.invoke(convert.convert, str(specfile))
assert result.exit_code != 0
assert "Error: BOOP" in result.stderr

0 comments on commit 0e9cf55

Please sign in to comment.