refactor(config): close empty-list root gap in strict check; assert tolerant returns
Review follow-ups on the final salvage stack:
- `fast_safe_load(f) or {}` collapsed a falsy non-mapping root (`[]`) to `{}`
before the strict isinstance check, so an empty-list config evaded the
raise the previous commit added. Only map a None document (empty file) to
`{}`; every other non-mapping root now hits the strict branch.
- Docstring: the strict flag covers non-mapping roots too, not just parse
failures.
- Tests: assert the tolerant call's return per shape instead of merely
calling it; add the empty-list-root case.
This commit is contained in:
@@ -2285,7 +2285,7 @@ def check_config_version(*, raise_on_parse_error: bool = False) -> Tuple[int, in
|
||||
Returns (current_version, latest_version). Tolerant runtime status callers
|
||||
retain the historical latest/latest fallback for malformed YAML. Mutation
|
||||
and explicit validation paths can set ``raise_on_parse_error`` so a parse
|
||||
failure cannot be mistaken for an up-to-date config.
|
||||
failure or a non-mapping root cannot be mistaken for an up-to-date config.
|
||||
"""
|
||||
latest = _coerce_config_version(DEFAULT_CONFIG.get("_config_version", 1)) or 1
|
||||
config_path = get_config_path()
|
||||
@@ -2294,7 +2294,7 @@ def check_config_version(*, raise_on_parse_error: bool = False) -> Tuple[int, in
|
||||
|
||||
try:
|
||||
with open(config_path, encoding="utf-8") as f:
|
||||
config = fast_safe_load(f) or {}
|
||||
config = fast_safe_load(f)
|
||||
except Exception as e:
|
||||
# Invalid YAML needs a parse warning, not an automatic schema rewrite
|
||||
# that could replace the user's broken file with defaults.
|
||||
@@ -2305,6 +2305,8 @@ def check_config_version(*, raise_on_parse_error: bool = False) -> Tuple[int, in
|
||||
) from e
|
||||
return latest, latest
|
||||
|
||||
if config is None:
|
||||
config = {} # empty file / bare document: valid first-run state
|
||||
if not isinstance(config, dict):
|
||||
# A list/scalar root parses fine but is just as unusable as broken
|
||||
# YAML: save_config() would refuse it later, after .env was already
|
||||
|
||||
@@ -786,13 +786,22 @@ class TestConfigVersionDetection:
|
||||
assert load_config()["_config_version"] == DEFAULT_CONFIG["_config_version"]
|
||||
assert check_config_version() == (0, DEFAULT_CONFIG["_config_version"])
|
||||
|
||||
_LATEST = DEFAULT_CONFIG["_config_version"]
|
||||
# (bytes, strict match, tolerant return): tolerant malformed YAML keeps
|
||||
# the historical latest/latest fallback; a parseable non-mapping root is
|
||||
# reported as legacy (0).
|
||||
_INVALID_CONFIG_CASES = [
|
||||
pytest.param(b"model: [unterminated\n", "not valid YAML", id="malformed-yaml"),
|
||||
pytest.param(b"- just_a_list\n", "must be a mapping", id="list-root"),
|
||||
pytest.param(
|
||||
b"model: [unterminated\n", "not valid YAML", (_LATEST, _LATEST), id="malformed-yaml"
|
||||
),
|
||||
pytest.param(b"- just_a_list\n", "must be a mapping", (0, _LATEST), id="list-root"),
|
||||
pytest.param(b"[]\n", "must be a mapping", (0, _LATEST), id="empty-list-root"),
|
||||
]
|
||||
|
||||
@pytest.mark.parametrize("config_bytes, match", _INVALID_CONFIG_CASES)
|
||||
def test_strict_check_rejects_invalid_config(self, tmp_path, config_bytes, match):
|
||||
@pytest.mark.parametrize("config_bytes, match, tolerant", _INVALID_CONFIG_CASES)
|
||||
def test_strict_check_rejects_invalid_config(
|
||||
self, tmp_path, config_bytes, match, tolerant
|
||||
):
|
||||
config_path = tmp_path / "config.yaml"
|
||||
config_path.write_bytes(config_bytes)
|
||||
|
||||
@@ -800,11 +809,11 @@ class TestConfigVersionDetection:
|
||||
with pytest.raises(InvalidUserConfigError, match=match):
|
||||
check_config_version(raise_on_parse_error=True)
|
||||
# Tolerant callers keep the historical non-raising behavior.
|
||||
check_config_version()
|
||||
assert check_config_version() == tolerant
|
||||
|
||||
@pytest.mark.parametrize("config_bytes, match", _INVALID_CONFIG_CASES)
|
||||
@pytest.mark.parametrize("config_bytes, match, _tolerant", _INVALID_CONFIG_CASES)
|
||||
def test_migration_rejects_invalid_config_before_sanitizing_env(
|
||||
self, tmp_path, config_bytes, match
|
||||
self, tmp_path, config_bytes, match, _tolerant
|
||||
):
|
||||
config_path = tmp_path / "config.yaml"
|
||||
config_path.write_bytes(config_bytes)
|
||||
|
||||
Reference in New Issue
Block a user