Fixed bad options not returning an error code

Fixes #153

Author: coordt

bumpversion/cli.py

@@ -24,8 +24,6 @@
 
 @click.group(
     context_settings={
-        "ignore_unknown_options": True,
-        "allow_interspersed_args": True,
         "help_option_names": ["-h", "--help"],
     },
     add_help_option=False,

tests/conftest.py

@@ -2,6 +2,7 @@
 
 import subprocess
 from contextlib import contextmanager
+from click.testing import CliRunner
 from pathlib import Path
 from typing import Generator
 
@@ -62,3 +63,10 @@ def hg_repo(tmp_path: Path) -> Path:
     """Generate a simple temporary mercurial repo and return the path."""
     subprocess.run(["hg", "init"], cwd=tmp_path, check=True, capture_output=True)
     return tmp_path
+
+
+@pytest.fixture
+def runner() -> CliRunner:
+    """Return a CliRunner instance."""
+
+    return CliRunner()

tests/test_cli/test_base_cli.py

@@ -0,0 +1,13 @@
+"""Tests for the base CLI commands."""
+
+from bumpversion import cli
+
+
+class TestBadOptionRaisesError:
+    """Tests to ensure that bad options raise an error."""
+
+    def test_bad_option_raises_error(self, runner):
+        """Passing an invalid option should raise an error."""
+        result = runner.invoke(cli.cli, ["--bad-option", "bump", "--current-version", "1.0.0", "patch"])
+        assert result.exit_code != 0
+        assert "No such option: --bad-option" in result.output

tests/test_cli/test_bump.py

@@ -9,7 +9,7 @@
 import pytest
 from pytest import param
 
-from click.testing import CliRunner, Result
+from click.testing import Result
 
 from bumpversion import cli
 from tests.conftest import inside_dir
@@ -18,12 +18,11 @@
 class TestNoConfiguredFilesOption:
     """Tests around the behavior of the --no-configured-files option."""
 
-    def test_no_files_marked_to_modify(self, mocker, tmp_path: Path):
+    def test_no_files_marked_to_modify(self, mocker, tmp_path: Path, runner):
         """
         No files are sent to the `do_bump` function if the --no-configured-files option is used.
         """
         mocked_do_bump = mocker.patch("bumpversion.cli.do_bump")
-        runner: CliRunner = CliRunner()
         with inside_dir(tmp_path):
             result: Result = runner.invoke(
                 cli.cli, ["bump", "--current-version", "1.0.0", "--no-configured-files", "patch"]
@@ -38,10 +37,9 @@ def test_no_files_marked_to_modify(self, mocker, tmp_path: Path):
         passed_config = call_args[2]
         assert len(passed_config.files) == 0
 
-    def test_file_args_marked_to_modify(self, mocker, tmp_path: Path):
+    def test_file_args_marked_to_modify(self, mocker, tmp_path: Path, runner):
         """File paths marked in the command-line arguments are sent to the `do_bump` function."""
         mocked_do_bump = mocker.patch("bumpversion.cli.do_bump")
-        runner: CliRunner = CliRunner()
         with inside_dir(tmp_path):
             result: Result = runner.invoke(
                 cli.cli,
@@ -59,7 +57,7 @@ def test_file_args_marked_to_modify(self, mocker, tmp_path: Path):
         assert passed_config.files[0].filename == "do-this-file.txt"
 
 
-def test_bump_nested_regex(tmp_path: Path, fixtures_path: Path, caplog):
+def test_bump_nested_regex(tmp_path: Path, fixtures_path: Path, caplog, runner):
     """
     Arrange/Act: Run the `bump` subcommand with --no-configured-files.
 
@@ -71,7 +69,6 @@ def test_bump_nested_regex(tmp_path: Path, fixtures_path: Path, caplog):
     config_path = tmp_path / ".bumpversion.toml"
     config_path.write_text(content)
 
-    runner: CliRunner = CliRunner()
     with inside_dir(tmp_path):
         result: Result = runner.invoke(cli.cli, ["bump", "-vv", "patch"])
 
@@ -85,23 +82,21 @@ def test_bump_nested_regex(tmp_path: Path, fixtures_path: Path, caplog):
     assert cff_path.read_text() == f"cff-version: 1.2.0\ndate-released: {now}\n"
 
 
-def test_missing_explicit_config_file(tmp_path: Path):
+def test_missing_explicit_config_file(tmp_path: Path, runner):
     """The command-line processor should raise an exception if the config file is missing."""
     with inside_dir(tmp_path):
-        runner: CliRunner = CliRunner()
         with inside_dir(tmp_path):
             result: Result = runner.invoke(cli.cli, ["bump", "--config-file", "missing-file.cfg"])
         assert result.exit_code != 0
         assert "'missing-file.cfg' does not exist." in result.output
 
 
-def test_cli_options_override_config(tmp_path: Path, fixtures_path: Path, mocker):
+def test_cli_options_override_config(tmp_path: Path, fixtures_path: Path, mocker, runner):
     """The command-line processor should override the config file."""
     # Arrange
     config_path = tmp_path / "this_config.toml"
     fixture_toml = fixtures_path / "basic_cfg.toml"
     shutil.copy(fixture_toml, config_path)
-    runner: CliRunner = CliRunner()
     mocked_do_bump = mocker.patch("bumpversion.cli.do_bump")
 
     # Act
@@ -163,13 +158,12 @@ def test_cli_options_override_config(tmp_path: Path, fixtures_path: Path, mocker
         param("hg_repo", "hg", id="hg"),
     ],
 )
-def test_dirty_work_dir_raises_error(repo: str, scm_command: str, request):
+def test_dirty_work_dir_raises_error(repo: str, scm_command: str, request, runner):
     repo_path: Path = request.getfixturevalue(repo)
     with inside_dir(repo_path):
         # Arrange
         repo_path.joinpath("dirty2").write_text("i'm dirty! 1.1.1")
         subprocess.run([scm_command, "add", "dirty2"], check=True)
-        runner: CliRunner = CliRunner()
 
         # Act
         result: Result = runner.invoke(
@@ -181,7 +175,7 @@ def test_dirty_work_dir_raises_error(repo: str, scm_command: str, request):
     assert "working directory is not clean" in result.output
 
 
-def test_non_scm_operations_if_scm_not_installed(tmp_path: Path, monkeypatch):
+def test_non_scm_operations_if_scm_not_installed(tmp_path: Path, monkeypatch, runner):
     """Everything works except SCM commands if the SCM is unusable."""
     # Arrange
     monkeypatch.setenv("PATH", "")
@@ -190,8 +184,6 @@ def test_non_scm_operations_if_scm_not_installed(tmp_path: Path, monkeypatch):
         version_path = tmp_path / "VERSION"
         version_path.write_text("31.0.3")
 
-        runner: CliRunner = CliRunner()
-
         # Act
         runner.invoke(cli.cli, ["bump", "major", "--current-version", "31.0.3", "VERSION"])
 
@@ -206,7 +198,7 @@ def test_non_scm_operations_if_scm_not_installed(tmp_path: Path, monkeypatch):
         param("", id="missing_version_part"),
     ],
 )
-def test_detects_bad_or_missing_version_part(version_part: str, tmp_path: Path, monkeypatch):
+def test_detects_bad_or_missing_version_part(version_part: str, tmp_path: Path, monkeypatch, runner):
     """It properly detects bad or missing version part."""
     # Arrange
     monkeypatch.setenv("PATH", "")
@@ -215,7 +207,6 @@ def test_detects_bad_or_missing_version_part(version_part: str, tmp_path: Path,
         version_path = tmp_path / "VERSION"
         version_path.write_text("31.0.3")
 
-        runner: CliRunner = CliRunner()
         args = ["bump", "--current-version", "31.0.3"]
         if version_part:
             args.append(version_part)
@@ -228,7 +219,7 @@ def test_detects_bad_or_missing_version_part(version_part: str, tmp_path: Path,
     assert "Unknown version part:" in result.stdout
 
 
-def test_ignores_missing_files_with_option(tmp_path, fixtures_path):
+def test_ignores_missing_files_with_option(tmp_path, fixtures_path, runner):
     """The replace subcommand should ignore missing."""
 
     config_file = tmp_path / ".bumpversion.toml"
@@ -242,7 +233,6 @@ def test_ignores_missing_files_with_option(tmp_path, fixtures_path):
     )
 
     # Act
-    runner: CliRunner = CliRunner()
     with inside_dir(tmp_path):
         result: Result = runner.invoke(
             cli.cli,

tests/test_cli/test_replace.py

@@ -4,7 +4,7 @@
 import traceback
 from pathlib import Path
 
-from click.testing import CliRunner, Result
+from click.testing import Result
 
 from bumpversion import cli
 from tests.conftest import inside_dir
@@ -41,15 +41,14 @@ class TestReplaceCLI:
     class TestDefaultReplacesVersion:
         """Test the default behavior of the replace subcommand."""
 
-        def test_default_affects_all_configured_files(self, mocker, tmp_path, fixtures_path):
+        def test_default_affects_all_configured_files(self, mocker, tmp_path, fixtures_path, runner):
             """The replace subcommand should replace the version in all configured files."""
             # Arrange
             toml_path = fixtures_path / "basic_cfg.toml"
             config_path = tmp_path / "pyproject.toml"
             shutil.copy(toml_path, config_path)
 
             mocked_modify_files = mocker.patch("bumpversion.cli.modify_files")
-            runner: CliRunner = CliRunner()
             with inside_dir(tmp_path):
                 result: Result = runner.invoke(cli.cli, ["replace", "--new-version", "1.1.0"])
 
@@ -66,15 +65,14 @@ def test_default_affects_all_configured_files(self, mocker, tmp_path, fixtures_p
             actual_filenames = {f.file_change.filename for f in configured_files}
             assert actual_filenames == {"setup.py", "CHANGELOG.md", "bumpversion/__init__.py"}
 
-        def test_can_limit_to_specific_files(self, mocker, git_repo, fixtures_path):
+        def test_can_limit_to_specific_files(self, mocker, git_repo, fixtures_path, runner):
             """The replace subcommand should set the files to only the specified files."""
             # Arrange
             toml_path = fixtures_path / "basic_cfg.toml"
             config_path = git_repo / "pyproject.toml"
             shutil.copy(toml_path, config_path)
 
             mocked_modify_files = mocker.patch("bumpversion.cli.modify_files")
-            runner: CliRunner = CliRunner()
             with inside_dir(git_repo):
                 result: Result = runner.invoke(cli.cli, ["replace", "--no-configured-files", "VERSION"])
 
@@ -93,15 +91,14 @@ def test_can_limit_to_specific_files(self, mocker, git_repo, fixtures_path):
     class TestOptions:
         """Test the options of the replace subcommand."""
 
-        def test_missing_newversion_is_set_to_none(self, mocker, tmp_path, fixtures_path):
+        def test_missing_newversion_is_set_to_none(self, mocker, tmp_path, fixtures_path, runner):
             """The replace subcommand should set new_version to None in the context."""
             # Arrange
             toml_path = fixtures_path / "basic_cfg.toml"
             config_path = tmp_path / "pyproject.toml"
             shutil.copy(toml_path, config_path)
 
             mocked_modify_files = mocker.patch("bumpversion.cli.modify_files")
-            runner: CliRunner = CliRunner()
             with inside_dir(tmp_path):
                 result: Result = runner.invoke(cli.cli, ["replace"])
 
@@ -115,7 +112,7 @@ def test_missing_newversion_is_set_to_none(self, mocker, tmp_path, fixtures_path
             call_args = mocked_modify_files.call_args[0]
             assert call_args[2] is None
 
-        def test_ignores_missing_files_with_option(self, mocker, tmp_path, fixtures_path):
+        def test_ignores_missing_files_with_option(self, mocker, tmp_path, fixtures_path, runner):
             """The replace subcommand should ignore missing."""
 
             config_file = tmp_path / ".bumpversion.toml"
@@ -129,7 +126,6 @@ def test_ignores_missing_files_with_option(self, mocker, tmp_path, fixtures_path
             )
 
             # Act
-            runner: CliRunner = CliRunner()
             with inside_dir(tmp_path):
                 result: Result = runner.invoke(
                     cli.cli,
@@ -154,7 +150,7 @@ def test_ignores_missing_files_with_option(self, mocker, tmp_path, fixtures_path
     class TestSearchInputs:
         """Test the search inputs of the replace subcommand."""
 
-        def test_accepts_plain_string(self, tmp_path, fixtures_path):
+        def test_accepts_plain_string(self, tmp_path, fixtures_path, runner):
             """Replace should not worry if the search values have version info."""
             from tomlkit import dumps
 
@@ -164,7 +160,6 @@ def test_accepts_plain_string(self, tmp_path, fixtures_path):
             doc_path = tmp_path / "docs.yaml"
             doc_path.write_text("url: https://github.com/sampleuser/workflows/main/.github/update_mailmap.py")
 
-            runner: CliRunner = CliRunner()
             with inside_dir(tmp_path):
                 result: Result = runner.invoke(
                     cli.cli,
@@ -186,7 +181,7 @@ def test_accepts_plain_string(self, tmp_path, fixtures_path):
 
             assert result.exit_code == 0
 
-        def test_unintentional_valid_regex_still_found(self, tmp_path: Path, caplog) -> None:
+        def test_unintentional_valid_regex_still_found(self, tmp_path: Path, caplog, runner) -> None:
             """A search string not meant to be a regex (but is) is still found and replaced correctly."""
             # Arrange
             search = "(unreleased)"
@@ -209,7 +204,6 @@ def test_unintentional_valid_regex_still_found(self, tmp_path: Path, caplog) ->
             )
 
             # Act
-            runner: CliRunner = CliRunner()
             with inside_dir(tmp_path):
                 result: Result = runner.invoke(
                     cli.cli,
@@ -241,7 +235,7 @@ def test_unintentional_valid_regex_still_found(self, tmp_path: Path, caplog) ->
     class TestReplaceInputs:
         """Test the replace inputs of the replace subcommand."""
 
-        def test_accepts_empty_string(self, tmp_path, fixtures_path):
+        def test_accepts_empty_string(self, tmp_path, fixtures_path, runner):
             """Replace should be able to replace strings with an empty string."""
             from tomlkit import dumps
 
@@ -251,7 +245,6 @@ def test_accepts_empty_string(self, tmp_path, fixtures_path):
             doc_path = tmp_path / "docs.yaml"
             doc_path.write_text("We should censor profanity\n\n")
 
-            runner: CliRunner = CliRunner()
             with inside_dir(tmp_path):
                 result: Result = runner.invoke(
                     cli.cli,
@@ -275,7 +268,7 @@ def test_accepts_empty_string(self, tmp_path, fixtures_path):
             assert result.exit_code == 0
             assert doc_path.read_text() == "We should censor \n\n"
 
-        def test_accepts_plain_string(self, tmp_path, fixtures_path):
+        def test_accepts_plain_string(self, tmp_path, fixtures_path, runner):
             """Replace should not worry if the replace values have version info."""
             from tomlkit import dumps
 
@@ -285,7 +278,6 @@ def test_accepts_plain_string(self, tmp_path, fixtures_path):
             doc_path = tmp_path / "docs.yaml"
             doc_path.write_text("url: https://github.com/sampleuser/workflows/v2.17.7/.github/update_mailmap.py")
 
-            runner: CliRunner = CliRunner()
             with inside_dir(tmp_path):
                 result: Result = runner.invoke(
                     cli.cli,

tests/test_cli/test_show.py

@@ -1,19 +1,18 @@
 import shutil
 from pathlib import Path
 
-from click.testing import CliRunner, Result
+from click.testing import Result
 
 from bumpversion import cli
 from tests.conftest import inside_dir
 
 
-def test_show(tmp_path: Path, fixtures_path: Path):
+def test_show(tmp_path: Path, fixtures_path: Path, runner):
     """The show subcommand should list the parts of the configuration."""
     # Arrange
     config_path = tmp_path / "pyproject.toml"
     toml_path = fixtures_path / "basic_cfg.toml"
     shutil.copy(toml_path, config_path)
-    runner: CliRunner = CliRunner()
 
     with inside_dir(tmp_path):
         result: Result = runner.invoke(cli.cli, ["show", "current_version"])
@@ -26,13 +25,12 @@ def test_show(tmp_path: Path, fixtures_path: Path):
     assert result.output.splitlines(keepends=False) == ["1.0.0"]
 
 
-def test_show_and_increment(tmp_path: Path, fixtures_path: Path):
+def test_show_and_increment(tmp_path: Path, fixtures_path: Path, runner):
     """The show subcommand should incrment the version and display it."""
     # Arrange
     config_path = tmp_path / "pyproject.toml"
     toml_path = fixtures_path / "basic_cfg.toml"
     shutil.copy(toml_path, config_path)
-    runner: CliRunner = CliRunner()
 
     with inside_dir(tmp_path):
         result: Result = runner.invoke(cli.cli, ["show", "new_version", "--increment", "minor"])
@@ -45,15 +43,14 @@ def test_show_and_increment(tmp_path: Path, fixtures_path: Path):
     assert result.output.splitlines(keepends=False) == ["1.1.0-dev"]
 
 
-def test_show_no_args(tmp_path: Path, fixtures_path: Path):
+def test_show_no_args(tmp_path: Path, fixtures_path: Path, runner):
     """The show subcommand should list the entire configuration."""
     # Arrange
     config_path = tmp_path / "pyproject.toml"
     toml_path = fixtures_path / "basic_cfg.toml"
     expected_output = fixtures_path.joinpath("basic_cfg_expected.yaml").read_text()
 
     shutil.copy(toml_path, config_path)
-    runner: CliRunner = CliRunner()
 
     with inside_dir(tmp_path):
         result: Result = runner.invoke(cli.cli, ["show", "--format", "yaml"])