Skip to content

Commit 1985e54

Browse files
fix: address review comments on version command PR
- Parametrize invalid combination tests (major/minor/patch/tag without project) - Fix version scheme validation to use hasattr check instead of broken isinstance/issubclass on Protocol with non-method members - Format test file with ruff Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2913d60 commit 1985e54

2 files changed

Lines changed: 32 additions & 29 deletions

File tree

commitizen/version_schemes.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -422,11 +422,16 @@ def get_version_scheme(settings: Settings, name: str | None = None) -> VersionSc
422422
raise VersionSchemeUnknown(f'Version scheme "{name}" unknown.')
423423
scheme = cast("VersionScheme", ep.load())
424424

425-
# `VersionProtocol` is a `@runtime_checkable` Protocol with properties, so
426-
# `issubclass(scheme, VersionProtocol)` is not supported. The historical
427-
# `isinstance(scheme, VersionProtocol)` check is only meaningful for instances;
428-
# for loaded classes it is effectively a no-op for valid schemes.
429-
if not isinstance(scheme, VersionProtocol):
425+
# `VersionProtocol` is a `@runtime_checkable` Protocol, but `issubclass()` is not
426+
# supported for Protocols with non-method members. We check an instance instead by
427+
# verifying the loaded object is a class with the expected interface.
428+
if isinstance(scheme, type):
429+
# Check for a key method/attribute that VersionProtocol requires
430+
if not hasattr(scheme, "bump"):
431+
warnings.warn(
432+
f"Version scheme {name} does not implement the VersionProtocol"
433+
)
434+
else:
430435
warnings.warn(f"Version scheme {name} does not implement the VersionProtocol")
431436

432437
return scheme

tests/commands/test_version_command.py

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -147,19 +147,30 @@ def test_version_just_minor(config, capsys, version: str, expected_version: str)
147147
assert expected_version == captured.out
148148

149149

150-
@pytest.mark.parametrize("argument", ["major", "minor", "patch"])
151-
def test_version_just_major_error_no_project(config, capsys, argument: str):
152-
commands.Version(
153-
config,
154-
{
155-
argument: True, # type: ignore[misc]
156-
},
157-
)()
150+
@pytest.mark.parametrize(
151+
("args", "expected_error"),
152+
[
153+
(
154+
{"major": True},
155+
"can only be used with MANUAL_VERSION, --project or --verbose.",
156+
),
157+
(
158+
{"minor": True},
159+
"can only be used with MANUAL_VERSION, --project or --verbose.",
160+
),
161+
(
162+
{"patch": True},
163+
"can only be used with MANUAL_VERSION, --project or --verbose.",
164+
),
165+
({"tag": True}, "Tag can only be used with --project or --verbose."),
166+
],
167+
)
168+
def test_version_invalid_combinations(config, capsys, args: dict, expected_error: str):
169+
"""Test that certain flag combinations produce errors."""
170+
commands.Version(config, args)() # type: ignore[arg-type]
158171
captured = capsys.readouterr()
159172
assert not captured.out
160-
assert (
161-
"can only be used with MANUAL_VERSION, --project or --verbose." in captured.err
162-
)
173+
assert expected_error in captured.err
163174

164175

165176
@pytest.mark.parametrize(
@@ -188,19 +199,6 @@ def test_version_with_tag_format(
188199
assert captured.out == expected_output
189200

190201

191-
def test_version_tag_without_project_error(config, capsys):
192-
"""Test --tag requires --project or --verbose"""
193-
commands.Version(
194-
config,
195-
{
196-
"tag": True,
197-
},
198-
)()
199-
captured = capsys.readouterr()
200-
assert not captured.out
201-
assert "Tag can only be used with --project or --verbose." in captured.err
202-
203-
204202
@pytest.mark.parametrize(
205203
("next_increment", "current_version", "expected_version"),
206204
[

0 commit comments

Comments
 (0)