diff --git a/crates/README.md b/crates/README.md index 2f50320c3b..414d3707ea 100644 --- a/crates/README.md +++ b/crates/README.md @@ -64,12 +64,12 @@ stages run natively: The shipped tier covers discovery and rendering: declaration files and scopes, model config, reference extraction, the model loop, macro calls and the macro-call store, and attachments. SQL -analysis, contracts, lineage, SQL-test glue and project assembly are still `preview`. When a -native render stage fails, the compile runs the Python stage again so the error is exactly -Python's; native error messages replace that re-run later. Because the macro-call store is -shipped, the [macro determinism contract](../website/src/content/docs/docs/concepts/macros.mdx) -applies to every default compile; `SQLBUILD_COMPILER_ENGINE=python` runs every macro call each -time. +analysis, contracts, lineage, SQL-test glue and project assembly are still `preview`. A failing +render raises its first error directly, as the Python stage would, and runs each macro call at +most once; a divergence the bridge detects is reported as a native stage mismatch. Because the +macro-call store is shipped, the +[macro determinism contract](../website/src/content/docs/docs/concepts/macros.mdx) applies to +every default compile; `SQLBUILD_COMPILER_ENGINE=python` runs every macro call each time. Each native stage declares its tier once, in `NATIVE_STAGE_TIERS` in `src/sqlbuild/compiler/frontier/constants.py`. A stage moves from `preview` to `shipped` by diff --git a/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py index 1cfe9d0c6c..aa3cd6717e 100644 --- a/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py @@ -23,6 +23,10 @@ 'def cents(expression: str) -> str:\n """Convert to cents."""\n' ' return f"({expression} * 100)"\n' ) +_EXPLODE_MACRO: str = ( + '\n\ndef explode(expression: str) -> str:\n """Fail while rendering."""\n' + ' raise ValueError(f"cannot render {expression}")\n' +) _ORDERS_FOR: str = ( "FUNCTION (\n" ' description "Orders for one customer",\n' @@ -93,6 +97,51 @@ def _macro_cases() -> tuple[FailureCase, ...]: }, ), ), + failure_case( + name="macro-raises-after-a-call", + expected_code="P001", + expected_message="failed: cannot render status", + files=_staging_columns( + columns='@cents("amount") AS amount, @explode("status") AS status', + files={_STAGING_MACROS: _CENTS_MACRO + _EXPLODE_MACRO}, + ), + ), + failure_case( + name="macro-arguments-do-not-parse", + expected_code="P001", + expected_message="could not be parsed", + files=_staging_columns( + columns='amount, status, @cents("amount",,) AS cents', + files={_STAGING_MACROS: _CENTS_MACRO}, + ), + ), + failure_case( + name="macro-private-helper", + expected_code="P001", + expected_message="Unknown macro '@_scaled'", + files=_staging_columns( + columns='amount, status, @_scaled("amount") AS scaled', + files={ + _STAGING_MACROS: _CENTS_MACRO + + "\n\ndef _scaled(expression: str) -> str:\n return expression\n" + }, + ), + ), + failure_case( + name="macro-first-of-two-failing-models", + expected_code="P001", + expected_message="failed: cannot render total", + files={ + **_staging_columns( + columns='amount, @explode("status") AS status', + files={_STAGING_MACROS: _CENTS_MACRO + _EXPLODE_MACRO}, + ), + "macros/explode.py": _EXPLODE_MACRO.replace("def explode", "def blow_up"), + FAILURE_MART_PATH: FAILURE_BASE_MART.replace( + "SUM(amount)", 'SUM(@blow_up("total"))' + ), + }, + ), failure_case( name="macro-output-calls-macro", expected_code="P001", diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros.md index 0068663e6c..15e69e1124 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros.md @@ -225,6 +225,8 @@ SQLBuild relies on this contract to reuse macro results: - [Compile reuse](../cli/compile.md) replays an unchanged compile without running macros again. - A compile may run each distinct macro call once and reuse its result for identical calls elsewhere in the project. +- A compile that fails stops at its first error. It does not run the project's macros a second + time to report that error. - Macro call results are kept between compiles under `target/cache/` by default, so editing a model does not run its unchanged macro calls again. They are recomputed when macro or other Python files, configuration, declarations, seeds, sources, variables, the target, diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py b/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py index 5f9c113de5..eb2dba4c8b 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py @@ -1001,7 +1001,7 @@ def native_model_validation( def native_validation_error( *, error: _native.NativeConfigError, values: dict[str, object] ) -> Exception: - """Return a native validator error; validators read expanded SQL, so the bridge decides.""" + """Return a native validator error, unmarked because validators read macro-expanded SQL.""" return native_config_error(error=error, bridge_independent=False, values=values) diff --git a/src/sqlbuild/compiler/macro_bridge/_helpers/splicing.py b/src/sqlbuild/compiler/macro_bridge/_helpers/splicing.py new file mode 100644 index 0000000000..f160f2f1e8 --- /dev/null +++ b/src/sqlbuild/compiler/macro_bridge/_helpers/splicing.py @@ -0,0 +1,23 @@ +"""Splice macro outputs in Python for text the native splice cannot read.""" + +from __future__ import annotations + + +def splice_text( + *, sql: str, bounds: list[tuple[int, int]], outputs: list[str] +) -> tuple[str, list[tuple[int, int, int, int]]]: + """Replace each `(start, end)` span with its output, as `splice_macro_calls` does.""" + + parts: list[str] = [] + spans: list[tuple[int, int, int, int]] = [] + cursor: int = 0 + length: int = 0 + for (start, end), output in zip(bounds, outputs, strict=True): + parts.append(sql[cursor:start]) + length += start - cursor + parts.append(output) + spans.append((start, end, length, length + len(output))) + length += len(output) + cursor = end + parts.append(sql[cursor:]) + return "".join(parts), spans diff --git a/src/sqlbuild/compiler/macro_bridge/_helpers/store_keys.py b/src/sqlbuild/compiler/macro_bridge/_helpers/store_keys.py index f507172914..35a6b520ad 100644 --- a/src/sqlbuild/compiler/macro_bridge/_helpers/store_keys.py +++ b/src/sqlbuild/compiler/macro_bridge/_helpers/store_keys.py @@ -21,18 +21,29 @@ from sqlbuild.compiler.scopes.models import DeclarationIdentity from sqlbuild.python_nodes.models import SqlResourceRef +_UNSTABLE_KEY_ERRORS: tuple[type[Exception], ...] = ( + UncacheableValueError, + UnicodeEncodeError, + RecursionError, +) -def macro_store_token(*, loaded: LoadedMacro, identity: DeclarationIdentity | None) -> str: - """Name a macro by its name, file, declaration identity and exact source.""" - return _key_text( - [ - loaded.name, - loaded.relative_path.as_posix(), - _canonical(identity), - _native.content_digest([loaded.raw_source]), - ] - ) +def macro_store_token(*, loaded: LoadedMacro, identity: DeclarationIdentity | None) -> str | None: + """Name a macro by its name, file, identity and exact source, or None when it has no key.""" + + try: + return _encodable( + _key_text( + [ + loaded.name, + loaded.relative_path.as_posix(), + _canonical(identity), + _native.content_digest([loaded.raw_source]), + ] + ) + ) + except _UNSTABLE_KEY_ERRORS: + return None def context_store_token( @@ -41,22 +52,25 @@ def context_store_token( """Digest the whole context a macro may read, or None when a value has no stable text.""" try: - text: str = _key_text( + return _native.content_digest( [ - _canonical_context(macro_context), - None - if declarations is None - else [ - _canonical(declarations.constants), - _canonical(declarations.enums), - sorted(declarations.inaccessible_constants), - sorted(declarations.inaccessible_enums), - ], + _key_text( + [ + _canonical_context(macro_context), + None + if declarations is None + else [ + _canonical(declarations.constants), + _canonical(declarations.enums), + sorted(declarations.inaccessible_constants), + sorted(declarations.inaccessible_enums), + ], + ] + ) ] ) - except UncacheableValueError: + except _UNSTABLE_KEY_ERRORS: return None - return _native.content_digest([text]) def call_class_store_text( @@ -64,18 +78,23 @@ def call_class_store_text( macro_tokens: tuple[str, ...], context_token: str | None, prior_relations: tuple[SqlResourceRef, ...] | None, -) -> str: - """Text of one persistent call class: its macros, context and relations passed so far.""" - - return _key_text( - [ - list(macro_tokens), - context_token, - None - if prior_relations is None - else [[relation.kind.value, relation.name] for relation in prior_relations], - ] - ) +) -> str | None: + """Text of one persistent call class, or None when a relation name has no UTF-8 text.""" + + try: + return _encodable( + _key_text( + [ + list(macro_tokens), + context_token, + None + if prior_relations is None + else [[relation.kind.value, relation.name] for relation in prior_relations], + ] + ) + ) + except UnicodeEncodeError: + return None def _canonical_context(macro_context: MacroContext) -> object: @@ -141,3 +160,10 @@ def _type_name(kind: type) -> str: def _key_text(value: object) -> str: return json.dumps(value, ensure_ascii=False, separators=(",", ":")) + + +def _encodable(text: str) -> str: + """Return `text`, raising `UnicodeEncodeError` when the native store cannot hold it.""" + + _ = text.encode("utf-8") + return text diff --git a/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py b/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py index e2d317fa45..731fe2ca5b 100644 --- a/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py +++ b/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py @@ -14,6 +14,7 @@ LoadedMacro, MacroContext, ) +from sqlbuild.compiler.macro_bridge._helpers.splicing import splice_text from sqlbuild.compiler.macro_bridge._helpers.store_environment import ( digest_module_files, module_digests_metadata, @@ -51,7 +52,6 @@ def __init__(self, *, python_version: tuple[int, int], unicode_version: str) -> self._module_digests: dict[str, str] = {} self._observed_modules: dict[str, object] = {} self._project_inputs: tuple[Path, list[str], str] = (Path(), [], "") - self._scanned: bool = False def attach_store(self, *, cache_dir: Path, project_dir: Path, model_paths: list[str]) -> None: """Reuse call results stored by earlier compiles of the same code and project files.""" @@ -134,10 +134,12 @@ def _observe_modules(self) -> bool: def scan(self, sql: str) -> tuple[MacroCallSite, ...] | None: """Return the top-level call sites of `sql`, or None when Python must expand it.""" - self._scanned = True - rows: list[tuple[int, int, str, list[str], bool]] | None = _native.scan_macro_call_sites( - sql, self._python_version, self._unicode_version - ) + try: + rows: list[tuple[int, int, str, list[str], bool]] | None = ( + _native.scan_macro_call_sites(sql, self._python_version, self._unicode_version) + ) + except UnicodeEncodeError: + return None if rows is None: return None return tuple( @@ -151,12 +153,6 @@ def scan(self, sql: str) -> tuple[MacroCallSite, ...] | None: for start, end, name, tree_names, typed_reference_text in rows ) - @property - def scanned(self) -> bool: - """Whether any authored string has been scanned, so this compile left the Python path.""" - - return self._scanned - def call_class( self, *, @@ -213,15 +209,17 @@ def lookup( class_id: int | None = self._class_ids.get(class_key) if class_id is None: class_id = self._class_ids[class_key] = len(self._class_ids) - if self._store_path is not None and call_class.persistent: - self._memo.set_persistent_class( - class_id, - call_class_store_text( - macro_tokens=call_class.macro_store_tokens, - context_token=call_class.context_store_token, - prior_relations=prior_relations, - ), + class_text: str | None = ( + call_class_store_text( + macro_tokens=call_class.macro_store_tokens, + context_token=call_class.context_store_token, + prior_relations=prior_relations, ) + if self._store_path is not None and call_class.persistent + else None + ) + if class_text is not None: + self._memo.set_persistent_class(class_id, class_text) found: tuple[str, list[tuple[str, str]], list[tuple[int, str, str]]] | None = ( self._memo.lookup(class_id, call_text) ) @@ -249,11 +247,18 @@ def record( if self._store_trusted and len(sys.modules) != len(self._observed_modules): _ = self._observe_modules() - self._memo.record( - class_id, - call_text, - (sql, [(relation.kind.value, relation.name) for relation in relations], list(events)), - ) + try: + self._memo.record( + class_id, + call_text, + ( + sql, + [(relation.kind.value, relation.name) for relation in relations], + list(events), + ), + ) + except UnicodeEncodeError: + return def splice( self, *, sql: str, sites: tuple[MacroCallSite, ...], outputs: list[str] @@ -262,9 +267,11 @@ def splice( rendered: str spans: list[tuple[int, int, int, int]] - rendered, spans = _native.splice_macro_calls( - sql, [(site.start, site.end) for site in sites], outputs - ) + bounds: list[tuple[int, int]] = [(site.start, site.end) for site in sites] + try: + rendered, spans = _native.splice_macro_calls(sql, bounds, outputs) + except UnicodeEncodeError: + rendered, spans = splice_text(sql=sql, bounds=bounds, outputs=outputs) return rendered, tuple( ExpansionSpan( source_start=source_start, diff --git a/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py b/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py index c30adf809f..fcaa59d694 100644 --- a/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py +++ b/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py @@ -1,4 +1,4 @@ -"""Run one render stage through the native macro bridge, re-running Python on its failures.""" +"""Run one render stage through the native macro bridge, storing its calls on success.""" from __future__ import annotations @@ -9,14 +9,12 @@ from contextvars import Token import sqlbuild._native as _native -from sqlbuild.compiler.compile.exceptions import CompileInputError -from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.macro_bridge.classes.macro_bridge import MacroBridge from sqlbuild.compiler.macro_bridge.constants import ACTIVE_MACRO_BRIDGE, STORE_MODULE_PATHS def run_with_macro_bridge[T](*, stage: Callable[[], T]) -> T: - """Run `stage` with the bridge, storing calls on success; failures it could shape re-run.""" + """Run `stage` once with the bridge; its errors are the Python stage's, raised as they occur.""" python_version: tuple[int, int] = (sys.version_info[0], sys.version_info[1]) if not _native.native_text_supported(python_version, unicodedata.unidata_version): @@ -25,34 +23,21 @@ def run_with_macro_bridge[T](*, stage: Callable[[], T]) -> T: python_version=python_version, unicode_version=unicodedata.unidata_version ) token: Token[object | None] = ACTIVE_MACRO_BRIDGE.set(bridge) - native_error: Exception | None = None try: result: T = stage() - except Exception as error: - if not bridge.scanned or ( - isinstance(error, CompileInputError) and error.bridge_independent - ): - raise - native_error = error - else: - bridge.save_store() - STORE_MODULE_PATHS.update(bridge.store_module_paths()) - hits, misses, recorded = bridge.stats() - store_hits, store_records = bridge.store_stats() - logging.getLogger(__name__).debug( - "Native macro bridge: %d memo hits, %d store hits, %d misses, %d recorded calls, " - "%d stored calls", - hits, - store_hits, - misses, - recorded, - store_records, - ) - return result finally: ACTIVE_MACRO_BRIDGE.reset(token) - _ = stage() - raise NativeStageMismatchError( - "The native macro bridge failed where the Python render succeeded; " - "run with SQLBUILD_COMPILER_ENGINE=python" - ) from native_error + bridge.save_store() + STORE_MODULE_PATHS.update(bridge.store_module_paths()) + hits, misses, recorded = bridge.stats() + store_hits, store_records = bridge.store_stats() + logging.getLogger(__name__).debug( + "Native macro bridge: %d memo hits, %d store hits, %d misses, %d recorded calls, " + "%d stored calls", + hits, + store_hits, + misses, + recorded, + store_records, + ) + return result diff --git a/src/sqlbuild/compiler/model_config/main/_native_config_error.py b/src/sqlbuild/compiler/model_config/main/_native_config_error.py index 9b5ba695c2..b8814631ae 100644 --- a/src/sqlbuild/compiler/model_config/main/_native_config_error.py +++ b/src/sqlbuild/compiler/model_config/main/_native_config_error.py @@ -23,7 +23,7 @@ def native_config_error( bridge_independent: bool, values: Mapping[str, object] | None = None, ) -> Exception: - """Return Python's exception for `error`; mark it bridge-independent only if SQL is unread.""" + """Return Python's exception for `error`, marked `bridge_independent` when SQL is unread.""" if error.class_name == CONFIG_VALUE_TYPE_ERROR and error.key is not None and values is not None: return ConfigValueTypeError( diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/build/test_model_config_engines.py b/tests/e2e/src/sqlbuild/cli/commands/main/build/test_model_config_engines.py index f124f6dd00..d3e3ae5bfe 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/build/test_model_config_engines.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/build/test_model_config_engines.py @@ -1,8 +1,8 @@ """E2E: layered, templated and validated model config builds the same on every compiler engine. -The native and preview engines build model config (path defaults, templates, storage policies) and checks the -model validators natively; a validator rejection re-runs Python for the exact error. Every engine -must build the same relations and report the same validator errors. +The native and preview engines build model config (path defaults, templates, storage policies) +and check the model validators natively with Python's exact errors; inputs only Python can judge +run in Python. Every engine must build the same relations and report the same validator errors. """ from __future__ import annotations diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index a1f174b8ff..c0587a725b 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -943,6 +943,35 @@ class ReferenceScanErrorTestCase: expected_error_types: tuple[str, ...] +@dataclass(frozen=True) +class NativeMacroErrorTestCase: + """A failing macro project every engine must report identically, running each call once.""" + + description: str + project_files: dict[str, str] + expected_report_text: str + expected_macro_runs: list[str] + + +@dataclass(frozen=True) +class UnkeyableMacroCallTestCase: + """Compile arguments whose values no store key can hold; every engine must match Python.""" + + description: str + project_files: dict[str, str] + compile_args: tuple[str, ...] + expected_returncodes: tuple[int, ...] + + +@dataclass(frozen=True) +class MacroErrorLifecycleTestCase: + """A failing macro project whose invocation lifecycle every engine reports the same way.""" + + description: str + project_files: dict[str, str] + expected_error_types: tuple[str, ...] + + @dataclass(frozen=True) class NativeConfigErrorTestCase: """A failing project whose model config error both engines must report the same way.""" @@ -956,7 +985,7 @@ class NativeConfigErrorTestCase: @dataclass(frozen=True) class MacroExpandedValidatorErrorTestCase: - """A validator error from macro-expanded SQL that the preview bridge must re-run.""" + """A validator error from macro-expanded SQL that every engine reports, running macros once.""" description: str project_files: dict[str, str] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py index 9ffdf0f4cf..9839838436 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py @@ -1,4 +1,4 @@ -"""Every engine reports attachment errors identically; bridge-independent ones never re-run.""" +"""Every engine reports attachment errors identically and runs each macro call once.""" from __future__ import annotations @@ -56,9 +56,9 @@ expected_error_types=_ERROR_TYPES, ), NativeAttachmentErrorTestCase( - description="a cursor intrinsic in a generic audit re-runs as it reads expanded SQL", + description="a cursor intrinsic in a generic audit reads expanded SQL", case_name="generic-audit-cursor-intrinsic", - expected_macro_calls=(1, 2, 2), + expected_macro_calls=(1, 1, 1), expected_error_types=_ERROR_TYPES, ), NativeAttachmentErrorTestCase( diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py index 4f7c22fc3b..3e227f83bd 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py @@ -1,4 +1,4 @@ -"""Every engine reports SQL test, scenario and relationship errors identically, without re-runs.""" +"""Every engine reports SQL test, scenario and relationship errors identically.""" from __future__ import annotations diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_macro_call_store.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_macro_call_store.py index 2d610f33f0..544ed4fc50 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_macro_call_store.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_macro_call_store.py @@ -2,11 +2,13 @@ from __future__ import annotations +import shutil import sys from pathlib import Path import pytest +from scripts.compiler_differential.constants import FAILURE_BASE_FILES from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( BrokenMacroCallStoreKeyTestCase, EngineMacroCallGateTestCase, @@ -16,6 +18,7 @@ SecondCompileStoreTestCase, StaleMacroModuleStoreTestCase, StaleStoreArrangement, + UnkeyableMacroCallTestCase, ) from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import ( MACRO_CALL_STORE_ENGINE, @@ -45,6 +48,7 @@ pretend_fresh_process, recompile_in_process_after_edit, replace_project_text, + report_without_engine, rezip_flavor_between_processes, run_reuse_compile, store_files, @@ -181,6 +185,18 @@ ) +_TAGGED_STAGING: dict[str, str] = { + "models/staging/_sqlbuild/_macros/tags.py": ( + 'def tagged(ctx, expr: str) -> str:\n """Tag."""\n return expr\n' + ), + "models/staging/stg_orders.sql": ( + 'MODEL (\n description "Staged orders",\n);\n\n' + "SELECT order_id, customer_id, @tagged('amount') AS amount, status\n" + 'FROM __source("raw_orders")\n' + ), +} + + @pytest.mark.parametrize( "test_case", [ @@ -493,3 +509,41 @@ def test_given_no_proc_filesystem_when_compiling_repeatedly_then_the_store_still assert runs.returncodes == (0, 0) assert runs.logged_calls == test_case.expected_logged_calls assert runs.store_files == test_case.expected_store_files + + +@pytest.mark.parametrize( + "test_case", + [ + UnkeyableMacroCallTestCase( + description="context macro with a lone surrogate var", + project_files=_TAGGED_STAGING, + compile_args=("--vars", '{"regions": ["\\udcff"]}'), + expected_returncodes=(0, 0, 0), + ), + ], + ids=lambda case: case.description, +) +def test_given_var_no_store_key_can_hold_when_compiling_then_every_engine_matches_python( + test_case: UnkeyableMacroCallTestCase, tmp_path: Path +) -> None: + project_dir: Path = tmp_path / "orders" + runs: list[CompileReuseRun] = [] + for engine in ("python", "native", "native-preview"): + shutil.rmtree(project_dir, ignore_errors=True) + for relative_path, contents in {**FAILURE_BASE_FILES, **test_case.project_files}.items(): + write_project_file(project_dir, relative_path, contents) + runs.append( + run_reuse_compile( + project_dir=project_dir, + args=test_case.compile_args, + global_args=("--compiler-engine", engine), + ) + ) + + assert ( + tuple(run.returncode for run in runs), + {report_without_engine(run) for run in runs} == {report_without_engine(runs[0])}, + [run.compiled for run in runs] == [runs[0].compiled] * len(runs), + ) == (test_case.expected_returncodes, True, True), tuple( + run.report + run.stderr for run in runs + ) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_config_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_config_errors.py index 3a2a19e347..e3b74d071c 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_config_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_config_errors.py @@ -73,7 +73,7 @@ + _STAGING_BODY }, expected_report_text="unknown incremental_strategy 'upsert'", - expected_macro_calls=[1, 2], + expected_macro_calls=[1, 1], expected_python_fallbacks=[], ), NativeConfigErrorTestCase( @@ -85,7 +85,7 @@ + _STAGING_BODY }, expected_report_text="cursor_start must be before exclusive cursor_end", - expected_macro_calls=[1, 2], + expected_macro_calls=[1, 1], expected_python_fallbacks=[], ), NativeConfigErrorTestCase( @@ -127,7 +127,7 @@ ], ids=lambda case: case.description, ) -def test_given_config_error_when_compiling_with_preview_then_error_matches_python_once( +def test_given_config_error_when_compiling_with_preview_then_error_matches_python( test_case: NativeConfigErrorTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -195,12 +195,12 @@ def test_given_config_error_when_compiling_with_preview_then_error_matches_pytho }, engines=("python", "native-preview", "native"), expected_report_text="references unknown model 'ghost'", - expected_macro_calls=[1, 2, 2], + expected_macro_calls=[1, 1, 1], ) ], ids=lambda case: case.description, ) -def test_given_validator_error_from_macro_sql_when_compiling_then_preview_bridge_reruns_it( +def test_given_validator_error_from_macro_sql_when_compiling_then_every_engine_runs_macros_once( test_case: MacroExpandedValidatorErrorTestCase, tmp_path: Path ) -> None: project_dir: Path = tmp_path / "orders" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_macro_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_macro_errors.py new file mode 100644 index 0000000000..eec6cd6f8e --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_macro_errors.py @@ -0,0 +1,215 @@ +"""Every engine reports macro errors as Python does and runs each macro call once.""" + +from __future__ import annotations + +import shutil +from pathlib import Path + +import pytest + +from scripts.compiler_differential.constants import FAILURE_BASE_FILES, FAILURE_BASE_MART +from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + MacroErrorLifecycleTestCase, + NativeMacroErrorTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import ( + CompileReuseRun, + lifecycle_error_type, + report_without_engine, + run_reuse_compile, + stderr_without_durations, + write_project_file, +) + +_ENGINES: tuple[str, ...] = ("python", "native", "native-preview") +_CALL_LOG: str = "macro_calls.log" +_MACROS: str = ( + "from pathlib import Path\n\n\n" + "def _log(name: str) -> None:\n" + f" with (Path(__file__).resolve().parents[1] / {_CALL_LOG!r}).open(\n" + ' "a", encoding="utf-8"\n' + " ) as log:\n" + ' _ = log.write(f"{name}\\n")\n\n\n' + "def counted(expression: str) -> str:\n" + ' """Return the expression, logging each execution."""\n' + ' _log(f"counted {expression}")\n' + " return expression\n\n\n" + "def explode(expression: str) -> str:\n" + ' """Fail after logging the call."""\n' + ' _log(f"explode {expression}")\n' + ' raise ValueError(f"cannot render {expression}")\n\n\n' + "def answer() -> int:\n" + ' """Return a value that is not SQL."""\n' + ' _log("answer")\n' + " return 42\n\n\n" + "def upstream() -> str:\n" + ' """Read a model that does not exist."""\n' + ' _log("upstream")\n' + " return '__ref(\"ghost\")'\n" +) +_MART_MACROS: str = ( + "def mart_only(expression: str) -> str:\n" + ' """Qualify an expression for marts."""\n' + " return expression\n" +) +_CYCLIC_MACROS: str = ( + 'def ping(expression: str) -> str:\n """Call pong."""\n return pong(expression)\n\n\n' + 'def pong(expression: str) -> str:\n """Call ping."""\n return ping(expression)\n' +) +_COUNTED_MART: str = FAILURE_BASE_MART.replace("SUM(amount)", "SUM(@counted('amount'))") +_STAGING_PATH: str = "models/staging/stg_orders.sql" +_MART_PATH: str = "models/marts/customer_totals.sql" +_STAGING_PREFIX: str = ( + 'MODEL (\n description "Staged orders",\n);\n\nSELECT order_id, customer_id, ' +) +_STAGING_SUFFIX: str = ' AS amount, status\nFROM __source("raw_orders")\n' + + +@pytest.mark.parametrize( + "test_case", + [ + NativeMacroErrorTestCase( + description="macro_raises_after_a_side_effect", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@explode('amount')" + _STAGING_SUFFIX}, + expected_report_text="stg_orders.sql' failed: cannot render amount", + expected_macro_runs=["counted amount", "explode amount"], + ), + NativeMacroErrorTestCase( + description="macro_returns_a_value_that_is_not_sql", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@answer()" + _STAGING_SUFFIX}, + expected_report_text="must return a SQL string when used directly in SQL", + expected_macro_runs=["counted amount", "answer"], + ), + NativeMacroErrorTestCase( + description="macro_arguments_do_not_parse", + project_files={ + _STAGING_PATH: _STAGING_PREFIX + "@counted('amount',,)" + _STAGING_SUFFIX + }, + expected_report_text="could not be parsed", + expected_macro_runs=["counted amount"], + ), + NativeMacroErrorTestCase( + description="unknown_macro", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@missing('amount')" + _STAGING_SUFFIX}, + expected_report_text="Unknown macro '@missing'", + expected_macro_runs=["counted amount"], + ), + NativeMacroErrorTestCase( + description="private_macro_helper", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@_log('amount')" + _STAGING_SUFFIX}, + expected_report_text="Unknown macro '@_log'", + expected_macro_runs=["counted amount"], + ), + NativeMacroErrorTestCase( + description="macro_in_a_sibling_scope", + project_files={ + _STAGING_PATH: _STAGING_PREFIX + "@mart_only('amount')" + _STAGING_SUFFIX, + "models/marts/_sqlbuild/macros/local.py": _MART_MACROS, + }, + expected_report_text="stg_orders.sql' is inaccessible", + expected_macro_runs=["counted amount"], + ), + NativeMacroErrorTestCase( + description="macro_call_cycle", + project_files={"macros/cyclic.py": _CYCLIC_MACROS}, + expected_report_text="Macro call cycle in 'macros/cyclic.py': ping -> pong -> ping", + expected_macro_runs=[], + ), + NativeMacroErrorTestCase( + description="first_of_two_failing_models", + project_files={ + _STAGING_PATH: _STAGING_PREFIX + "@explode('amount')" + _STAGING_SUFFIX, + _MART_PATH: FAILURE_BASE_MART.replace("SUM(amount)", "SUM(@explode('total'))"), + }, + expected_report_text="customer_totals.sql' failed: cannot render total", + expected_macro_runs=["explode total"], + ), + NativeMacroErrorTestCase( + description="reference_error_in_macro_sql", + project_files={ + _STAGING_PATH: _STAGING_PREFIX + + "amount FROM @upstream() UNION ALL SELECT 1, 2, amount" + + _STAGING_SUFFIX + }, + expected_report_text="references unknown model 'ghost'", + expected_macro_runs=["counted amount", "upstream"], + ), + ], + ids=lambda case: case.description, +) +def test_given_macro_error_when_compiling_with_each_engine_then_error_matches_python_once( + test_case: NativeMacroErrorTestCase, tmp_path: Path +) -> None: + project_dir: Path = tmp_path / "orders" + runs: list[CompileReuseRun] = [] + macro_runs: list[list[str]] = [] + for engine in _ENGINES: + shutil.rmtree(project_dir, ignore_errors=True) + for relative_path, contents in { + **FAILURE_BASE_FILES, + "macros/orders.py": _MACROS, + _MART_PATH: _COUNTED_MART, + _CALL_LOG: "", + **test_case.project_files, + }.items(): + write_project_file(project_dir, relative_path, contents) + runs.append( + run_reuse_compile(project_dir=project_dir, global_args=("--compiler-engine", engine)) + ) + macro_runs.append((project_dir / _CALL_LOG).read_text(encoding="utf-8").splitlines()) + + assert ( + tuple(run.returncode for run in runs), + {report_without_engine(run) for run in runs} == {report_without_engine(runs[0])}, + {stderr_without_durations(stderr=run.stderr) for run in runs} + == {stderr_without_durations(stderr=runs[0].stderr)}, + test_case.expected_report_text in runs[0].report + runs[0].stderr, + macro_runs, + ) == ( + (1,) * len(_ENGINES), + True, + True, + True, + [test_case.expected_macro_runs] * len(_ENGINES), + ), tuple(run.report + run.stderr for run in runs) + + +@pytest.mark.parametrize( + "test_case", + [ + MacroErrorLifecycleTestCase( + description="macro_raises_after_a_side_effect", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@explode('amount')" + _STAGING_SUFFIX}, + expected_error_types=("CompileInputError",) * 3, + ), + MacroErrorLifecycleTestCase( + description="unknown_macro", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@missing('amount')" + _STAGING_SUFFIX}, + expected_error_types=("CompileInputError",) * 3, + ), + ], + ids=lambda case: case.description, +) +def test_given_macro_error_when_compiling_with_each_engine_then_lifecycle_error_type_matches( + test_case: MacroErrorLifecycleTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + error_types: list[str] = [] + for engine in _ENGINES: + project_dir: Path = tmp_path / engine + for relative_path, contents in { + **FAILURE_BASE_FILES, + "macros/orders.py": _MACROS, + _MART_PATH: _COUNTED_MART, + _CALL_LOG: "", + **test_case.project_files, + }.items(): + write_project_file(project_dir, relative_path, contents) + error_types.append( + lifecycle_error_type(project_dir=project_dir, engine=engine, monkeypatch=monkeypatch) + ) + + assert tuple(error_types) == test_case.expected_error_types, test_case.description + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py index c58998d27b..97e099d6ec 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py @@ -104,11 +104,11 @@ expected_message=( f"{FAILURE_MART_PATH}: SQL reference contains an unclosed parenthesis" ), - expected_macro_calls=(1, 2, 2), + expected_macro_calls=(1, 1, 1), expected_error_types=("CompileInputError",) * 3, ), ReferenceScanErrorTestCase( - description="authored_fault_after_a_macro_is_located_without_a_python_rerun", + description="authored_fault_after_a_macro_is_located_natively", project_files={ "macros/emit.py": _EMIT_MACRO, FAILURE_MART_PATH: _MART_HEADER + "SELECT customer_id, @emit('1') AS total_amount\n" diff --git a/tests/integration/src/sqlbuild/compiler/attachments/test_native_attachment_project.py b/tests/integration/src/sqlbuild/compiler/attachments/test_native_attachment_project.py index 41bc763176..1dd66a3c41 100644 --- a/tests/integration/src/sqlbuild/compiler/attachments/test_native_attachment_project.py +++ b/tests/integration/src/sqlbuild/compiler/attachments/test_native_attachment_project.py @@ -102,7 +102,7 @@ + '__expected__orders AS (SELECT id, 1.5 AS amount, label FROM __source("order_events")),\n' + "__assert__no_rows AS (SELECT 1 FROM __source(order_events))\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="error: SQL scenario file tests/scenarios/orders_scenario.sql CTE " "'__expected__orders' must not reference project source", ), @@ -114,7 +114,7 @@ + "__seed__channel_codes AS (SELECT 1 AS id, 'web' AS label),\n" + "__expected__orders AS (SELECT 1 AS id FROM __source(order_events))\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="error: SQL scenario file tests/scenarios/orders_scenario.sql " "references unknown source 'missing_events'", ), @@ -126,7 +126,7 @@ + "__seed__channel_codes AS (SELECT 1 AS id FROM __source(order_events)),\n" + "__expected__orders AS (SELECT 1 AS id, 1.5 AS amount, 'web' AS label)\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="error: SQL scenario file tests/scenarios/orders_scenario.sql " "references unknown source 'missing_events'", ), @@ -137,7 +137,7 @@ + "WITH\n__seed__ AS (SELECT 1 AS id, 'web' AS label),\n" + "__expected__orders AS (SELECT 1 AS id, 1.5 AS amount, 'web' AS label)\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="error: SQL scenario 'tests/scenarios/orders_scenario.sql' " "must use __seed__ to identify a target", ), @@ -149,7 +149,7 @@ + "__macro__tidy AS (SELECT 'x'),\n" + "__expected__orders AS (SELECT 1 AS id, 1.5 AS amount, 'web' AS label)\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="does not support macro mock CTE '__macro__tidy'", ), AttachmentProjectTestCase( @@ -161,7 +161,7 @@ + "__expected__orders AS (SELECT 1 AS id, 1.5 AS amount, 'web' AS label " + "FROM __assert__no_rows)\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="check CTE '__expected__orders' must not depend on " "'__assert__no_rows'", ), diff --git a/tests/integration/src/sqlbuild/compiler/compile/_test_types.py b/tests/integration/src/sqlbuild/compiler/compile/_test_types.py index 2b8f407f13..2267a116aa 100644 --- a/tests/integration/src/sqlbuild/compiler/compile/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/compile/_test_types.py @@ -40,3 +40,18 @@ class MacroExpansionDifferentialTestCase: samples: int expected_minimum_successes: int expected_mismatches: list[tuple[object, object, object]] + + +@dataclass(frozen=True) +class FailingMacroProjectParityTestCase: + """Seeded projects whose failing macros every engine must report identically, running once.""" + + description: str + seed: int + projects: int + engines: tuple[str, ...] + expected_minimum_failures: int + expected_minimum_successes: int + expected_minimum_errors_after_macro_runs: int + expected_minimum_distinct_errors: int + expected_mismatches: list[tuple[object, object, object]] diff --git a/tests/integration/src/sqlbuild/compiler/compile/helpers.py b/tests/integration/src/sqlbuild/compiler/compile/helpers.py index fe10c48a49..eeb2dbc841 100644 --- a/tests/integration/src/sqlbuild/compiler/compile/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/compile/helpers.py @@ -6,8 +6,11 @@ import re from collections.abc import Callable from contextvars import Token +from dataclasses import dataclass from functools import partial +from itertools import compress from pathlib import Path +from typing import cast import pytest @@ -39,6 +42,7 @@ from sqlbuild.spec.contracts.main.resolve_effective_collection_rendering import ( resolve_effective_collection_rendering, ) +from tests.integration.src.sqlbuild.compiler.helpers import mismatches MACRO_CALL_LOG_ENV_VAR: str = "SQLBUILD_TEST_MACRO_CALL_LOG" _RUN_ID_PATTERN: re.Pattern[str] = re.compile(r'"run_id": "[^"]*"') @@ -47,7 +51,8 @@ "@n(@mm(3), '@m(4)')", "@tag(__ref('orders'))", "@tag(__seed('rates'))", "@tag(__ref('customers'))", "@cols(__ref('orders'))", "@gen()", "@nope(1)", "@m(x)", "@ab c(1)", "@m('é')", "'@m(1)'", "-- @m(1)\n", "/* @n() */", "$$@m()$$", "`@m()`", "@@v", - "@", " ", "\n", "\u3000", "é", "'", "(", ")", "SELECT ", ", ", "\"q\"", + "@", " ", "\n", "\u3000", "é", "'", "(", ")", "SELECT ", ", ", "\"q\"", "'\udcff'", + "@m('\udcff')", "@sur()", ) # fmt: skip _DIFFERENTIAL_FILE: Path = Path("models/orders_summary.sql") @@ -290,6 +295,10 @@ def _cols(relation: object) -> str: return f"SELECT * FROM {relation}" +def _surrogate() -> str: + return "x\udcff" + + def _generated() -> str: return '(SELECT id FROM __ref("orders"))' @@ -310,6 +319,7 @@ def _loaded_macro(*, name: str, function: Callable[..., object]) -> LoadedMacro: ("tag", _tag), ("cols", _cols), ("gen", _generated), + ("sur", _surrogate), ) } _DIFFERENTIAL_CONTEXT: MacroContext = MacroContext( @@ -318,7 +328,7 @@ def _loaded_macro(*, name: str, function: Callable[..., object]) -> LoadedMacro: class ExpansionFailureCapture: - """Swallow and keep the compile input error a block raises.""" + """Swallow and keep the error a block raises.""" def __init__(self) -> None: self.failure: BaseException | None = None @@ -328,7 +338,7 @@ def __enter__(self) -> ExpansionFailureCapture: def __exit__(self, error_type: object, error: BaseException | None, traceback: object) -> bool: self.failure = error - return isinstance(error, CompileInputError) + return isinstance(error, Exception) def random_macro_sql(*, rng: random.Random) -> str: @@ -363,3 +373,187 @@ def expansion_outcome(*, sql: str, bridge: MacroBridge | None) -> tuple[object, True: rendered[0], False: (type(capture.failure).__name__, str(capture.failure)), }[capture.failure is None] + + +_FAILING_MACRO_LOG: str = ( + "def _log(value: object) -> None:\n" + f' path = os.environ.get("{MACRO_CALL_LOG_ENV_VAR}")\n' + " if path:\n" + ' with open(path, "a", encoding="utf-8") as log:\n' + ' log.write(f"{value}\\n")\n\n\n' +) +_FAILING_MACROS: str = ( + '"""Macros that render, raise after a side effect or return values that are not SQL."""\n\n' + "import os\n\n\n" + _FAILING_MACRO_LOG + "def cents(column: str) -> str:\n" + ' """Convert an amount to cents."""\n' + " _log(column)\n" + ' return f"{column} * 100"\n\n\n' + "def wrap(expression: str) -> str:\n" + ' """Parenthesise an expression."""\n' + ' _log("wrap " + expression)\n' + ' return f"({expression})"\n\n\n' + "def explode(column: str) -> str:\n" + ' """Fail after logging the call."""\n' + " _log(column)\n" + ' raise ValueError(f"cannot render {column}")\n\n\n' + "def number(column: str) -> int:\n" + ' """Return a value that is not SQL."""\n' + " _log(column)\n" + " return 1\n\n\n" + "def _hidden(column: str) -> str:\n" + " return column\n" +) +_LOCAL_MACROS: str = ( + '"""Macros only the marts models can see."""\n\n' + "import os\n\n\n" + _FAILING_MACRO_LOG + "def mart_only(column: str) -> str:\n" + ' """Qualify a column for marts."""\n' + " _log(column)\n" + ' return f"marts_{column}"\n' +) +_CYCLIC_MACROS: str = ( + '"""Macros that call each other forever."""\n\n\n' + "def ping(column: str) -> str:\n" + ' """Call pong."""\n' + " return pong(column)\n\n\n" + "def pong(column: str) -> str:\n" + ' """Call ping."""\n' + " return ping(column)\n" +) +_FAILING_CALLS: tuple[str, ...] = ( + *("@cents('{arg}')",) * 24, *("@wrap(@cents('{arg}'))",) * 6, "@mart_only('{arg}')", + "@mart_only('{arg}')", "@explode('{arg}')", + "@wrap(@explode('{arg}'))", "@number('{arg}')", "@cents('{arg}', 'extra')", + "@cents('{arg}',,)", "@cents({arg})", "@nope('{arg}')", "@_hidden('{arg}')", + "@cents('{arg}'", "@cents x('{arg}')", +) # fmt: skip +_MODEL_FOLDERS: tuple[str, ...] = ("staging", "marts") +_ERROR_SPECIFICS: re.Pattern[str] = re.compile(r"'[^']*'|c\d+_\d+") +_CYCLE_CHANCE: tuple[bool, ...] = (False,) * 11 + (True,) + + +def generated_failing_macro_project(*, rng: random.Random) -> dict[str, str]: + """Return a project whose models call macros that may raise, mistype or be unreachable.""" + + models: dict[str, str] = dict( + _generated_model(rng=rng, model=model) for model in range(rng.randint(2, 6)) + ) + cyclic: dict[str, str] = {True: {"macros/cyclic.py": _CYCLIC_MACROS}, False: {}}[ + rng.choice(_CYCLE_CHANCE) + ] + return { + "sqlbuild_project.toml": MACRO_BRIDGE_PROJECT_FILES["sqlbuild_project.toml"], + "macros/common.py": _FAILING_MACROS, + "models/marts/_sqlbuild/macros/local.py": _LOCAL_MACROS, + **cyclic, + **models, + } + + +def _generated_model(*, rng: random.Random, model: int) -> tuple[str, str]: + calls: list[str] = [ + f"{rng.choice(_FAILING_CALLS).format(arg=f'c{model}_{call}')} AS c{call}" + for call in range(rng.randint(1, 3)) + ] + return ( + f"models/{rng.choice(_MODEL_FOLDERS)}/m{model}.sql", + f'MODEL (description "Generated {model}");\n\nSELECT {", ".join(calls)}\n', + ) + + +@dataclass(frozen=True) +class FailingMacroParity: + """How each engine's render of generated failing-macro projects compared with Python's.""" + + mismatches: list[list[tuple[object, object, object]]] + failures: int + successes: int + failures_after_macro_runs: int + distinct_errors: list[str] + + +def failing_macro_parity( + *, + projects: list[dict[str, str]], + root: Path, + engines: tuple[str, ...], + monkeypatch: pytest.MonkeyPatch, +) -> FailingMacroParity: + """Render every project under each engine; compare errors and macro runs with the first.""" + + project_dirs: list[Path] = [ + write_project(root=root / f"project_{index}", files=files) + for index, files in enumerate(projects) + ] + outcomes: dict[str, list[object]] = { + engine: _engine_outcomes( + project_dirs=project_dirs, + engine=engine, + log_path=root / f"{engine}.log", + monkeypatch=monkeypatch, + ) + for engine in engines + } + python_outcomes: list[object] = outcomes[engines[0]] + failed: list[bool] = [_is_failure(outcome) for outcome in python_outcomes] + failures: list[tuple[tuple[object, ...], tuple[str, ...]]] = cast( + list[tuple[tuple[object, ...], tuple[str, ...]]], list(compress(python_outcomes, failed)) + ) + return FailingMacroParity( + mismatches=[ + mismatches( + inputs=list[object](project_dirs), + expected=python_outcomes, + actual=outcomes[engine], + ) + for engine in engines[1:] + ], + failures=len(failures), + successes=len(python_outcomes) - len(failures), + failures_after_macro_runs=sum(bool(runs) for _, runs in failures), + distinct_errors=sorted( + {_ERROR_SPECIFICS.sub("''", str(error[1])) for error, _ in failures} + ), + ) + + +def _engine_outcomes( + *, project_dirs: list[Path], engine: str, log_path: Path, monkeypatch: pytest.MonkeyPatch +) -> list[object]: + return [ + engine_render_outcome( + project_dir=project_dir, engine=engine, log_path=log_path, monkeypatch=monkeypatch + ) + for project_dir in project_dirs + ] + + +def _is_failure(outcome: object) -> bool: + return isinstance(cast(tuple[object, ...], outcome)[0], tuple) + + +def engine_render_outcome( + *, project_dir: Path, engine: str, log_path: Path, monkeypatch: pytest.MonkeyPatch +) -> tuple[object, tuple[str, ...]]: + """Render under `engine`; return the error's full shape or the capture, and macro runs.""" + + _ = log_path.write_text("", encoding="utf-8") + monkeypatch.setenv(MACRO_CALL_LOG_ENV_VAR, str(log_path)) + capture: ExpansionFailureCapture = ExpansionFailureCapture() + rendered: list[object] = [None] + with capture: + rendered[0] = comparable_capture( + render_compile_inputs(project_dir=project_dir, engine=engine, monkeypatch=monkeypatch) + ) + failure: BaseException | None = capture.failure + outcome: object = { + True: rendered[0], + False: ( + type(failure).__name__, + str(failure), + getattr(failure, "code", None), + getattr(failure, "help", None), + type(getattr(failure, "__cause__", None)).__name__, + str(getattr(failure, "__cause__", None)), + ), + }[failure is None] + return outcome, tuple(log_path.read_text(encoding="utf-8").splitlines()) diff --git a/tests/integration/src/sqlbuild/compiler/compile/test_native_macro_bridge.py b/tests/integration/src/sqlbuild/compiler/compile/test_native_macro_bridge.py index 2650ab34a3..cb5135b328 100644 --- a/tests/integration/src/sqlbuild/compiler/compile/test_native_macro_bridge.py +++ b/tests/integration/src/sqlbuild/compiler/compile/test_native_macro_bridge.py @@ -11,6 +11,7 @@ from sqlbuild.compiler.macro_bridge.classes.macro_bridge import MacroBridge from tests.integration.src.sqlbuild.compiler.compile._test_types import ( + FailingMacroProjectParityTestCase, MacroBridgeFailureTestCase, MacroBridgeMemoTestCase, MacroBridgeParityTestCase, @@ -19,8 +20,11 @@ from tests.integration.src.sqlbuild.compiler.compile.helpers import ( MACRO_BRIDGE_PROJECT_FILES, MACRO_CALL_LOG_ENV_VAR, + FailingMacroParity, comparable_capture, expansion_outcome, + failing_macro_parity, + generated_failing_macro_project, random_macro_sql, render_compile_inputs, render_error, @@ -224,5 +228,45 @@ def test_given_random_macro_sql_when_expanding_through_bridge_then_matches_pytho assert bridge.stats()[0] > 0 +@pytest.mark.parametrize( + "test_case", + [ + FailingMacroProjectParityTestCase( + description="seeded projects with raising, mistyped, unparsable and unreachable macros", + seed=1043, + projects=150, + engines=("python", "native", "native-preview"), + expected_minimum_failures=100, + expected_minimum_successes=10, + expected_minimum_errors_after_macro_runs=80, + expected_minimum_distinct_errors=10, + expected_mismatches=[], + ), + ], + ids=lambda case: case.description, +) +def test_given_failing_macro_projects_when_rendering_then_every_engine_raises_python_error_once( + test_case: FailingMacroProjectParityTestCase, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + rng: random.Random = random.Random(test_case.seed) + + parity: FailingMacroParity = failing_macro_parity( + projects=[generated_failing_macro_project(rng=rng) for _ in range(test_case.projects)], + root=tmp_path, + engines=test_case.engines, + monkeypatch=monkeypatch, + ) + + assert parity.mismatches == [test_case.expected_mismatches] * (len(test_case.engines) - 1) + assert ( + parity.failures >= test_case.expected_minimum_failures, + parity.successes >= test_case.expected_minimum_successes, + parity.failures_after_macro_runs >= test_case.expected_minimum_errors_after_macro_runs, + len(parity.distinct_errors) >= test_case.expected_minimum_distinct_errors, + ) == (True, True, True, True), parity + + if __name__ == "__main__": pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/helpers.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/helpers.py index 3058392ef8..acb50cbbe0 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/helpers.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/helpers.py @@ -196,3 +196,12 @@ def environment_of(root: Path) -> str: project_dir=root, fingerprint=project_fingerprint(project_dir=root, model_paths=EXCLUDED_MODEL_PATHS), ) + + +def deeply_nested_list(*, depth: int) -> list[object]: + """Return a list nested `depth` levels deep.""" + + nested: list[object] = [] + for _ in range(depth): + nested = [nested] + return nested diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/test_store_keys.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/test_store_keys.py index e32305902d..41d079e5b6 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/test_store_keys.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/test_store_keys.py @@ -21,6 +21,7 @@ from tests.unit.src.sqlbuild.compiler.macro_bridge._helpers.helpers import ( BASE_CONTEXT, constant_declarations, + deeply_nested_list, loaded_macro, with_vars, ) @@ -113,6 +114,16 @@ def test_given_two_contexts_when_naming_their_store_class_then_only_equal_values context=with_vars(region={"key": [object()]}), expected_stored=False, ), + ContextValueTestCase( + description="lone surrogate var", + context=with_vars(region="\udcff"), + expected_stored=False, + ), + ContextValueTestCase( + description="var nested beyond the recursion limit", + context=with_vars(region=deeply_nested_list(depth=100_000)), + expected_stored=False, + ), ], ids=lambda case: case.description, ) @@ -151,14 +162,14 @@ def test_given_context_value_when_naming_store_class_then_only_stable_values_are def test_given_loaded_macro_change_when_naming_it_then_only_the_same_file_and_source_match( test_case: MacroStoreTokenTestCase, ) -> None: - original: str = macro_store_token( + original: str | None = macro_store_token( loaded=loaded_macro( relative_path=Path("macros/common.py"), raw_source="def cents(column): ...\n" ), identity=None, ) - changed: str = macro_store_token( + changed: str | None = macro_store_token( loaded=loaded_macro(relative_path=test_case.relative_path, raw_source=test_case.raw_source), identity=None, ) @@ -196,11 +207,11 @@ def test_given_loaded_macro_change_when_naming_it_then_only_the_same_file_and_so def test_given_call_class_inputs_when_naming_it_then_only_identical_inputs_match( test_case: CallClassStoreTextTestCase, ) -> None: - plain: str = call_class_store_text( + plain: str | None = call_class_store_text( macro_tokens=("cents",), context_token=None, prior_relations=None ) - text: str = call_class_store_text( + text: str | None = call_class_store_text( macro_tokens=("cents",), context_token=test_case.context_token, prior_relations=test_case.prior_relations, diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/_test_types.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/_test_types.py index 699b51074f..9dae9cd388 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/_test_types.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/_test_types.py @@ -10,8 +10,7 @@ class MacroBridgeFailureTestCase: description: str stage_with_bridge: Callable[[], str] - stage_without_bridge: Callable[[], str] - expected_error: type[Exception] + expected_error_message: str expected_bridged_runs: list[bool] @@ -22,12 +21,3 @@ class MacroBridgeSuccessTestCase: description: str expected_result: str expected_bridged_runs: list[bool] - - -@dataclass(frozen=True) -class MacroBridgeErrorContextTestCase: - """A stage whose Python error must not chain the bridge failure as its context.""" - - description: str - stage: Callable[[], str] - expected_context: BaseException | None diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py index b9d597ef70..401f3038df 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py @@ -6,6 +6,7 @@ from typing import cast from sqlbuild.compiler.compile.exceptions import CompileInputError +from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.macro_bridge.classes.macro_bridge import MacroBridge from sqlbuild.compiler.macro_bridge.main.active_macro_bridge import active_macro_bridge @@ -29,13 +30,24 @@ def failed_after_scan() -> str: raise CompileInputError("render failed") -def declaration_failed_after_scan() -> str: - """Fail on an `@enum` reference after the active bridge scanned a macro call.""" +def mismatched_after_scan() -> str: + """Report a native scan mismatch after the active bridge scanned a macro call.""" _scan_with_active_bridge() - raise CompileInputError( - "Unknown enum 'order_status' in 'models/orders.sql'", bridge_independent=True - ) + raise NativeStageMismatchError("native scan ended early") + + +def recording_failure(*, stage: Callable[[], str], errors: list[Exception]) -> Callable[[], str]: + """Return `stage`, recording each error it raises before it propagates.""" + + def recorded() -> str: + try: + return stage() + except Exception as error: + errors.append(error) + raise + + return recorded def _scan_with_active_bridge() -> None: diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py index 81aa6d6c78..cb996b6853 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py @@ -4,19 +4,17 @@ import pytest -from sqlbuild.compiler.compile.exceptions import CompileInputError -from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.macro_bridge.main.active_macro_bridge import active_macro_bridge from sqlbuild.compiler.macro_bridge.main.run_with_macro_bridge import run_with_macro_bridge from tests.unit.src.sqlbuild.compiler.macro_bridge.main._test_types import ( - MacroBridgeErrorContextTestCase, MacroBridgeFailureTestCase, MacroBridgeSuccessTestCase, ) from tests.unit.src.sqlbuild.compiler.macro_bridge.main.helpers import ( - declaration_failed_after_scan, failed, failed_after_scan, + mismatched_after_scan, + recording_failure, recording_stage, rendered, ) @@ -38,9 +36,7 @@ def test_given_succeeding_stage_when_running_with_macro_bridge_then_runs_once_br ) -> None: runs: list[bool] = [] - result: str = run_with_macro_bridge( - stage=recording_stage(runs=runs, stages={True: rendered, False: failed}) - ) + result: str = run_with_macro_bridge(stage=recording_stage(runs=runs, stages={True: rendered})) assert result == test_case.expected_result assert runs == test_case.expected_bridged_runs @@ -51,70 +47,45 @@ def test_given_succeeding_stage_when_running_with_macro_bridge_then_runs_once_br "test_case", [ MacroBridgeFailureTestCase( - description="failure after a scan re-runs Python and raises its error", + description="failure after a scan raises the stage error without a re-run", stage_with_bridge=failed_after_scan, - stage_without_bridge=failed, - expected_error=CompileInputError, - expected_bridged_runs=[True, False], - ), - MacroBridgeFailureTestCase( - description="native-only failure after a scan is reported as a divergence", - stage_with_bridge=failed_after_scan, - stage_without_bridge=rendered, - expected_error=NativeStageMismatchError, - expected_bridged_runs=[True, False], + expected_error_message="render failed", + expected_bridged_runs=[True], ), MacroBridgeFailureTestCase( - description="failure before the bridge scans anything is raised without a re-run", + description="failure before the bridge scans anything raises without a re-run", stage_with_bridge=failed, - stage_without_bridge=rendered, - expected_error=CompileInputError, + expected_error_message="render failed", expected_bridged_runs=[True], ), MacroBridgeFailureTestCase( - description="declaration reference failure after a scan is raised without a re-run", - stage_with_bridge=declaration_failed_after_scan, - stage_without_bridge=rendered, - expected_error=CompileInputError, + description="a bridge mismatch after a scan stays an explicit mismatch", + stage_with_bridge=mismatched_after_scan, + expected_error_message="native scan ended early", expected_bridged_runs=[True], ), ], ids=lambda case: case.description, ) -def test_given_failing_stage_when_running_with_macro_bridge_then_only_bridge_failures_rerun( +def test_given_failing_stage_when_running_with_macro_bridge_then_raises_its_error_once( test_case: MacroBridgeFailureTestCase, ) -> None: runs: list[bool] = [] + raised_errors: list[Exception] = [] stage: Callable[[], str] = recording_stage( runs=runs, - stages={True: test_case.stage_with_bridge, False: test_case.stage_without_bridge}, + stages={True: recording_failure(stage=test_case.stage_with_bridge, errors=raised_errors)}, ) - with pytest.raises(test_case.expected_error): + with pytest.raises(Exception) as raised: _ = run_with_macro_bridge(stage=stage) - assert runs == test_case.expected_bridged_runs - assert active_macro_bridge() is None - - -@pytest.mark.parametrize( - "test_case", - [ - MacroBridgeErrorContextTestCase( - description="python error after a bridge failure", - stage=recording_stage(runs=[], stages={True: failed_after_scan, False: failed}), - expected_context=None, - ), - ], - ids=lambda case: case.description, -) -def test_given_python_error_when_rerun_after_bridge_failure_then_error_has_no_bridge_context( - test_case: MacroBridgeErrorContextTestCase, -) -> None: - with pytest.raises(CompileInputError) as raised: - _ = run_with_macro_bridge(stage=test_case.stage) - - assert raised.value.__context__ is test_case.expected_context + assert (str(raised.value), [raised.value], runs, active_macro_bridge()) == ( + test_case.expected_error_message, + raised_errors, + test_case.expected_bridged_runs, + None, + ) if __name__ == "__main__": diff --git a/website/src/content/docs/docs/concepts/macros.mdx b/website/src/content/docs/docs/concepts/macros.mdx index 24cc009ba3..09e3391d77 100644 --- a/website/src/content/docs/docs/concepts/macros.mdx +++ b/website/src/content/docs/docs/concepts/macros.mdx @@ -215,6 +215,8 @@ SQLBuild relies on this contract to reuse macro results: - [Compile reuse](/docs/cli/compile) replays an unchanged compile without running macros again. - A compile may run each distinct macro call once and reuse its result for identical calls elsewhere in the project. +- A compile that fails stops at its first error. It does not run the project's macros a second + time to report that error. - Macro call results are kept between compiles under `target/cache/` by default, so editing a model does not run its unchanged macro calls again. They are recomputed when macro or other Python files, configuration, declarations, seeds, sources, variables, the target,