Skip to content

Commit d6296fe

Browse files
committed
fix(test): reject mocks reached through reference calls that read referencing helpers
A mock a check reaches through a reference call such as __source("orders"), directly or from a helper the check reads, is now checked like a mock read by CTE name, so one that reads a helper calling a reference fails compile with a located P013 instead of failing at run time. The mock-only P013 now points at the mocked model's __ref() call, and its help says to call __ref() when the check reads the mock by CTE name.
1 parent 37df06a commit d6296fe

3 files changed

Lines changed: 236 additions & 7 deletions

File tree

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

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -251,28 +251,36 @@ def report_test_without_target_model(
251251
for key in reachable_cte_keys(graph=graph, roots=graph.reader_reads)
252252
if graph.ctes[key].name.startswith(_MOCK_CTE_PREFIXES)
253253
]
254-
first_call: str | None = None
254+
first_calls: dict[SqlReferenceKind, tuple[str, str]] = {}
255+
called_models: set[str] = set()
255256
for cte in _reader_ctes(payload):
256257
for reference in _references(sql=cte.sql_body, syntax=syntax):
257258
kind: SqlReferenceKind = SqlReferenceKind(reference.ref_kind)
258259
if reference.ref_name not in mocked.get(kind, frozenset()):
259260
continue
260261
read_mocks.append(f"{kind.fixture_cte_prefix}{reference.ref_name}")
261-
if first_call is None and kind is not SqlReferenceKind.TABLE_FUNCTION:
262-
first_call = _reference_call(reference)
262+
if kind is SqlReferenceKind.REF:
263+
called_models.add(reference.ref_name)
264+
if kind is not SqlReferenceKind.TABLE_FUNCTION:
265+
first_calls.setdefault(kind, (cte.name, _reference_call(reference)))
263266
mocks: tuple[str, ...] = tuple(dict.fromkeys(read_mocks))
264267
mocked_models: tuple[str, ...] = tuple(
265268
mock.removeprefix(REF_TEST_CTE_PREFIX)
266269
for mock in mocks
267270
if mock.startswith(REF_TEST_CTE_PREFIX)
268271
)
269272
reader: CompileSqlTestCte | None = next(iter(_reader_ctes(payload)), None)
273+
located: tuple[str, str] | None = first_calls.get(SqlReferenceKind.REF) or (
274+
_mock_name_read(readers=_reader_ctes(payload), mocks=mocks)
275+
or next(iter(first_calls.values()), None)
276+
)
270277
test_name: str = test_block.name or test_file.relative_path.stem
271278
message: str
272279
help_text: str
273280
if mocked_models:
274281
plural: str = "s" if len(mocked_models) > 1 else ""
275282
owner: str = "models'" if plural else "model's"
283+
verb: str = "keep" if called_models.issuperset(mocked_models) else "call"
276284
mocked_ctes: str = ", ".join(f"{REF_TEST_CTE_PREFIX}{name}" for name in mocked_models)
277285
calls: str = " and ".join(
278286
SqlReferenceKind.REF.example_call(name, quote='"') for name in mocked_models
@@ -283,7 +291,7 @@ def report_test_without_target_model(
283291
)
284292
help_text = (
285293
f"Mock the {owner} inputs instead (for example __source__<source> or "
286-
f"__ref__<upstream model>) and keep {calls} in the __assert__ or __expected__ CTE, "
294+
f"__ref__<upstream model>) and {verb} {calls} in the __assert__ or __expected__ CTE, "
287295
"or remove the test."
288296
)
289297
else:
@@ -294,13 +302,27 @@ def report_test_without_target_model(
294302
"runs that model with its mocks, or remove the test."
295303
)
296304
_HelperDiagnostics(test_file=test_file, test_block=test_block).report(
297-
cte_name=reader.name if reader is not None else test_name,
298-
call=first_call,
305+
cte_name=located[0] if located else (reader.name if reader is not None else test_name),
306+
call=located[1] if located else None,
299307
message=message,
300308
help=help_text,
301309
)
302310

303311

312+
def _mock_name_read(
313+
*, readers: tuple[CompileSqlTestCte, ...], mocks: tuple[str, ...]
314+
) -> tuple[str, str] | None:
315+
"""Return the first check reading a mocked model by CTE name, with that name."""
316+
317+
for mock in mocks:
318+
if not mock.startswith(REF_TEST_CTE_PREFIX):
319+
continue
320+
for reader in readers:
321+
if re.search(rf"(?<![\w$]){re.escape(mock)}(?![\w$])", reader.sql_body, re.IGNORECASE):
322+
return reader.name, mock
323+
return None
324+
325+
304326
def report_mocks_reading_referencing_helpers(
305327
*,
306328
authored_ctes: tuple[CompileSqlTestCte, ...],
@@ -333,8 +355,18 @@ def report_mocks_reading_referencing_helpers(
333355
graph: SqlTestCteGraph = sql_test_cte_graph(
334356
authored_ctes=authored_ctes, reader_ctes=reader_ctes
335357
)
358+
called_mocks: list[str] = []
359+
for sql in (
360+
*(cte.sql_body for cte in reader_ctes),
361+
*(graph.ctes[key].sql_body for key in _read_helper_keys(graph)),
362+
):
363+
for reference in _references(sql=sql, syntax=syntax):
364+
called_mocks.extend(
365+
_mock_keys(kind=SqlReferenceKind(reference.ref_kind), reference=reference)
366+
)
336367
used_mocks: tuple[str, ...] = _used_mock_keys(
337368
graph=graph,
369+
called_mocks=tuple(called_mocks),
338370
model_references={
339371
model_input.model_file.file_path.stem: model_input.references
340372
for model_input in model_inputs
@@ -386,17 +418,19 @@ def _read_helper_keys(graph: SqlTestCteGraph) -> tuple[str, ...]:
386418
def _used_mock_keys(
387419
*,
388420
graph: SqlTestCteGraph,
421+
called_mocks: tuple[str, ...],
389422
model_references: dict[str, tuple[CompileSqlReference, ...]],
390423
target_model_names: tuple[str, ...],
391424
mocked: frozenset[str],
392425
) -> tuple[str, ...]:
393-
"""Return mocks the test's readers or the models it runs read, in a stable order."""
426+
"""Return mocks the test's checks and read helpers, or the models it runs, read in order."""
394427

395428
used: list[str] = [
396429
key
397430
for key in reachable_cte_keys(graph=graph, roots=graph.reader_reads)
398431
if graph.ctes[key].name.startswith(_MOCK_CTE_PREFIXES)
399432
]
433+
used.extend(key for key in dict.fromkeys(called_mocks) if key in graph.ctes and key not in used)
400434
pending: list[str] = [name for name in target_model_names if name not in mocked]
401435
visited: set[str] = set(pending)
402436
while pending:

‎tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,6 +1849,39 @@ def prepare_optional_ceremony_project(*, tmp_path: Path, test_files: dict[str, s
18491849
' SELECT item_id, amount_doubled FROM __ref("item_archive")\n'
18501850
")\n"
18511851
),
1852+
"mock_called_reads_referencing_helper": (
1853+
'extras AS (SELECT item_id, amount FROM __ref("item_extras")),\n'
1854+
"mid AS (SELECT item_id, amount FROM extras),\n"
1855+
"__ref__items AS (SELECT 7 AS item_id, 5 AS amount),\n"
1856+
"__source__item_returns AS (SELECT item_id FROM mid),\n"
1857+
"__expected__item_totals AS (SELECT 7 AS item_id, 10 AS amount_doubled),\n"
1858+
"__assert__no_returns AS (\n"
1859+
' SELECT item_id FROM __source("item_returns") WHERE item_id <> 7\n'
1860+
")\n"
1861+
),
1862+
"mock_called_reads_referencing_helper_directly": (
1863+
'extras AS (SELECT item_id, amount FROM __ref("item_extras")),\n'
1864+
"__ref__items AS (SELECT 7 AS item_id, 5 AS amount),\n"
1865+
"__source__item_returns AS (SELECT item_id FROM extras),\n"
1866+
"__expected__item_totals AS (SELECT 7 AS item_id, 10 AS amount_doubled),\n"
1867+
"__assert__no_returns AS (\n"
1868+
' SELECT item_id FROM __source("item_returns") WHERE item_id <> 7\n'
1869+
")\n"
1870+
),
1871+
"assertion_reads_mocked_source_and_model": (
1872+
"__ref__items AS (\n SELECT 7 AS item_id, 5 AS amount\n),\n"
1873+
"__source__item_returns AS (SELECT 7 AS item_id),\n"
1874+
"__assert__returned_items AS (\n"
1875+
' SELECT r.item_id FROM __source("item_returns") AS r\n'
1876+
' JOIN __ref("items") AS i USING (item_id) WHERE i.amount <> 5\n'
1877+
")\n"
1878+
),
1879+
"assertion_reads_mock_by_name": (
1880+
"__ref__items AS (\n SELECT 7 AS item_id, 5 AS amount\n),\n"
1881+
"__assert__only_mocked_items AS (\n"
1882+
" SELECT item_id FROM __ref__items WHERE item_id <> 7\n"
1883+
")\n"
1884+
),
18521885
"mock_reads_referencing_helper": (
18531886
"__ref__items AS (\n SELECT item_id, 10 AS amount FROM base_rows\n),\n"
18541887
'base_rows AS (SELECT item_id FROM __ref("item_totals")),\n'

‎tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,168 @@ def test_given_helper_cte_references_when_testing_then_helpers_read_the_tested_m
706706
expected_column=39,
707707
expected_help_fragment='__ref("<model>")',
708708
),
709+
HelperReferenceErrorE2ETestCase(
710+
description="a mock a check calls reading a referencing helper through a middle helper on the python engine",
711+
test_name="mock_called_reads_referencing_helper",
712+
engine="python",
713+
expected_code="P013",
714+
expected_message=(
715+
"SQL test mock '__source__item_returns' reads helper CTE 'extras', which calls "
716+
'__ref("item_extras"); mocks and fixtures are defined before the models the '
717+
"test runs, so the helper cannot be resolved for them"
718+
),
719+
expected_line=4,
720+
expected_column=40,
721+
expected_help_fragment='FROM __ref__item_extras rather than FROM __ref("item_extras")',
722+
),
723+
HelperReferenceErrorE2ETestCase(
724+
description="a mock a check calls reading a referencing helper directly on the python engine",
725+
test_name="mock_called_reads_referencing_helper_directly",
726+
engine="python",
727+
expected_code="P013",
728+
expected_message=(
729+
"SQL test mock '__source__item_returns' reads helper CTE 'extras', which calls "
730+
'__ref("item_extras"); mocks and fixtures are defined before the models the '
731+
"test runs, so the helper cannot be resolved for them"
732+
),
733+
expected_line=4,
734+
expected_column=40,
735+
expected_help_fragment='FROM __ref__item_extras rather than FROM __ref("item_extras")',
736+
),
737+
HelperReferenceErrorE2ETestCase(
738+
description="test reading a mocked source and a mocked model on the python engine",
739+
test_name="assertion_reads_mocked_source_and_model",
740+
engine="python",
741+
expected_code="P013",
742+
expected_message=(
743+
"SQL test 'assertion_reads_mocked_source_and_model' mocks the model it tests "
744+
"(__ref__items), so the test has no model to run against"
745+
),
746+
expected_line=10,
747+
expected_column=8,
748+
expected_help_fragment='and keep __ref("items") in the __assert__ or __expected__ CTE',
749+
),
750+
HelperReferenceErrorE2ETestCase(
751+
description="test reading a mocked model by CTE name on the python engine",
752+
test_name="assertion_reads_mock_by_name",
753+
engine="python",
754+
expected_code="P013",
755+
expected_message=(
756+
"SQL test 'assertion_reads_mock_by_name' mocks the model it tests (__ref__items), "
757+
"so the test has no model to run against"
758+
),
759+
expected_line=8,
760+
expected_column=23,
761+
expected_help_fragment='and call __ref("items") in the __assert__ or __expected__ CTE',
762+
),
763+
HelperReferenceErrorE2ETestCase(
764+
description="a mock a check calls reading a referencing helper through a middle helper on the native engine",
765+
test_name="mock_called_reads_referencing_helper",
766+
engine="native",
767+
expected_code="P013",
768+
expected_message=(
769+
"SQL test mock '__source__item_returns' reads helper CTE 'extras', which calls "
770+
'__ref("item_extras"); mocks and fixtures are defined before the models the '
771+
"test runs, so the helper cannot be resolved for them"
772+
),
773+
expected_line=4,
774+
expected_column=40,
775+
expected_help_fragment='FROM __ref__item_extras rather than FROM __ref("item_extras")',
776+
),
777+
HelperReferenceErrorE2ETestCase(
778+
description="a mock a check calls reading a referencing helper directly on the native engine",
779+
test_name="mock_called_reads_referencing_helper_directly",
780+
engine="native",
781+
expected_code="P013",
782+
expected_message=(
783+
"SQL test mock '__source__item_returns' reads helper CTE 'extras', which calls "
784+
'__ref("item_extras"); mocks and fixtures are defined before the models the '
785+
"test runs, so the helper cannot be resolved for them"
786+
),
787+
expected_line=4,
788+
expected_column=40,
789+
expected_help_fragment='FROM __ref__item_extras rather than FROM __ref("item_extras")',
790+
),
791+
HelperReferenceErrorE2ETestCase(
792+
description="test reading a mocked source and a mocked model on the native engine",
793+
test_name="assertion_reads_mocked_source_and_model",
794+
engine="native",
795+
expected_code="P013",
796+
expected_message=(
797+
"SQL test 'assertion_reads_mocked_source_and_model' mocks the model it tests "
798+
"(__ref__items), so the test has no model to run against"
799+
),
800+
expected_line=10,
801+
expected_column=8,
802+
expected_help_fragment='and keep __ref("items") in the __assert__ or __expected__ CTE',
803+
),
804+
HelperReferenceErrorE2ETestCase(
805+
description="test reading a mocked model by CTE name on the native engine",
806+
test_name="assertion_reads_mock_by_name",
807+
engine="native",
808+
expected_code="P013",
809+
expected_message=(
810+
"SQL test 'assertion_reads_mock_by_name' mocks the model it tests (__ref__items), "
811+
"so the test has no model to run against"
812+
),
813+
expected_line=8,
814+
expected_column=23,
815+
expected_help_fragment='and call __ref("items") in the __assert__ or __expected__ CTE',
816+
),
817+
HelperReferenceErrorE2ETestCase(
818+
description="a mock a check calls reading a referencing helper through a middle helper on the native-preview engine",
819+
test_name="mock_called_reads_referencing_helper",
820+
engine="native-preview",
821+
expected_code="P013",
822+
expected_message=(
823+
"SQL test mock '__source__item_returns' reads helper CTE 'extras', which calls "
824+
'__ref("item_extras"); mocks and fixtures are defined before the models the '
825+
"test runs, so the helper cannot be resolved for them"
826+
),
827+
expected_line=4,
828+
expected_column=40,
829+
expected_help_fragment='FROM __ref__item_extras rather than FROM __ref("item_extras")',
830+
),
831+
HelperReferenceErrorE2ETestCase(
832+
description="a mock a check calls reading a referencing helper directly on the native-preview engine",
833+
test_name="mock_called_reads_referencing_helper_directly",
834+
engine="native-preview",
835+
expected_code="P013",
836+
expected_message=(
837+
"SQL test mock '__source__item_returns' reads helper CTE 'extras', which calls "
838+
'__ref("item_extras"); mocks and fixtures are defined before the models the '
839+
"test runs, so the helper cannot be resolved for them"
840+
),
841+
expected_line=4,
842+
expected_column=40,
843+
expected_help_fragment='FROM __ref__item_extras rather than FROM __ref("item_extras")',
844+
),
845+
HelperReferenceErrorE2ETestCase(
846+
description="test reading a mocked source and a mocked model on the native-preview engine",
847+
test_name="assertion_reads_mocked_source_and_model",
848+
engine="native-preview",
849+
expected_code="P013",
850+
expected_message=(
851+
"SQL test 'assertion_reads_mocked_source_and_model' mocks the model it tests "
852+
"(__ref__items), so the test has no model to run against"
853+
),
854+
expected_line=10,
855+
expected_column=8,
856+
expected_help_fragment='and keep __ref("items") in the __assert__ or __expected__ CTE',
857+
),
858+
HelperReferenceErrorE2ETestCase(
859+
description="test reading a mocked model by CTE name on the native-preview engine",
860+
test_name="assertion_reads_mock_by_name",
861+
engine="native-preview",
862+
expected_code="P013",
863+
expected_message=(
864+
"SQL test 'assertion_reads_mock_by_name' mocks the model it tests (__ref__items), "
865+
"so the test has no model to run against"
866+
),
867+
expected_line=8,
868+
expected_column=23,
869+
expected_help_fragment='and call __ref("items") in the __assert__ or __expected__ CTE',
870+
),
709871
),
710872
ids=lambda case: case.description,
711873
)

0 commit comments

Comments
 (0)