CLI: Don’t crash on spec file parsing errors

rpmautospec/subcommands/changelog.py

@@ -39,7 +39,10 @@ def do_generate_changelog(
 @handle_expected_exceptions
 def generate_changelog(obj: dict[str, Any], spec_or_path: Path) -> None:
     """Generate changelog entries from git commit logs"""
-    changelog = do_generate_changelog(
-        spec_or_path, error_on_unparseable_spec=obj["error_on_unparseable_spec"]
-    )
+    try:
+        changelog = do_generate_changelog(
+            spec_or_path, error_on_unparseable_spec=obj["error_on_unparseable_spec"]
+        )
+    except SpecParseFailure as exc:
+        raise click.ClickException(*exc.args) from exc
     pager.page(changelog, enabled=obj["pager"])

rpmautospec/subcommands/process_distgit.py

@@ -151,6 +151,9 @@ def do_process_distgit(
 @handle_expected_exceptions
 def process_distgit(obj: dict[str, Any], spec_or_path: Path, target: Path) -> None:
     """Work repository history and commit logs into a spec file"""
-    do_process_distgit(
-        spec_or_path, target, error_on_unparseable_spec=obj["error_on_unparseable_spec"]
-    )
+    try:
+        do_process_distgit(
+            spec_or_path, target, error_on_unparseable_spec=obj["error_on_unparseable_spec"]
+        )
+    except SpecParseFailure as exc:
+        raise click.ClickException(*exc.args) from exc

rpmautospec/subcommands/release.py

@@ -68,9 +68,12 @@ def do_calculate_release_number(
 @handle_expected_exceptions
 def calculate_release(obj: dict[str, Any], complete_release: bool, spec_or_path: Path) -> None:
     """Calculate the next release tag for a package build"""
-    release = do_calculate_release(
-        spec_or_path,
-        complete_release=complete_release,
-        error_on_unparseable_spec=obj["error_on_unparseable_spec"],
-    )
+    try:
+        release = do_calculate_release(
+            spec_or_path,
+            complete_release=complete_release,
+            error_on_unparseable_spec=obj["error_on_unparseable_spec"],
+        )
+    except SpecParseFailure as exc:
+        raise click.ClickException(*exc.args) from exc
     print("Calculated release number:", release)

tests/rpmautospec/subcommands/test_changelog.py

@@ -68,13 +68,12 @@ def test_do_generate_changelog_error():
     assert str(excinfo.value) == "Couldn’t parse spec file test.spec:\nBOOP"
 
 
-def test_generate_changelog(cli_runner):
+@pytest.mark.parametrize("testcase", ("success", "specfile-parse-failure"))
+def test_generate_changelog(testcase, cli_runner):
     with (
         mock.patch.object(changelog, "do_generate_changelog") as do_generate_changelog,
         mock.patch.object(changelog, "pager") as pager,
     ):
-        generated_changelog = do_generate_changelog.return_value
-
         pager_sentinel = object()
         error_on_unparseable_spec_sentinel = object()
 
@@ -83,11 +82,23 @@ def test_generate_changelog(cli_runner):
             "error_on_unparseable_spec": error_on_unparseable_spec_sentinel,
         }
 
-        result = cli_runner.invoke(changelog.generate_changelog, ["some_path"], obj=ctx_obj)
+        if "specfile-parse-failure" in testcase:
+            do_generate_changelog.side_effect = changelog.SpecParseFailure("BOO")
 
-        assert result.exit_code == 0
+        result = cli_runner.invoke(changelog.generate_changelog, ["some_path"], obj=ctx_obj)
 
         do_generate_changelog.assert_called_once_with(
             "some_path", error_on_unparseable_spec=error_on_unparseable_spec_sentinel
         )
-        pager.page.assert_called_once_with(generated_changelog, enabled=pager_sentinel)
+
+        if "success" in testcase:
+            assert result.exit_code == 0
+
+            pager.page.assert_called_once_with(
+                do_generate_changelog.return_value, enabled=pager_sentinel
+            )
+        else:
+            assert result.exit_code != 0
+            assert "Error: BOO" in result.stderr
+
+            pager.page.assert_not_called()

tests/rpmautospec/subcommands/test_convert.py

@@ -158,7 +158,6 @@ 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(click.UsageError, match="Commit message cannot be empty"):
     result = cli_runner.invoke(
         convert.convert, ["--commit", "--message=", str(specfile)], catch_exceptions=False
     )
@@ -168,7 +167,6 @@ def test_convert_empty_commit_message(PkgConverter, cli_runner, specfile):
 
 @mock.patch.object(convert, "PkgConverter")
 def test_convert_no_changes(PkgConverter, cli_runner, specfile):
-    # with pytest.raises(ValueError, match="All changes are disabled"):
     result = cli_runner.invoke(
         convert.convert, ["--no-changelog", "--no-release", str(specfile)], catch_exceptions=False
     )

tests/rpmautospec/subcommands/test_process_distgit.py

@@ -485,9 +485,15 @@ def wrap_cls(*args, **kwargs):
 
 
 @pytest.mark.parametrize(
-    "override_locale", (False, "C", "de_DE.UTF-8"), ids=("locale-unset", "locale-C", "locale-de")
+    "testcase", ("locale-unset", "locale-C", "locale-de", "specfile-parse-failure")
 )
-def test_process_distgit(override_locale, tmp_path, locale, cli_runner):
+def test_process_distgit(testcase, tmp_path, locale, cli_runner):
+    override_locale = False
+    if "locale-C" in testcase:
+        override_locale = "C"
+    elif "locale-de" in testcase:
+        override_locale = "de_DE.UTF-8"
+
     output_spec_file = tmp_path / "test.spec"
     unpacked_repo_dir, test_spec_file_path = gen_testrepo(tmp_path, "rawhide")
 
@@ -497,13 +503,24 @@ def test_process_distgit(override_locale, tmp_path, locale, cli_runner):
     if override_locale:
         locale.setlocale(locale.LC_ALL, override_locale)
 
-    with mock.patch.object(
-        process_distgit, "do_process_distgit", wraps=process_distgit.do_process_distgit
-    ) as do_process_distgit_fn:
-        cli_runner.invoke(process_distgit.process_distgit, args, obj=ctx_obj)
+    with (
+        mock.patch.object(
+            process_distgit, "do_process_distgit", wraps=process_distgit.do_process_distgit
+        ) as do_process_distgit_fn,
+        mock.patch("rpm.setLogFile"),  # rpm can’t cope with fake sys.stderr
+    ):
+        if "specfile-parse-failure" in testcase:
+            do_process_distgit_fn.side_effect = process_distgit.SpecParseFailure("BOO")
+        result = cli_runner.invoke(process_distgit.process_distgit, args, obj=ctx_obj)
 
     do_process_distgit_fn.assert_called_once_with(
         str(test_spec_file_path),
         str(output_spec_file),
         error_on_unparseable_spec=ctx_obj["error_on_unparseable_spec"],
     )
+
+    if "specfile-parse-failure" in testcase:
+        assert result.exit_code != 0
+        assert "Error: BOO" in result.stderr
+    else:
+        assert result.exit_code == 0

tests/rpmautospec/subcommands/test_release.py

@@ -63,6 +63,23 @@ def test_calculate_release(self, method_to_test, cli_runner):
 
                     assert f"Calculated release number: {expected_release}" in result.stdout
 
+    def test_calculate_release_specfile_parse_failure(self, cli_runner):
+        error_on_unparseable_spec_sentinel = object()
+        ctx_obj = {"error_on_unparseable_spec": error_on_unparseable_spec_sentinel}
+
+        with mock.patch.object(release, "do_calculate_release") as do_calculate_release:
+            do_calculate_release.side_effect = release.SpecParseFailure("BOO")
+            result = cli_runner.invoke(release.calculate_release, ["some_path"], obj=ctx_obj)
+
+        do_calculate_release.assert_called_once_with(
+            "some_path",
+            complete_release=True,
+            error_on_unparseable_spec=error_on_unparseable_spec_sentinel,
+        )
+
+        assert result.exit_code != 0
+        assert "Error: BOO" in result.stderr
+
     def test_do_calculate_release_error(self, cli_runner):
         with mock.patch.object(release, "PkgHistoryProcessor") as PkgHistoryProcessor:
             processor = PkgHistoryProcessor.return_value