Skip to content

Commit 649264e

Browse files
committed
fix(scenarios): preserve scenario labels for unclosed comments
1 parent 91a05a9 commit 649264e

4 files changed

Lines changed: 71 additions & 19 deletions

File tree

‎src/sqlbuild/compiler/compile/_helpers/scenarios/core.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def extract_sql_scenario_ctes(*, sql: str, file_label: str) -> CompileSqlScenari
6060
def extract_sql_scenario_expected_model_names(*, sql: str, file_label: str) -> tuple[str, ...]:
6161
"""Extract explicit expected-model relationships without inspecting CTE bodies."""
6262

63-
start: int = _skip_ignorable(sql=sql, start=0)
63+
start: int = _skip_ignorable(sql=sql, start=0, context_label=_CONTEXT)
6464
if _try_consume_keyword(sql=sql, start=start, keyword=SQL_WITH_KEYWORD) is None:
6565
return ()
6666
ctes: tuple[CompileSqlScenarioCte, ...] = extract_top_level_ctes_with_scanner(

‎src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -215,16 +215,18 @@ def extract_top_level_ctes_with_scanner[CteT](
215215
"""Scan top-level `WITH` CTEs followed by the ceremonial `SELECT 1` of a test file."""
216216

217217
with_end: int | None = _try_consume_keyword(
218-
sql=sql, start=_skip_ignorable(sql=sql, start=0), keyword=SQL_WITH_KEYWORD
218+
sql=sql,
219+
start=_skip_ignorable(sql=sql, start=0, context_label=context_label),
220+
keyword=SQL_WITH_KEYWORD,
219221
)
220222
if with_end is None:
221223
raise CompileInputError(
222224
f"{context_label} '{file_label}' must declare {with_requirement} before `SELECT 1`"
223225
)
224-
index: int = _skip_ignorable(sql=sql, start=with_end)
226+
index: int = _skip_ignorable(sql=sql, start=with_end, context_label=context_label)
225227
recursive_end: int | None = _try_consume_keyword(sql=sql, start=index, keyword="RECURSIVE")
226228
if recursive_end is not None:
227-
index = _skip_ignorable(sql=sql, start=recursive_end)
229+
index = _skip_ignorable(sql=sql, start=recursive_end, context_label=context_label)
228230

229231
ctes: list[CteT] = []
230232
seen_cte_names: set[str] = set()
@@ -238,18 +240,18 @@ def extract_top_level_ctes_with_scanner[CteT](
238240
)
239241
seen_cte_names.add(cte_name)
240242

241-
index = _skip_ignorable(sql=sql, start=index)
243+
index = _skip_ignorable(sql=sql, start=index, context_label=context_label)
242244
if index < len(sql) and sql[index] == SQL_OPEN_PAREN_TOKEN:
243245
index = find_matching_paren(sql=sql, open_paren_index=index, context=context_label) + 1
244-
index = _skip_ignorable(sql=sql, start=index)
246+
index = _skip_ignorable(sql=sql, start=index, context_label=context_label)
245247
index = _consume_keyword(
246248
sql=sql,
247249
start=index,
248250
keyword="AS",
249251
file_label=file_label,
250252
context_label=context_label,
251253
)
252-
index = _skip_ignorable(sql=sql, start=index)
254+
index = _skip_ignorable(sql=sql, start=index, context_label=context_label)
253255
if index >= len(sql) or sql[index] != SQL_OPEN_PAREN_TOKEN:
254256
raise CompileInputError(
255257
f"{context_label} '{file_label}' CTE '{cte_name}' must use AS (...)"
@@ -259,9 +261,9 @@ def extract_top_level_ctes_with_scanner[CteT](
259261
sql=sql, open_paren_index=index, context=context_label
260262
)
261263
ctes.append(cte_type(name=cte_name, sql_body=sql[cte_body_start:cte_body_end].strip()))
262-
index = _skip_ignorable(sql=sql, start=cte_body_end + 1)
264+
index = _skip_ignorable(sql=sql, start=cte_body_end + 1, context_label=context_label)
263265
if index < len(sql) and sql[index] == SQL_ARGUMENT_SEPARATOR_TOKEN:
264-
index = _skip_ignorable(sql=sql, start=index + 1)
266+
index = _skip_ignorable(sql=sql, start=index + 1, context_label=context_label)
265267
continue
266268
break
267269

@@ -857,30 +859,30 @@ def _validate_ceremonial_select(
857859
file_label: str,
858860
context_label: str = _CONTEXT,
859861
) -> None:
860-
if _is_ceremonial_select_statement(sql=sql, start=start):
862+
if _is_ceremonial_select_statement(sql=sql, start=start, context_label=context_label):
861863
return
862864
raise CompileInputError(
863865
f"{context_label} '{file_label}' must end with a ceremonial top-level `SELECT 1` "
864866
"after its CTEs"
865867
)
866868

867869

868-
def _is_ceremonial_select_statement(*, sql: str, start: int) -> bool:
869-
index: int = _skip_ignorable(sql=sql, start=start)
870+
def _is_ceremonial_select_statement(*, sql: str, start: int, context_label: str = _CONTEXT) -> bool:
871+
index: int = _skip_ignorable(sql=sql, start=start, context_label=context_label)
870872
select_end: int | None = _try_consume_keyword(sql=sql, start=index, keyword="SELECT")
871873
if select_end is None:
872874
return False
873-
index = _skip_ignorable(sql=sql, start=select_end)
875+
index = _skip_ignorable(sql=sql, start=select_end, context_label=context_label)
874876
if index >= len(sql) or sql[index] != SQL_CEREMONIAL_SELECT_VALUE:
875877
return False
876-
index = _skip_ignorable(sql=sql, start=index + 1)
877-
return _is_statement_end(sql=sql, start=index)
878+
index = _skip_ignorable(sql=sql, start=index + 1, context_label=context_label)
879+
return _is_statement_end(sql=sql, start=index, context_label=context_label)
878880

879881

880-
def _is_statement_end(*, sql: str, start: int) -> bool:
882+
def _is_statement_end(*, sql: str, start: int, context_label: str = _CONTEXT) -> bool:
881883
index: int = start
882884
if index < len(sql) and sql[index] == SQL_STATEMENT_TERMINATOR_TOKEN:
883-
index = _skip_ignorable(sql=sql, start=index + 1)
885+
index = _skip_ignorable(sql=sql, start=index + 1, context_label=context_label)
884886
return index == len(sql)
885887

886888

@@ -1097,7 +1099,7 @@ def _read_identifier(
10971099
return sql[start:index], index
10981100

10991101

1100-
def _skip_ignorable(*, sql: str, start: int) -> int:
1102+
def _skip_ignorable(*, sql: str, start: int, context_label: str = _CONTEXT) -> int:
11011103
index: int = start
11021104
while index < len(sql):
11031105
if sql[index].isspace():
@@ -1107,7 +1109,7 @@ def _skip_ignorable(*, sql: str, start: int) -> int:
11071109
index = skip_line_comment(sql=sql, start=index)
11081110
continue
11091111
if sql.startswith("/*", index):
1110-
index = skip_block_comment(sql=sql, start=index, context=_CONTEXT)
1112+
index = skip_block_comment(sql=sql, start=index, context=context_label)
11111113
continue
11121114
return index
11131115
return index

‎tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_scenario.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,36 @@
4242
)
4343

4444

45+
@pytest.mark.parametrize(
46+
"test_case",
47+
(
48+
ScenarioCliE2ETestCase(
49+
description="unclosed scenario comment names the scenario",
50+
command=("--no-color", "scenario", "test"),
51+
expected_exit_code=1,
52+
expected_stderr_fragments=("SQL scenario contains an unclosed block comment",),
53+
),
54+
),
55+
ids=lambda case: case.description,
56+
)
57+
def test_given_unclosed_scenario_comment_when_running_cli_then_error_names_scenario(
58+
test_case: ScenarioCliE2ETestCase,
59+
tmp_path: Path,
60+
) -> None:
61+
repo_files: dict[str, str] = build_scenario_project_files()
62+
repo_files["tests/scenarios/order_totals_pass.sql"] = "SCENARIO ();\nWITH /* unfinished"
63+
project_dir: Path = prepare_inline_project(
64+
tmp_path=tmp_path, project_name="scenario_comment", repo_files=repo_files
65+
)
66+
result: subprocess.CompletedProcess[str] = run_sqb(
67+
command=test_case.command, project_dir=project_dir
68+
)
69+
70+
assert result.returncode == test_case.expected_exit_code
71+
for fragment in test_case.expected_stderr_fragments:
72+
assert fragment in result.stderr
73+
74+
4575
@pytest.mark.parametrize(
4676
"test_case",
4777
(

‎tests/unit/src/sqlbuild/compiler/compile/_helpers/test_scenarios.py‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,16 @@ def test_given_invalid_scenario_source_refs_when_building_inputs_then_it_raises_
457457
"one __expected__<model> or __assert__<assertion> CTE before `SELECT 1`"
458458
),
459459
),
460+
CteScannerMessageTestCase(
461+
description="unclosed comment before a fixture uses scenario wording",
462+
sql="WITH /* unfinished",
463+
expected_message="SQL scenario contains an unclosed block comment",
464+
),
465+
CteScannerMessageTestCase(
466+
description="unclosed trailing comment uses scenario wording",
467+
sql="WITH __expected__orders AS (SELECT 1 AS order_id) SELECT 1; /* unfinished",
468+
expected_message="SQL scenario contains an unclosed block comment",
469+
),
460470
CteScannerMessageTestCase(
461471
description="scenario with an invalid cte name uses scenario wording",
462472
sql="WITH 1orders AS (SELECT 1) SELECT 1",
@@ -505,6 +515,16 @@ def test_given_malformed_scenario_ctes_when_extracting_then_messages_name_the_sc
505515
sql="WITH __source__raw_orders (SELECT 1 AS order_id) SELECT 1",
506516
expected_message="SQL scenario 'tests/scenarios/orders.sql' expected keyword AS",
507517
),
518+
CteScannerMessageTestCase(
519+
description="expected model scan unclosed comment uses scenario wording",
520+
sql="WITH /* unfinished",
521+
expected_message="SQL scenario contains an unclosed block comment",
522+
),
523+
CteScannerMessageTestCase(
524+
description="expected model scan leading unclosed comment uses scenario wording",
525+
sql="/* unfinished",
526+
expected_message="SQL scenario contains an unclosed block comment",
527+
),
508528
CteScannerMessageTestCase(
509529
description="expected model scan without a target uses scenario wording",
510530
sql=(

0 commit comments

Comments
 (0)