From 51b37c4579846928c048921953d1fb7018a25aa7 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 22:59:09 +0100 Subject: [PATCH 1/3] fix: error a unit test when its tested output query fails Storing a unit test's actual output once and comparing against it fell back to the inline comparison on any storage error. On a warehouse that prunes unread columns, the inline comparison of the expected columns can pass when computing the full output fails, for example when a fixture value cannot be converted in a column the expected rows leave out, so the test reported PASS over a warehouse error. When storing fails, the output query now runs on its own; if it fails too, the test errors with the warehouse's message, and only a failure to store a computable output compares inline. --- .../references/docs/concepts/testing.md | 5 +- .../classes/sql_test_actual_outputs.py | 12 +- .../cli/commands/main/test/_test_types.py | 12 ++ .../test/test_output_evaluation_errors.py | 128 ++++++++++++++++++ .../sqlbuild/executor/testing/_test_types.py | 12 ++ .../testing/test_sql_test_execution.py | 61 +++++++++ .../content/docs/docs/concepts/testing.mdx | 5 +- 7 files changed, 232 insertions(+), 3 deletions(-) create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/test/test_output_evaluation_errors.py diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md index 5efe46e116..c0458a6737 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md @@ -919,7 +919,10 @@ SQLBuild drops after the test, including after a failure or timeout. PostgreSQL one transaction with an `ON COMMIT DROP` temporary table that the transaction's end removes, so the reads stay on one server session behind a transaction-mode connection pooler. Snowflake reads the first query's persisted result through `RESULT_SCAN`. If a stored output can no longer be read, -the test falls back to the inline comparison. Other adapters compare the inline query as before. `target/run/tests/` keeps the self-contained inline comparison query for each test. +the test falls back to the inline comparison. If storing an output fails, SQLBuild runs the output +query on its own: when that query fails too, for example because a fixture value cannot be +converted in a column the expected rows leave out, the test errors with the warehouse's message +instead of passing on a comparison of the other columns; otherwise it compares inline. Other adapters compare the inline query as before. `target/run/tests/` keeps the self-contained inline comparison query for each test. ## Skipping SQL analysis diff --git a/src/sqlbuild/executor/testing/classes/sql_test_actual_outputs.py b/src/sqlbuild/executor/testing/classes/sql_test_actual_outputs.py index 1dc461346a..9ac0675c67 100644 --- a/src/sqlbuild/executor/testing/classes/sql_test_actual_outputs.py +++ b/src/sqlbuild/executor/testing/classes/sql_test_actual_outputs.py @@ -50,6 +50,7 @@ def materialize(self) -> dict[int, str]: """Store each compared step's output, or return no relations to compare inline.""" token: str = uuid4().hex[:12] + sql: str | None = None try: for index in range(len(self._test_entry.chain)): (sql,) = render_sql_test_comparison_sql( @@ -75,16 +76,25 @@ def materialize(self) -> dict[int, str]: except (ItemTimeoutError, ExecutionDurationLimitError): raise except Exception as error: + self.release() + self._evaluate_output(sql=sql) log_debug_event( logger=_LOGGER, message="SQL test output could not be stored; comparing inline", test_name=self._test_entry.name, sqlbuild_error=DiagnosticRecordRedactor.text(str(error)), ) - self.release() return {} return dict(self.relations) + def _evaluate_output(self, *, sql: str | None) -> None: + """Run a step's output query alone so its own error fails the test, not a fallback.""" + + if not sql: + return + cursor: Any = self._adapter.execute(connection=self._connection, sql=sql) + _ = cursor.fetchall() + def release(self) -> None: """Drop every stored output; later comparisons run inline.""" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py index a8353abd4d..f9c278b2d2 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py @@ -470,3 +470,15 @@ class TemporaryFunctionConnectionE2ETestCase: description: str expected_exit_code: int expected_stderr_fragment: str + + +@dataclass(frozen=True) +class OutputEvaluationErrorE2ETestCase: + """How a unit test ends when storing its tested model's output fails for a given reason.""" + + description: str + adapter_source: str + expected_exit_code: int + expected_fragments: tuple[str, ...] + expected_json_fragments: tuple[str, ...] + unexpected_fragments: tuple[str, ...] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_output_evaluation_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_output_evaluation_errors.py new file mode 100644 index 0000000000..22e1a56e47 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_output_evaluation_errors.py @@ -0,0 +1,128 @@ +"""E2E coverage: a warehouse error in a tested model's output fails the unit test.""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from typing import Any + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.test._test_types import ( + OutputEvaluationErrorE2ETestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import ( + execute_duckdb, + prepare_inline_project, + run_sqb, +) + +_CONVERSION_ERROR: str = "Numeric value 'cluster::r1' is not recognized" +# Like a warehouse that prunes unread columns: the stored output computes every column and +# fails on the fixture, while an inline comparison of the expected columns alone succeeds. +_PRUNING_WAREHOUSE: str = ( + "from sqlbuild.adapters.duckdb.classes.duckdb_adapter import DuckDbAdapter\n\n\n" + "class PruningDuckDbAdapter(DuckDbAdapter):\n" + ' adapter_name = "pruning_duckdb"\n' + " failing_outputs = set()\n\n" + " def create_sql_test_output(self, *, connection, sql, name):\n" + " self.failing_outputs.add(sql)\n" + f' raise RuntimeError("{_CONVERSION_ERROR}")\n\n' + " def execute(self, *, connection, sql):\n" + " if sql in self.failing_outputs:\n" + f' raise RuntimeError("{_CONVERSION_ERROR}")\n' + " return super().execute(connection=connection, sql=sql)\n" +) +# A session that cannot keep test outputs at all, while every query runs. +_NO_TEMPORARY_TABLES: str = ( + "from sqlbuild.adapters.duckdb.classes.duckdb_adapter import DuckDbAdapter\n\n\n" + "class PruningDuckDbAdapter(DuckDbAdapter):\n" + ' adapter_name = "pruning_duckdb"\n\n' + " def create_sql_test_output(self, *, connection, sql, name):\n" + ' raise PermissionError("temporary tables are disabled on this session")\n' +) +_PROJECT_FILES: dict[str, str] = { + "sqlbuild_project.toml": ( + 'name = "orders"\nadapter = "pruning_duckdb"\n\n' + '[connection]\ndatabase = "orders.duckdb"\n\n' + '[defaults]\nmaterialized = "table"\n' + ), + "sources/raw_orders.yml": ( + "sources:\n" + " - name: raw_orders\n description: Test source raw_orders.\n" + " schema: main\n" + " table: raw_orders\n" + " columns:\n" + " - name: order_id\n type: INTEGER\n" + " - name: code\n type: VARCHAR\n" + ), + "models/orders.sql": ( + "MODEL (description 'Test model orders.');\n\n" + 'SELECT order_id, TRY_CAST(code AS INTEGER) AS code_number FROM __source("raw_orders")\n' + ), + "tests/unit/test_orders.sql": ( + 'TEST (name "orders_ids");\n\nWITH\n' + "__source__raw_orders AS (\n SELECT 1 AS order_id, 'cluster::r1' AS code\n),\n" + "__expected__orders AS (\n SELECT 1 AS order_id\n)\nSELECT 1\n" + ), +} + + +@pytest.mark.parametrize( + "test_case", + [ + OutputEvaluationErrorE2ETestCase( + description="an output the warehouse cannot compute errors the test", + adapter_source=_PRUNING_WAREHOUSE, + expected_exit_code=1, + expected_fragments=( + "test 'orders_ids' encountered an execution error while running 'orders'", + _CONVERSION_ERROR, + "PASS=0 FAIL=1", + ), + expected_json_fragments=('"error"', _CONVERSION_ERROR), + unexpected_fragments=("PASS=1 FAIL=0",), + ), + OutputEvaluationErrorE2ETestCase( + description="a session that cannot store outputs still compares inline", + adapter_source=_NO_TEMPORARY_TABLES, + expected_exit_code=0, + expected_fragments=("PASS=1 FAIL=0",), + expected_json_fragments=('"pass"',), + unexpected_fragments=("encountered an execution error",), + ), + ], + ids=lambda case: case.description, +) +def test_given_output_store_failure_when_testing_then_only_storage_problems_fall_back( + test_case: OutputEvaluationErrorE2ETestCase, tmp_path: Path +) -> None: + """An inline comparison of some columns must not hide an error computing the others.""" + + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="orders", + repo_files={**_PROJECT_FILES, "adapters/pruning_duckdb.py": test_case.adapter_source}, + ) + execute_duckdb( + db_path=project_dir / "orders.duckdb", + sql="CREATE TABLE raw_orders AS SELECT 1 AS order_id, '1' AS code", + ) + + tested: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "test"), project_dir=project_dir + ) + as_json: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "test", "--json"), project_dir=project_dir + ) + payload: dict[str, Any] = json.loads(as_json.stdout) + + output: str = tested.stdout + tested.stderr + assert tested.returncode == test_case.expected_exit_code, output + assert as_json.returncode == test_case.expected_exit_code, as_json.stdout + assert all(fragment in output for fragment in test_case.expected_fragments), output + assert not any(fragment in output for fragment in test_case.unexpected_fragments), output + assert all(fragment in json.dumps(payload) for fragment in test_case.expected_json_fragments), ( + as_json.stdout + ) diff --git a/tests/integration/src/sqlbuild/executor/testing/_test_types.py b/tests/integration/src/sqlbuild/executor/testing/_test_types.py index bf13ea00ec..d3f4a00336 100644 --- a/tests/integration/src/sqlbuild/executor/testing/_test_types.py +++ b/tests/integration/src/sqlbuild/executor/testing/_test_types.py @@ -64,3 +64,15 @@ class SqlTestBuildParityTestCase: model_name: str expected_whole_body_rendered: bool expected_verbatim_fragments: tuple[str, ...] = () + + +@dataclass(frozen=True) +class OutputStorageTestCase: + """A tested output that cannot be stored, and whether the test still runs inline.""" + + description: str + chain_steps: tuple[tuple[str, str, str | None], ...] + storage_error: str + output_fails: bool + expected_outcome: SqlTestOutcome + expected_error_fragment: str diff --git a/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py b/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py index 88c05db349..f5e7a0a933 100644 --- a/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py +++ b/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py @@ -25,6 +25,7 @@ from sqlbuild.executor.testing.types import SqlTestDifferenceDirection, SqlTestOutcome from tests.integration.src.sqlbuild.executor.testing._test_types import ( ExpectedBooleanTestCase, + OutputStorageTestCase, SqlTestComparisonSqlTestCase, SqlTestDiagnosticsTestCase, SqlTestDifferenceSqlTestCase, @@ -49,6 +50,29 @@ def execute(self, *, connection: Any, sql: str) -> Any: return super().execute(connection=connection, sql=sql) +class StorageFailingDuckDbAdapter(DuckDbAdapter): + """Fails to store a test output; with ``output_fails`` the output query itself errors. + + The output query failing while inline comparisons pass is how a warehouse that prunes + unread columns behaves when a fixture breaks a column the expected rows leave out. + """ + + def __init__(self, *, error: str, output_fails: bool) -> None: + super().__init__() + self._error: str = error + self._output_fails: bool = output_fails + self._outputs: set[str] = set() + + def create_sql_test_output(self, *, connection: Any, sql: str, name: str) -> str | None: + self._outputs.add(sql) + raise RuntimeError(self._error) + + def execute(self, *, connection: Any, sql: str) -> Any: + if self._output_fails and sql in self._outputs: + raise RuntimeError(self._error) + return super().execute(connection=connection, sql=sql) + + class DescriptionlessDifferenceSampleDuckDbAdapter(DuckDbAdapter): def execute(self, *, connection: Any, sql: str) -> Any: if "__sqlbuild_difference" in sql: @@ -700,3 +724,40 @@ def test_given_oversized_unit_test_sql_when_executing_then_it_returns_clear_erro assert "maximum of 80 bytes" in result.error_message assert "scenario test" in result.error_message verify_test_result(result=result, test_case=test_case) + + +@pytest.mark.parametrize( + "test_case", + [ + OutputStorageTestCase( + description="an output query the warehouse cannot compute errors the test", + chain_steps=(("orders", "SELECT 1 AS order_id", "SELECT 1 AS order_id"),), + storage_error="Numeric value 'cluster::r1' is not recognized", + output_fails=True, + expected_outcome=SqlTestOutcome.ERROR, + expected_error_fragment="Numeric value 'cluster::r1' is not recognized", + ), + OutputStorageTestCase( + description="a session that cannot store outputs still compares inline", + chain_steps=(("orders", "SELECT 1 AS order_id", "SELECT 1 AS order_id"),), + storage_error="temporary tables are not allowed in this session", + output_fails=False, + expected_outcome=SqlTestOutcome.PASS, + expected_error_fragment="", + ), + ], + ids=lambda case: case.description, +) +def test_given_output_that_cannot_be_stored_when_executing_then_only_query_errors_fail( + test_case: OutputStorageTestCase, connection: Any +) -> None: + result: SqlTestExecutionResult = execute_sql_test( + test_entry=build_sql_test_plan_entry(name="orders_ids", chain_steps=test_case.chain_steps), + adapter=StorageFailingDuckDbAdapter( + error=test_case.storage_error, output_fails=test_case.output_fails + ), + connection=connection, + ) + + assert result.outcome == test_case.expected_outcome, result.error_message + assert test_case.expected_error_fragment in (result.error_message or "") diff --git a/website/src/content/docs/docs/concepts/testing.mdx b/website/src/content/docs/docs/concepts/testing.mdx index 999c0fcae3..326337f61c 100644 --- a/website/src/content/docs/docs/concepts/testing.mdx +++ b/website/src/content/docs/docs/concepts/testing.mdx @@ -894,7 +894,10 @@ SQLBuild drops after the test, including after a failure or timeout. PostgreSQL one transaction with an `ON COMMIT DROP` temporary table that the transaction's end removes, so the reads stay on one server session behind a transaction-mode connection pooler. Snowflake reads the first query's persisted result through `RESULT_SCAN`. If a stored output can no longer be read, -the test falls back to the inline comparison. Other adapters compare the inline query as before. `target/run/tests/` keeps the self-contained inline comparison query for each test. +the test falls back to the inline comparison. If storing an output fails, SQLBuild runs the output +query on its own: when that query fails too, for example because a fixture value cannot be +converted in a column the expected rows leave out, the test errors with the warehouse's message +instead of passing on a comparison of the other columns; otherwise it compares inline. Other adapters compare the inline query as before. `target/run/tests/` keeps the self-contained inline comparison query for each test. ## Skipping SQL analysis From 7e376a7d1d4b542d4e611238ddb5e404c93f2395 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:25:30 +0100 Subject: [PATCH 2/3] fix: check a unit test output that cannot be stored on its own, inline When a later chain step's output could not be stored, the probe ran that step's query reading the earlier steps' stored outputs, which had just been dropped, so it reported a missing table instead of comparing inline. The probe now renders the failing step fully inline, runs only when storing itself failed rather than rendering, and keeps the storage error in the debug log when the step's query fails on its own. --- .../classes/sql_test_actual_outputs.py | 49 ++++++++++---- .../cli/commands/main/test/_test_types.py | 1 + .../cli/commands/main/test/helpers.py | 17 +++++ .../test/test_output_evaluation_errors.py | 48 +++++++++++++- .../sqlbuild/executor/testing/_test_types.py | 11 ++++ .../testing/test_sql_test_execution.py | 66 +++++++++++++++++++ 6 files changed, 177 insertions(+), 15 deletions(-) diff --git a/src/sqlbuild/executor/testing/classes/sql_test_actual_outputs.py b/src/sqlbuild/executor/testing/classes/sql_test_actual_outputs.py index 9ac0675c67..3c2da97960 100644 --- a/src/sqlbuild/executor/testing/classes/sql_test_actual_outputs.py +++ b/src/sqlbuild/executor/testing/classes/sql_test_actual_outputs.py @@ -50,23 +50,19 @@ def materialize(self) -> dict[int, str]: """Store each compared step's output, or return no relations to compare inline.""" token: str = uuid4().hex[:12] - sql: str | None = None + storing: int | None = None try: for index in range(len(self._test_entry.chain)): - (sql,) = render_sql_test_comparison_sql( - (self._test_entry,), - set_difference_operator=self._adapter.render_set_difference_operator(), - sql_analysis_dialect=self._adapter.sql_analysis_dialect(), - output_step_index=index, - actual_relations=dict(self.relations), - ) + sql: str = self._output_sql(index=index, relations=dict(self.relations)) if not sql: continue + storing = index relation: str | None = self._adapter.create_sql_test_output( connection=self._connection, sql=sql, name=f"{_OUTPUT_PREFIX}_{token}_{index}", ) + storing = None if relation is None: self.release() return {} @@ -77,7 +73,7 @@ def materialize(self) -> dict[int, str]: raise except Exception as error: self.release() - self._evaluate_output(sql=sql) + self._evaluate_output(index=storing, storage_error=error) log_debug_event( logger=_LOGGER, message="SQL test output could not be stored; comparing inline", @@ -87,13 +83,38 @@ def materialize(self) -> dict[int, str]: return {} return dict(self.relations) - def _evaluate_output(self, *, sql: str | None) -> None: - """Run a step's output query alone so its own error fails the test, not a fallback.""" + def _output_sql(self, *, index: int, relations: dict[int, str]) -> str: + """The SQL of step ``index``'s output, reading the given stored outputs of earlier steps.""" + + (sql,) = render_sql_test_comparison_sql( + (self._test_entry,), + set_difference_operator=self._adapter.render_set_difference_operator(), + sql_analysis_dialect=self._adapter.sql_analysis_dialect(), + output_step_index=index, + actual_relations=relations, + ) + return sql + + def _evaluate_output(self, *, index: int | None, storage_error: Exception) -> None: + """Run a step that could not be stored fully inline, raising the query's own error.""" - if not sql: + if index is None: return - cursor: Any = self._adapter.execute(connection=self._connection, sql=sql) - _ = cursor.fetchall() + try: + cursor: Any = self._adapter.execute( + connection=self._connection, sql=self._output_sql(index=index, relations={}) + ) + _ = cursor.fetchall() + except (ItemTimeoutError, ExecutionDurationLimitError): + raise + except Exception: + log_debug_event( + logger=_LOGGER, + message="SQL test output could not be stored, and its query fails on its own", + test_name=self._test_entry.name, + sqlbuild_error=DiagnosticRecordRedactor.text(str(storage_error)), + ) + raise def release(self) -> None: """Drop every stored output; later comparisons run inline.""" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py index f9c278b2d2..6a913f11c7 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py @@ -478,6 +478,7 @@ class OutputEvaluationErrorE2ETestCase: description: str adapter_source: str + extra_files: dict[str, str] expected_exit_code: int expected_fragments: tuple[str, ...] expected_json_fragments: tuple[str, ...] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py index a1e0f63271..c86779e631 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py @@ -2096,3 +2096,20 @@ def slow_promotion_files( f"SELECT {customer_id} AS customer_id\n" ), } + + +def order_totals_test_files(total_sql: str) -> dict[str, str]: + """Return an order_totals model computing ``total_sql`` and a test expecting both models.""" + + return { + "models/order_totals.sql": ( + "MODEL (description 'Test model order_totals.');\n\n" + f'SELECT order_id, {total_sql} AS total FROM __ref("orders")\n' + ), + "tests/unit/test_orders.sql": ( + 'TEST (name "orders_ids");\n\nWITH\n' + "__source__raw_orders AS (\n SELECT 1 AS order_id, '7' AS code\n),\n" + "__expected__orders AS (\n SELECT 1 AS order_id\n),\n" + "__expected__order_totals AS (\n SELECT 1 AS order_id\n)\nSELECT 1\n" + ), + } diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_output_evaluation_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_output_evaluation_errors.py index 22e1a56e47..c360e4b7e7 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_output_evaluation_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_output_evaluation_errors.py @@ -12,6 +12,7 @@ from tests.e2e.src.sqlbuild.cli.commands.main.test._test_types import ( OutputEvaluationErrorE2ETestCase, ) +from tests.e2e.src.sqlbuild.cli.commands.main.test.helpers import order_totals_test_files from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import ( execute_duckdb, prepare_inline_project, @@ -42,6 +43,20 @@ " def create_sql_test_output(self, *, connection, sql, name):\n" ' raise PermissionError("temporary tables are disabled on this session")\n' ) +# A session that keeps one test output and refuses every later one. +_ONE_TEMPORARY_TABLE: str = ( + "from sqlbuild.adapters.duckdb.classes.duckdb_adapter import DuckDbAdapter\n\n\n" + "class PruningDuckDbAdapter(DuckDbAdapter):\n" + ' adapter_name = "pruning_duckdb"\n' + " stored = []\n\n" + " def create_sql_test_output(self, *, connection, sql, name):\n" + " self.stored.append(name)\n" + " if len(self.stored) > 1:\n" + ' raise PermissionError("no more temporary tables in this session")\n' + " return super().create_sql_test_output(connection=connection, sql=sql, name=name)\n" +) + + _PROJECT_FILES: dict[str, str] = { "sqlbuild_project.toml": ( 'name = "orders"\nadapter = "pruning_duckdb"\n\n' @@ -75,6 +90,7 @@ OutputEvaluationErrorE2ETestCase( description="an output the warehouse cannot compute errors the test", adapter_source=_PRUNING_WAREHOUSE, + extra_files={}, expected_exit_code=1, expected_fragments=( "test 'orders_ids' encountered an execution error while running 'orders'", @@ -87,11 +103,30 @@ OutputEvaluationErrorE2ETestCase( description="a session that cannot store outputs still compares inline", adapter_source=_NO_TEMPORARY_TABLES, + extra_files={}, expected_exit_code=0, expected_fragments=("PASS=1 FAIL=0",), expected_json_fragments=('"pass"',), unexpected_fragments=("encountered an execution error",), ), + OutputEvaluationErrorE2ETestCase( + description="a computable later output that cannot be stored compares inline", + adapter_source=_ONE_TEMPORARY_TABLE, + extra_files=order_totals_test_files("code_number + 1"), + expected_exit_code=0, + expected_fragments=("PASS=1 FAIL=0",), + expected_json_fragments=('"pass"',), + unexpected_fragments=("encountered an execution error", "does not exist"), + ), + OutputEvaluationErrorE2ETestCase( + description="a later output that fails on its own reports the warehouse error", + adapter_source=_ONE_TEMPORARY_TABLE, + extra_files=order_totals_test_files("CAST('cluster::r1' AS INTEGER)"), + expected_exit_code=1, + expected_fragments=("Could not convert string 'cluster::r1'", "PASS=0 FAIL=1"), + expected_json_fragments=('"error"', "Could not convert string"), + unexpected_fragments=("does not exist",), + ), ], ids=lambda case: case.description, ) @@ -103,7 +138,18 @@ def test_given_output_store_failure_when_testing_then_only_storage_problems_fall project_dir: Path = prepare_inline_project( tmp_path=tmp_path, project_name="orders", - repo_files={**_PROJECT_FILES, "adapters/pruning_duckdb.py": test_case.adapter_source}, + repo_files={ + **_PROJECT_FILES, + "adapters/pruning_duckdb.py": test_case.adapter_source, + **test_case.extra_files, + }, + ) + config_path: Path = project_dir / "sqlbuild_project.toml" + _ = config_path.write_text( + config_path.read_text(encoding="utf-8").replace( + '"orders.duckdb"', f'"{(project_dir / "orders.duckdb").as_posix()}"' + ), + encoding="utf-8", ) execute_duckdb( db_path=project_dir / "orders.duckdb", diff --git a/tests/integration/src/sqlbuild/executor/testing/_test_types.py b/tests/integration/src/sqlbuild/executor/testing/_test_types.py index d3f4a00336..b556d7635d 100644 --- a/tests/integration/src/sqlbuild/executor/testing/_test_types.py +++ b/tests/integration/src/sqlbuild/executor/testing/_test_types.py @@ -76,3 +76,14 @@ class OutputStorageTestCase: output_fails: bool expected_outcome: SqlTestOutcome expected_error_fragment: str + + +@dataclass(frozen=True) +class LaterStepStorageTestCase: + """A chain whose first output is stored and whose second cannot be, and how the test ends.""" + + description: str + chain_steps: tuple[tuple[str, str, str | None], ...] + expected_outcome: SqlTestOutcome + expected_error_fragment: str + unexpected_error_fragment: str diff --git a/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py b/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py index f5e7a0a933..97da51d4f7 100644 --- a/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py +++ b/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py @@ -25,6 +25,7 @@ from sqlbuild.executor.testing.types import SqlTestDifferenceDirection, SqlTestOutcome from tests.integration.src.sqlbuild.executor.testing._test_types import ( ExpectedBooleanTestCase, + LaterStepStorageTestCase, OutputStorageTestCase, SqlTestComparisonSqlTestCase, SqlTestDiagnosticsTestCase, @@ -73,6 +74,20 @@ def execute(self, *, connection: Any, sql: str) -> Any: return super().execute(connection=connection, sql=sql) +class SecondStorageFailingDuckDbAdapter(DuckDbAdapter): + """Stores the first test output and refuses every later one, as a session at its limit may.""" + + def __init__(self) -> None: + super().__init__() + self._stored: int = 0 + + def create_sql_test_output(self, *, connection: Any, sql: str, name: str) -> str | None: + self._stored += 1 + if self._stored > 1: + raise RuntimeError("no more temporary tables in this session") + return super().create_sql_test_output(connection=connection, sql=sql, name=name) + + class DescriptionlessDifferenceSampleDuckDbAdapter(DuckDbAdapter): def execute(self, *, connection: Any, sql: str) -> Any: if "__sqlbuild_difference" in sql: @@ -761,3 +776,54 @@ def test_given_output_that_cannot_be_stored_when_executing_then_only_query_error assert result.outcome == test_case.expected_outcome, result.error_message assert test_case.expected_error_fragment in (result.error_message or "") + + +@pytest.mark.parametrize( + "test_case", + [ + LaterStepStorageTestCase( + description="a computable second output that cannot be stored compares inline", + chain_steps=( + ("stg_orders", "SELECT 1 AS order_id", "SELECT 1 AS order_id"), + ( + "orders", + "WITH __ref__stg_orders AS (SELECT 1 AS order_id)\n" + "SELECT order_id + 1 AS order_id FROM __ref__stg_orders", + "SELECT 2 AS order_id", + ), + ), + expected_outcome=SqlTestOutcome.PASS, + expected_error_fragment="", + unexpected_error_fragment="does not exist", + ), + LaterStepStorageTestCase( + description="a second output that fails on its own reports the warehouse error", + chain_steps=( + ("stg_orders", "SELECT 1 AS order_id", "SELECT 1 AS order_id"), + ( + "orders", + "WITH __ref__stg_orders AS (SELECT 1 AS order_id)\n" + "SELECT order_id + CAST('cluster::r1' AS INTEGER) AS order_id " + "FROM __ref__stg_orders", + "SELECT 2 AS order_id", + ), + ), + expected_outcome=SqlTestOutcome.ERROR, + expected_error_fragment="Could not convert string", + unexpected_error_fragment="does not exist", + ), + ], + ids=lambda case: case.description, +) +def test_given_later_step_that_cannot_be_stored_when_executing_then_it_is_checked_inline( + test_case: LaterStepStorageTestCase, connection: Any +) -> None: + result: SqlTestExecutionResult = execute_sql_test( + test_entry=build_sql_test_plan_entry(name="orders_ids", chain_steps=test_case.chain_steps), + adapter=SecondStorageFailingDuckDbAdapter(), + connection=connection, + ) + + assert result.outcome == test_case.expected_outcome, result.error_message + assert test_case.expected_error_fragment in (result.error_message or "") + assert test_case.unexpected_error_fragment not in (result.error_message or "") From 46460a92b5d84afabba816b5b312bbfb713e4457 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 22 Oct 2026 01:06:13 +0100 Subject: [PATCH 3/3] fix: compare a unit test inline when DuckDB fails to store it internally DuckDB 1.5.0 to 1.5.4 raise an internal binder error for two identical window functions over a GROUP BY of a constant input, which is the shape of a unit test's fixture CTE. Storing the output and running it on its own both hit it, so the output check reported the engine bug as the model's error while the inline comparison computes every column correctly. An internal engine error while storing an output now compares inline. --- .../references/docs/concepts/testing.md | 4 +- .../contract/classes/duckdb_backed_adapter.py | 19 +++++- src/sqlbuild/adapter/contract/constants.py | 1 + .../sqlbuild/executor/testing/_test_types.py | 10 +++ .../testing/test_sql_test_execution.py | 68 +++++++++++++++++++ .../content/docs/docs/concepts/testing.mdx | 4 +- 6 files changed, 101 insertions(+), 5 deletions(-) diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md index c0458a6737..7d530f30aa 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md @@ -922,7 +922,9 @@ first query's persisted result through `RESULT_SCAN`. If a stored output can no the test falls back to the inline comparison. If storing an output fails, SQLBuild runs the output query on its own: when that query fails too, for example because a fixture value cannot be converted in a column the expected rows leave out, the test errors with the warehouse's message -instead of passing on a comparison of the other columns; otherwise it compares inline. Other adapters compare the inline query as before. `target/run/tests/` keeps the self-contained inline comparison query for each test. +instead of passing on a comparison of the other columns; otherwise it compares inline. On DuckDB and +MotherDuck, an internal engine error while storing an output is a DuckDB defect rather than a +problem with the model, so the test compares inline. Other adapters compare the inline query as before. `target/run/tests/` keeps the self-contained inline comparison query for each test. ## Skipping SQL analysis diff --git a/src/sqlbuild/adapter/contract/classes/duckdb_backed_adapter.py b/src/sqlbuild/adapter/contract/classes/duckdb_backed_adapter.py index 30b52b2c6b..cfc6da6ee4 100644 --- a/src/sqlbuild/adapter/contract/classes/duckdb_backed_adapter.py +++ b/src/sqlbuild/adapter/contract/classes/duckdb_backed_adapter.py @@ -40,6 +40,7 @@ CANCELLABLE_DURATION_LIMIT_SECONDS, DIFF_LEFT_SIDE, DIFF_RIGHT_SIDE, + DUCKDB_INTERNAL_ERROR_CLASS_NAME, DUCKDB_MISSING_OBJECT_MARKER, DUCKDB_NOT_FOUND_ERROR_CLASS_NAMES, QUALIFIED_NAME_SEPARATOR, @@ -91,6 +92,8 @@ from sqlbuild.adapter.type_system.main.normalize_numeric_family import normalize_numeric_family from sqlbuild.adapter.type_system.main.types_equal import types_equal from sqlbuild.compiler.compile.types import FunctionLanguage +from sqlbuild.diagnostics.classes.diagnostic_record_redactor import DiagnosticRecordRedactor +from sqlbuild.diagnostics.main.log_debug_event import log_debug_event from sqlbuild.diagnostics.main.log_sql import log_sql from sqlbuild.runtime.user_code.main.user_code_spec import user_code_spec from sqlbuild.spec.contracts.constants import DEFAULT_SEED_CSV_SETTINGS @@ -118,9 +121,19 @@ def get_columns_for_relations( execution_duration_limit_seconds: ClassVar[int | None] = CANCELLABLE_DURATION_LIMIT_SECONDS def create_sql_test_output(self, *, connection: Any, sql: str, name: str) -> str | None: - """Store a unit test's actual output once in a session temporary table.""" - - _ = self.execute(connection=connection, sql=f"CREATE TEMPORARY TABLE {name} AS {sql}") + """Store a unit test's output in a temporary table; compare inline on an engine bug.""" + + try: + _ = self.execute(connection=connection, sql=f"CREATE TEMPORARY TABLE {name} AS {sql}") + except Exception as error: + if type(error).__name__ != DUCKDB_INTERNAL_ERROR_CLASS_NAME: + raise + log_debug_event( + logger=logging.getLogger("sqlbuild.adapter.duckdb"), + message="DuckDB could not store a SQL test output; comparing inline", + sqlbuild_error=DiagnosticRecordRedactor.text(str(error)), + ) + return None return name def drop_sql_test_output(self, *, connection: Any, relation: str) -> None: diff --git a/src/sqlbuild/adapter/contract/constants.py b/src/sqlbuild/adapter/contract/constants.py index 0a895325e8..c5671dbfd3 100644 --- a/src/sqlbuild/adapter/contract/constants.py +++ b/src/sqlbuild/adapter/contract/constants.py @@ -17,4 +17,5 @@ {"CatalogException", "BinderException"} ) DUCKDB_MISSING_OBJECT_MARKER: str = "does not exist" +DUCKDB_INTERNAL_ERROR_CLASS_NAME: str = "InternalException" CANCELLABLE_DURATION_LIMIT_SECONDS: int = 2_147_483 diff --git a/tests/integration/src/sqlbuild/executor/testing/_test_types.py b/tests/integration/src/sqlbuild/executor/testing/_test_types.py index b556d7635d..6cd0f5e6e8 100644 --- a/tests/integration/src/sqlbuild/executor/testing/_test_types.py +++ b/tests/integration/src/sqlbuild/executor/testing/_test_types.py @@ -87,3 +87,13 @@ class LaterStepStorageTestCase: expected_outcome: SqlTestOutcome expected_error_fragment: str unexpected_error_fragment: str + + +@dataclass(frozen=True) +class EngineInternalErrorTestCase: + """A tested output DuckDB fails to store with an engine assertion error.""" + + description: str + chain_steps: tuple[tuple[str, str, str | None], ...] + raise_internal_error: bool + expected_outcome: SqlTestOutcome diff --git a/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py b/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py index 97da51d4f7..0c5e55a618 100644 --- a/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py +++ b/tests/integration/src/sqlbuild/executor/testing/test_sql_test_execution.py @@ -24,6 +24,7 @@ ) from sqlbuild.executor.testing.types import SqlTestDifferenceDirection, SqlTestOutcome from tests.integration.src.sqlbuild.executor.testing._test_types import ( + EngineInternalErrorTestCase, ExpectedBooleanTestCase, LaterStepStorageTestCase, OutputStorageTestCase, @@ -88,6 +89,29 @@ def create_sql_test_output(self, *, connection: Any, sql: str, name: str) -> str return super().create_sql_test_output(connection=connection, sql=sql, name=name) +class InternalException(Exception): + """Stands in for DuckDB's engine assertion error, which the adapter matches by class name.""" + + +class InternalErrorOnStoreDuckDbAdapter(DuckDbAdapter): + """With ``fail``, the bare output query hits an engine assertion error; comparisons do not.""" + + def __init__(self, *, fail: bool) -> None: + super().__init__() + self._fail: bool = fail + self._outputs: set[str] = set() + + def create_sql_test_output(self, *, connection: Any, sql: str, name: str) -> str | None: + self._outputs.add(sql) + return super().create_sql_test_output(connection=connection, sql=sql, name=name) + + def execute(self, *, connection: Any, sql: str) -> Any: + stored: bool = sql.startswith("CREATE TEMPORARY TABLE") + if self._fail and (stored or sql in self._outputs): + raise InternalException('INTERNAL Error: Failed to bind column reference ""') + return super().execute(connection=connection, sql=sql) + + class DescriptionlessDifferenceSampleDuckDbAdapter(DuckDbAdapter): def execute(self, *, connection: Any, sql: str) -> Any: if "__sqlbuild_difference" in sql: @@ -827,3 +851,47 @@ def test_given_later_step_that_cannot_be_stored_when_executing_then_it_is_checke assert result.outcome == test_case.expected_outcome, result.error_message assert test_case.expected_error_fragment in (result.error_message or "") assert test_case.unexpected_error_fragment not in (result.error_message or "") + + +@pytest.mark.parametrize( + "test_case", + [ + EngineInternalErrorTestCase( + description="an engine assertion error storing the output compares inline", + chain_steps=(("orders", "SELECT 1 AS order_id", "SELECT 1 AS order_id"),), + raise_internal_error=True, + expected_outcome=SqlTestOutcome.PASS, + ), + EngineInternalErrorTestCase( + description="an engine assertion error does not hide a mismatch", + chain_steps=(("orders", "SELECT 1 AS order_id", "SELECT 2 AS order_id"),), + raise_internal_error=True, + expected_outcome=SqlTestOutcome.FAIL, + ), + EngineInternalErrorTestCase( + description="two equal windows over a grouped constant fixture pass", + chain_steps=( + ( + "orders", + "SELECT MAX(amount) OVER (PARTITION BY order_id) AS high, " + "MAX(amount) OVER (PARTITION BY order_id) AS top " + "FROM (SELECT 1 AS order_id, 1 AS amount) GROUP BY order_id, amount", + "SELECT 1 AS high, 1 AS top", + ), + ), + raise_internal_error=False, + expected_outcome=SqlTestOutcome.PASS, + ), + ], + ids=lambda case: case.description, +) +def test_given_engine_internal_error_storing_output_when_executing_then_compares_inline( + test_case: EngineInternalErrorTestCase, connection: Any +) -> None: + result: SqlTestExecutionResult = execute_sql_test( + test_entry=build_sql_test_plan_entry(name="orders_ids", chain_steps=test_case.chain_steps), + adapter=InternalErrorOnStoreDuckDbAdapter(fail=test_case.raise_internal_error), + connection=connection, + ) + + assert result.outcome == test_case.expected_outcome, result.error_message diff --git a/website/src/content/docs/docs/concepts/testing.mdx b/website/src/content/docs/docs/concepts/testing.mdx index 326337f61c..724102fcc8 100644 --- a/website/src/content/docs/docs/concepts/testing.mdx +++ b/website/src/content/docs/docs/concepts/testing.mdx @@ -897,7 +897,9 @@ first query's persisted result through `RESULT_SCAN`. If a stored output can no the test falls back to the inline comparison. If storing an output fails, SQLBuild runs the output query on its own: when that query fails too, for example because a fixture value cannot be converted in a column the expected rows leave out, the test errors with the warehouse's message -instead of passing on a comparison of the other columns; otherwise it compares inline. Other adapters compare the inline query as before. `target/run/tests/` keeps the self-contained inline comparison query for each test. +instead of passing on a comparison of the other columns; otherwise it compares inline. On DuckDB and +MotherDuck, an internal engine error while storing an output is a DuckDB defect rather than a +problem with the model, so the test compares inline. Other adapters compare the inline query as before. `target/run/tests/` keeps the self-contained inline comparison query for each test. ## Skipping SQL analysis