Repository: callowayproject/bump-my-version (python)

Issue:
Passing invalid command line arguments does not result in non-zero exit code
* bump-my-version version: 0.19.0
* Python version: 3.9.13 / 3.9.18
* Operating System: Windows 11 Pro 10.0.22631 Build 22631 / Debian Bookworm 12.5

### Description

When invalid argument(s) are passed to the `bump-my-version` tool, it displays the usage message, but then exits with a zero status which can mask the problem in automation jobs relying on command exit code.

### What I Did

```
bump-my-version.exe --foo bump patch && echo Success
```

Results in

```
 Usage: bump-my-version [OPTIONS] COMMAND [ARGS]...

 Version bump your Python project.

╭─ Options ────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ --version      Show the version and exit.                                                                            │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
╭─ Commands ───────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ bump                   Change the version.                                                                           │
│ replace                Replace the version in files.                                                                 │
│ sample-config          Print a sample configuration file.                                                            │
│ show                   Show current configuration information.                                                       │
│ show-bump              Show the possible versions resulting from the bump subcommand.                                │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

Success
```


Pull request:
Fixed bad options not returning an error code
Fixes #153

Patch:
diff --git a/bumpversion/cli.py b/bumpversion/cli.py
index cd14f726..019b3646 100644
--- a/bumpversion/cli.py
+++ b/bumpversion/cli.py
@@ -24,8 +24,6 @@ logger = get_indented_logger(__name__)
 
 @click.group(
     context_settings={
-        "ignore_unknown_options": True,
-        "allow_interspersed_args": True,
         "help_option_names": ["-h", "--help"],
     },
     add_help_option=False,


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 1644fa91..a7fcd29b 100644
--- a/tests/conftest.py
+++ b/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()
diff --git a/tests/test_cli/test_base_cli.py b/tests/test_cli/test_base_cli.py
new file mode 100644
index 00000000..3258e243
--- /dev/null
+++ b/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" in result.output
diff --git a/tests/test_cli/test_bump.py b/tests/test_cli/test_bump.py
index e6df7a5c..1fc178ae 100644
--- a/tests/test_cli/test_bump.py
+++ b/tests/test_cli/test_bump.py
@@ -9,7 +9,7 @@ from pathlib import Path
 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 @@ from tests.conftest import inside_dir
 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 @@ class TestNoConfiguredFilesOption:
         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 @@ class TestNoConfiguredFilesOption:
         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,
diff --git a/tests/test_cli/test_replace.py b/tests/test_cli/test_replace.py
index 3c6c061b..7cb57c4f 100644
--- a/tests/test_cli/test_replace.py
+++ b/tests/test_cli/test_replace.py
@@ -4,7 +4,7 @@ import shutil
 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,7 +41,7 @@ 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"
@@ -49,7 +49,6 @@ class TestReplaceCLI:
             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,7 +65,7 @@ class TestReplaceCLI:
             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"
@@ -74,7 +73,6 @@ class TestReplaceCLI:
             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,7 +91,7 @@ class TestReplaceCLI:
     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"
@@ -101,7 +99,6 @@ class TestReplaceCLI:
             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 @@ class TestReplaceCLI:
             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 @@ class TestReplaceCLI:
             )
 
             # Act
-            runner: CliRunner = CliRunner()
             with inside_dir(tmp_path):
                 result: Result = runner.invoke(
                     cli.cli,
@@ -154,7 +150,7 @@ class TestReplaceCLI:
     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 @@ class TestReplaceCLI:
             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 @@ class TestReplaceCLI:
 
             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 @@ class TestReplaceCLI:
             )
 
             # Act
-            runner: CliRunner = CliRunner()
             with inside_dir(tmp_path):
                 result: Result = runner.invoke(
                     cli.cli,
@@ -241,7 +235,7 @@ class TestReplaceCLI:
     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 @@ class TestReplaceCLI:
             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 @@ class TestReplaceCLI:
             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 @@ class TestReplaceCLI:
             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,
diff --git a/tests/test_cli/test_show.py b/tests/test_cli/test_show.py
index 4074cefd..59ca042f 100644
--- a/tests/test_cli/test_show.py
+++ b/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,7 +43,7 @@ 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"
@@ -53,7 +51,6 @@ def test_show_no_args(tmp_path: Path, fixtures_path: Path):
     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"])
diff --git a/tests/test_cli/test_show_bump.py b/tests/test_cli/test_show_bump.py
index 97e8a752..e06121b2 100644
--- a/tests/test_cli/test_show_bump.py
+++ b/tests/test_cli/test_show_bump.py
@@ -3,18 +3,17 @@
 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_bump_uses_current_version(tmp_path: Path, fixtures_path: Path):
+def test_show_bump_uses_current_version(tmp_path: Path, fixtures_path: Path, runner):
     """The show_bump 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-bump"])
@@ -35,13 +34,12 @@ def test_show_bump_uses_current_version(tmp_path: Path, fixtures_path: Path):
     )
 
 
-def test_show_bump_uses_passed_version(tmp_path: Path, fixtures_path: Path):
+def test_show_bump_uses_passed_version(tmp_path: Path, fixtures_path: Path, runner):
     """The show_bump 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-bump", "1.2.3"])
diff --git a/tests/test_cli/test_version_display.py b/tests/test_cli/test_version_display.py
index 06bd5be7..904628e2 100644
--- a/tests/test_cli/test_version_display.py
+++ b/tests/test_cli/test_version_display.py
@@ -1,15 +1,14 @@
-from click.testing import CliRunner, Result
+from click.testing import Result
 
 from bumpversion import cli
 from bumpversion import __version__
 
 
-def test_version_displays_library_version():
+def test_version_displays_library_version(runner):
     """
     Arrange/Act: Run the `version` subcommand.
 
     Assert: The output matches the library version.
     """
-    runner: CliRunner = CliRunner()
     result: Result = runner.invoke(cli.cli, ["--version"])
     assert __version__ in result.output.strip(), "Version number should match library version."