Skip to content

Commit 4434bd2

Browse files
committed
fix: report enum variable validation errors cleanly
1 parent ea3ceb5 commit 4434bd2

4 files changed

Lines changed: 95 additions & 7 deletions

File tree

structkit/commands/generate.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import argparse
55
from structkit.file_item import FileItem
66
from structkit.completers import file_strategy_completer, structures_completer
7-
from structkit.template_renderer import TemplateRenderer
7+
from structkit.template_renderer import TemplateRenderer, TemplateVariableError
88
from structkit.sources import SourceError, resolve_structures_path
99

1010
import subprocess
@@ -186,7 +186,11 @@ def execute(self, args):
186186
return
187187

188188
# Actually generate structure
189-
self._create_structure(args, mappings)
189+
try:
190+
self._create_structure(args, mappings)
191+
except TemplateVariableError as exc:
192+
self.logger.error(f"❗ {exc}")
193+
raise SystemExit(1) from None
190194

191195
# Run post-hooks
192196
if not self._run_hooks(post_hooks, hook_type="post"):

structkit/template_renderer.py

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
from structkit.utils import get_current_repo
2020

2121

22+
class TemplateVariableError(ValueError):
23+
"""Raised when a template variable is missing or fails validation."""
24+
25+
2226
class TemplateRenderer:
2327
def __init__(self, config_variables, input_store, non_interactive, mappings=None):
2428
self.config_variables = config_variables
@@ -147,8 +151,15 @@ def prompt_for_missing_vars(self, content, vars):
147151
self.logger.debug(f"Default values from config: {default_values}")
148152

149153
for var in undeclared_variables:
154+
conf = schema.get(var, {})
155+
if var in vars:
156+
if conf:
157+
coerced = self._coerce_and_validate(var, vars[var], conf)
158+
self.input_store.set_value(var, coerced)
159+
vars[var] = coerced
160+
continue
161+
150162
if var not in vars:
151-
conf = schema.get(var, {})
152163
required = conf.get('required', False)
153164
default = self.input_data.get(var, default_values.get(var, ""))
154165
if self.non_interactive:
@@ -190,7 +201,7 @@ def prompt_for_missing_vars(self, content, vars):
190201
user_input = raw
191202
else:
192203
# For invalid enum input, raise immediately instead of re-prompting
193-
raise ValueError(f"Variable '{var}' must be one of {enum}, got: {raw}")
204+
raise self._enum_value_error(var, raw, enum)
194205
else:
195206
if description:
196207
print(f"{icon} {BOLD}{var}{RESET}: {description}")
@@ -223,12 +234,12 @@ def _coerce_and_validate(self, name, value, conf):
223234
else:
224235
coerced = '' if value is None else str(value)
225236
except Exception:
226-
raise ValueError(f"Variable '{name}' could not be coerced to {vtype} (value: {original})")
237+
raise TemplateVariableError(f"Variable '{name}' could not be coerced to {vtype} (value: {original})")
227238

228239
# Enum validation
229240
enum = conf.get('enum')
230241
if enum is not None and coerced not in enum:
231-
raise ValueError(f"Variable '{name}' must be one of {enum}, got: {coerced}")
242+
raise self._enum_value_error(name, coerced, enum)
232243

233244
# Regex validation (only for strings)
234245
pattern = conf.get('regex') or conf.get('pattern')
@@ -255,3 +266,14 @@ def _as_num(x):
255266
raise ValueError(f"Variable '{name}' must be <= {maxv}, got {coerced}")
256267

257268
return coerced
269+
270+
def _enum_value_error(self, name, value, enum):
271+
allowed_values = ", ".join(str(item) for item in enum)
272+
if value is None or value == "":
273+
return TemplateVariableError(
274+
f"Variable '{name}' must be set to one of: {allowed_values}. "
275+
f"No value was provided. Pass --vars {name}=<value> or define a default."
276+
)
277+
return TemplateVariableError(
278+
f"Variable '{name}' must be one of: {allowed_values}. Got: {value}."
279+
)

tests/test_commands_more.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from structkit.commands.list import ListCommand
1010
from structkit.commands.mcp import MCPCommand
1111
from structkit.commands.validate import ValidateCommand
12+
from structkit.template_renderer import TemplateVariableError
1213

1314

1415
@pytest.fixture
@@ -114,6 +115,25 @@ def test_generate_mappings_file_not_found(parser, tmp_path):
114115
command.execute(args)
115116

116117

118+
def test_generate_reports_template_variable_error_without_traceback(parser, tmp_path, caplog):
119+
command = GenerateCommand(parser)
120+
args = parser.parse_args(['struct-x', str(tmp_path)])
121+
args.structures_path = None
122+
args.mappings_file = None
123+
args.backup = None
124+
125+
with patch.object(command, '_load_yaml_config', return_value={'files': [], 'folders': []}), \
126+
patch.object(command, '_create_structure', side_effect=TemplateVariableError(
127+
"Variable 'environment' must be one of: dev, staging, prod. Got: qa."
128+
)):
129+
with pytest.raises(SystemExit) as excinfo:
130+
command.execute(args)
131+
132+
assert excinfo.value.code == 1
133+
assert "Variable 'environment' must be one of: dev, staging, prod. Got: qa." in caplog.text
134+
assert "Traceback" not in caplog.text
135+
136+
117137
def test_info_nonexistent_file_logs_error(parser):
118138
command = InfoCommand(parser)
119139
args = parser.parse_args(['does-not-exist'])

tests/test_template_renderer.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import pytest
22
from unittest.mock import patch, MagicMock
3-
from structkit.template_renderer import TemplateRenderer
3+
from structkit.template_renderer import TemplateRenderer, TemplateVariableError
44

55
@pytest.fixture
66
def renderer():
@@ -204,3 +204,45 @@ def test_variable_icon_selection():
204204
assert renderer._get_variable_icon("version", "string") == "🏷️"
205205
assert renderer._get_variable_icon("config_path", "string") == "📁"
206206
assert renderer._get_variable_icon("random_var", "string") == "🔧"
207+
208+
209+
def test_enum_missing_value_has_clean_error(tmp_path):
210+
config_variables = [
211+
{"environment": {
212+
"type": "string",
213+
"description": "Target deployment environment",
214+
"enum": ["dev", "staging", "prod"],
215+
}}
216+
]
217+
renderer = TemplateRenderer(
218+
config_variables,
219+
str(tmp_path / "input.json"),
220+
non_interactive=True,
221+
)
222+
223+
with pytest.raises(TemplateVariableError) as excinfo:
224+
renderer.prompt_for_missing_vars("{{@ environment @}}", {})
225+
226+
message = str(excinfo.value)
227+
assert "Variable 'environment' must be set to one of: dev, staging, prod" in message
228+
assert "No value was provided" in message
229+
assert "--vars environment=<value>" in message
230+
231+
232+
def test_enum_invalid_value_has_clean_error(tmp_path):
233+
config_variables = [
234+
{"environment": {
235+
"type": "string",
236+
"enum": ["dev", "staging", "prod"],
237+
}}
238+
]
239+
renderer = TemplateRenderer(
240+
config_variables,
241+
str(tmp_path / "input.json"),
242+
non_interactive=True,
243+
)
244+
245+
with pytest.raises(TemplateVariableError) as excinfo:
246+
renderer.prompt_for_missing_vars("{{@ environment @}}", {"environment": "qa"})
247+
248+
assert str(excinfo.value) == "Variable 'environment' must be one of: dev, staging, prod. Got: qa."

0 commit comments

Comments
 (0)