Skip to content

Commit 43f0435

Browse files
committed
Improve config resolution and add error handling for paths
Refactor `process_value` to handle `None` values and raise a `BumpVersionError` for non-existent files. Update related tests to ensure correct behavior for missing, existing, and URL-based config paths. These changes enhance robustness and user feedback in handling configuration inputs.
1 parent c84243d commit 43f0435

File tree

2 files changed

+58
-29
lines changed

2 files changed

+58
-29
lines changed

bumpversion/click_config.py

+10-4
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from click import Context, Option
1010
from click.decorators import FC, _param_memo # noqa: PLC2701
1111

12-
from bumpversion.exceptions import BadInputError
12+
from bumpversion.exceptions import BadInputError, BumpVersionError
1313
from bumpversion.ui import get_indented_logger
1414

1515
logger = get_indented_logger(__name__)
@@ -65,10 +65,10 @@ def __init__(
6565
**attrs,
6666
)
6767

68-
def process_value(self, ctx: Context, value: Any) -> Path:
68+
def process_value(self, ctx: Context, value: Any) -> Optional[Path]:
6969
"""Process the value of the option."""
7070
value = super().process_value(ctx, value)
71-
return resolve_conf_location(value)
71+
return resolve_conf_location(value) if value else None
7272

7373

7474
def config_option(*param_decls: str, cls: Optional[type[ConfigOption]] = None, **attrs: Any) -> Callable[[FC], FC]:
@@ -108,6 +108,9 @@ def resolve_conf_location(url_or_path: str) -> Path:
108108
Args:
109109
url_or_path: The URL or path to resolve.
110110
111+
Raises:
112+
BumpVersionError: if the file does not exist.
113+
111114
Returns:
112115
The contents of the location.
113116
"""
@@ -116,7 +119,10 @@ def resolve_conf_location(url_or_path: str) -> Path:
116119
if parsed_url.scheme in ("http", "https"):
117120
return download_url(url_or_path)
118121

119-
return Path(url_or_path)
122+
path = Path(url_or_path)
123+
if not path.exists():
124+
raise BumpVersionError(f"'{path}' does not exist.")
125+
return path
120126

121127

122128
def download_url(url: str) -> Path:

tests/test_click_config.py

+48-25
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,63 @@
11
"""Tests for the bumpversion.click_config module."""
22

33
from pathlib import Path
4+
from typing import Optional
45

56
import click
67
import httpx
78
import pytest
89

9-
from bumpversion.click_config import config_option, resolve_conf_location, download_url
10+
from bumpversion.click_config import config_option, download_url, resolve_conf_location
1011
from bumpversion.exceptions import BadInputError
1112

1213

1314
@click.command()
1415
@config_option()
15-
def hello(config: Path):
16+
def hello(config: Optional[Path]):
1617
"""Say hello."""
1718
click.echo(f"Hello {config}!")
18-
click.echo(f"path exists: {config.exists()}")
19+
if config:
20+
click.echo(f"path exists: {config.exists()}")
21+
else:
22+
click.echo("Nothing was passed.")
1923

2024

2125
class TestConfigOption:
2226
"""Tests to for config_option."""
2327

24-
def test_config_option(self, runner):
25-
"""Passing an invalid option should raise an error."""
28+
def test_passing_missing_file_raises_error(self, runner):
29+
"""Explicitly passing a path that does not exist should raise an error."""
2630
result = runner.invoke(hello, ["--config", "idont/exist.txt"])
2731

32+
assert result.exit_code != 0
33+
34+
def test_passing_nothing_returns_none(self, runner):
35+
"""If nothing is passed, the value is `None`."""
36+
result = runner.invoke(hello, [])
37+
38+
assert result.exit_code == 0
39+
assert "Hello None!" in result.output
40+
41+
def test_passing_existing_file_returns_path(self, runner, fixtures_path: Path):
42+
"""Passing an existing file path should return a Path object."""
43+
config_path = fixtures_path / "basic_cfg.toml"
44+
result = runner.invoke(hello, ["--config", str(config_path)])
45+
46+
assert result.exit_code == 0
47+
assert f"Hello {config_path}!" in result.output
48+
49+
def test_passing_url_returns_path(self, runner, fixtures_path: Path, httpserver):
50+
"""Passing an existing file path should return a Path object."""
51+
# Assemble
52+
config = fixtures_path.joinpath("basic_cfg.toml").read_text()
53+
httpserver.serve_content(config)
54+
55+
# Act
56+
result = runner.invoke(hello, ["--config", f"{httpserver.url}/basic_cfg.toml"])
57+
58+
# Assert
2859
assert result.exit_code == 0
29-
assert "path exists: False" in result.output
60+
assert "path exists: True" in result.output
3061

3162

3263
class TestResolveConfLocation:
@@ -39,45 +70,37 @@ def test_resolves_file_path(self, fixtures_path: Path):
3970
assert isinstance(result, Path)
4071
assert result == dummy_path
4172

42-
def test_resolves_valid_url(self, mocker):
73+
def test_resolves_valid_url(self, fixtures_path: Path, httpserver):
4374
"""Should download the file if given a valid URL."""
4475
# Arrange
45-
mocked_download = mocker.patch(
46-
"bumpversion.click_config.download_url", return_value=Path("/tmp/downloaded_config.txt")
47-
)
48-
url = "http://example.com/config.txt"
76+
config = fixtures_path.joinpath("basic_cfg.toml").read_text()
77+
httpserver.serve_content(config)
78+
url = f"{httpserver.url}/basic_cfg.toml"
4979

5080
# Act
5181
result = resolve_conf_location(url)
5282

5383
# Assert
54-
mocked_download.assert_called_once_with(url)
5584
assert isinstance(result, Path)
56-
assert str(result) == "/tmp/downloaded_config.txt"
85+
assert result.read_text() == config
5786

5887

5988
class TestDownloadUrl:
6089
"""Tests for download_url."""
6190

62-
def test_download_url_success(self, mocker, tmp_path: Path):
91+
def test_download_url_success(self, fixtures_path: Path, httpserver):
6392
"""Should return a Path for a valid request."""
6493
# Arrange
65-
mock_response = mocker.Mock(spec=httpx.Response)
66-
mock_response.status_code = 200
67-
mock_response.text = "content"
68-
mocker.patch("httpx.get", return_value=mock_response)
69-
tmp_file = tmp_path / "tempfile.txt"
70-
mock_tempfile = mocker.patch("bumpversion.click_config.NamedTemporaryFile", autospec=True)
71-
temp_file_instance = mock_tempfile.return_value.__enter__.return_value
72-
temp_file_instance.name = str(tmp_file)
94+
config = fixtures_path.joinpath("basic_cfg.toml").read_text()
95+
httpserver.serve_content(config)
96+
url = f"{httpserver.url}/basic_cfg.toml"
7397

7498
# Act
75-
result = download_url("http://example.com/config.txt")
99+
result = download_url(url)
76100

77101
# Assert
78102
assert isinstance(result, Path)
79-
assert result == tmp_file
80-
mock_tempfile.assert_called_once()
103+
assert result.read_text() == config
81104

82105
def test_download_url_request_error(self, mocker):
83106
"""Should raise BadInputError for a RequestError."""

0 commit comments

Comments
 (0)