From f3e7960d409aa1935fca4f6ebe1758013b4472aa Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 8 Oct 2026 05:58:27 +0100 Subject: [PATCH 1/4] fix(test): resolve mocked and expected-row references consistently in SQL tests A mocked name now always means the mock in every SQL-test CTE: an assertion calling __ref() on a mocked model reads the mock instead of running the real model, and mocked models no longer count as test targets. References written directly in __expected__ CTEs are resolved like assertions and helpers. Compile reports unmocked __source(), __seed() and __dbt_ref() calls and unknown models in expected rows with P013. The native planner computes the models helpers and expected rows read itself and rejects any reference left raw. --- .../_helpers/sql_tests/helper_scope.rs | 78 ++++-- .../src/compiler/_helpers/sql_tests/mod.rs | 1 + .../compiler/_helpers/sql_tests/planning.rs | 116 ++++---- .../_helpers/sql_tests/reader_references.rs | 68 +++++ .../src/compiler/tests/helpers.rs | 33 ++- .../tests/test_sql_test_helper_references.rs | 198 ++++++++++++- .../src/compiler/tests/test_types.rs | 10 +- .../_helpers/corpus/render_failure_cases.py | 38 +++ .../references/docs/concepts/testing.md | 27 +- .../compile/_helpers/assembly/project.py | 7 +- .../compile/_helpers/attachment/sql_tests.py | 49 ++-- .../compile/_helpers/sql_tests/helper_ctes.py | 98 ++++--- src/sqlbuild/compiler/compile/models.py | 3 +- .../_helpers/sql_tests/native_planning.py | 1 - .../cli/commands/main/test/helpers.py | 34 +++ .../main/test/test_helper_references.py | 264 ++++++++++++++++++ .../sql_test_assembly/test_plan_test.py | 4 +- .../content/docs/docs/concepts/testing.mdx | 27 +- 18 files changed, 878 insertions(+), 178 deletions(-) create mode 100644 crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs index 3dbcec432b..6e22e9c62e 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs @@ -344,6 +344,11 @@ impl ScopeGraph { .collect() } + /// The authored name of one helper. + pub(crate) fn helper_name(&self, index: usize) -> &str { + &self.helpers[index].name + } + /// Replace a helper's references with the CTEs a reader must define ahead of it. pub(crate) fn resolve_helper(&mut self, index: usize, resolved: ResolvedHelper) { let placement = self.placement; @@ -520,6 +525,55 @@ fn test_readers(fixtures: &TestFixtures) -> Vec<(String, &str)> { readers } +/// Authored SQL of the helpers `readers` reach, directly or through other helpers and mocks. +pub(crate) fn read_helper_sql<'f>( + fixtures: &'f TestFixtures, + readers: &[&str], + patterns: &SqlTestPatterns, +) -> Vec<&'f str> { + if fixtures.helpers.is_empty() { + return Vec::new(); + } + let mut bodies: HashMap = fixtures + .helpers + .iter() + .map(|cte| (cte.name.to_ascii_lowercase(), (cte.sql_body.as_str(), true))) + .collect(); + for (prefix, group) in [ + (REF_PREFIX, &fixtures.mock_refs), + (SOURCE_PREFIX, &fixtures.mock_sources), + (SEED_PREFIX, &fixtures.mock_seeds), + (DBT_REF_PREFIX, &fixtures.mock_dbt_refs), + (TABLE_FUNCTION_PREFIX, &fixtures.mock_table_functions), + ] { + for (name, sql) in group { + bodies.insert( + format!("{prefix}{name}").to_ascii_lowercase(), + (sql.as_str(), false), + ); + } + } + let mut pending: Vec = readers + .iter() + .flat_map(|sql| identifier_tokens(sql, patterns)) + .collect(); + let mut visited: HashSet = HashSet::new(); + let mut helpers: Vec<&'f str> = Vec::new(); + while let Some(token) = pending.pop() { + let Some((sql, is_helper)) = bodies.get(&token).copied() else { + continue; + }; + if !visited.insert(token) { + continue; + } + if is_helper { + helpers.push(sql); + } + pending.extend(identifier_tokens(sql, patterns)); + } + helpers +} + /// Return the helper and mock CTEs `sql` reads, transitively and dependencies first. pub(crate) fn helper_scope_ctes( sql: &str, @@ -549,35 +603,21 @@ pub(crate) fn helper_scope_ctes( Ok(graph.collect_scope(nodes)) } -/// Merge scoped CTEs into a step's CTEs; relations resolved helpers read come first and win. +/// Append scoped CTEs after a step's CTEs; a same-named mock or model CTE keeps the step's copy. pub(crate) fn merged_scoped_ctes( lifted: Vec<(String, String)>, scoped: HelperScope, file_label: &str, ) -> Result, String> { - type Ctes = Vec<(String, String)>; - let (generated, authored): (Ctes, Ctes) = scoped - .ctes - .into_iter() - .partition(|(name, _)| scoped.generated.contains(&name.to_ascii_lowercase())); let mut merged: Vec<(String, String)> = lifted; - if !generated.is_empty() { - let own = std::mem::replace(&mut merged, generated); - for (name, sql) in own { - if !merged - .iter() - .any(|(existing, _)| existing.eq_ignore_ascii_case(&name)) - { - merged.push((name, sql)); - } - } - } - for (name, sql) in authored { + for (name, sql) in scoped.ctes { match merged .iter() .find(|(existing, _)| existing.eq_ignore_ascii_case(&name)) { - Some((_, existing_sql)) if *existing_sql == sql => {} + Some((_, existing_sql)) + if *existing_sql == sql + || scoped.generated.contains(&name.to_ascii_lowercase()) => {} Some(_) => { return Err(compile_error(&format!( "SQL test '{file_label}' defines CTE '{name}', which conflicts with the generated CTE" diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs index cb1217cfcd..b4273df693 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs @@ -7,6 +7,7 @@ pub mod helper_names; pub mod helper_scope; pub mod markers; pub mod planning; +pub mod reader_references; pub mod relation_markers; pub mod rendering; pub mod scenario_extraction; diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs index a163db7201..976a955912 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs @@ -23,6 +23,9 @@ use crate::compiler::_helpers::sql_tests::markers::{ ProtectedRanges, marker_names, marker_names_in, replace_callable_markers, replace_dbt_ref_markers, replace_named_markers, }; +use crate::compiler::_helpers::sql_tests::reader_references::{ + calls_reference, mock_cte_example, reader_ref_targets, unresolved_relation_call, +}; use crate::compiler::_helpers::sql_tests::relation_markers::relation_marker_calls; use crate::compiler::_helpers::sql_tests::rendering::{ AssertionStep, ChainStep, RenderRequest, render_comparison_sql, render_dialect, @@ -154,8 +157,6 @@ enum TestPayload { expected_model_names: Vec, #[serde(default)] assertion_ctes: Vec, - #[serde(default)] - helper_target_model_names: Vec, }, Direct { mode: String, @@ -222,12 +223,12 @@ struct ProjectContext { #[derive(Clone)] pub(crate) struct SqlTestPatterns { - reference: Regex, - source: Regex, - seed: Regex, + pub(crate) reference: Regex, + pub(crate) source: Regex, + pub(crate) seed: Regex, pub(crate) udf: Regex, table_function: Regex, - dbt_reference: Regex, + pub(crate) dbt_reference: Regex, pub(crate) test_reference: Regex, pub(crate) lexical: LexicalSyntax, pub(crate) identifier: Regex, @@ -302,7 +303,6 @@ struct ModelTestPlan { expected_ctes: Vec, expected_model_names: Vec, assertion_ctes: Vec, - helper_target_model_names: Vec, } struct TextualChainRequest<'a> { @@ -580,18 +580,12 @@ pub(crate) fn resolve_chains_json(request_json: &str) -> Result TestPayload::Model { authored_ctes, model_query_overrides, + expected_ctes, expected_model_names, assertion_ctes, - helper_target_model_names, - .. } => { - let fixtures = classify_fixtures(authored_ctes, Vec::new(), assertion_ctes); - let expected_names = chain_root_names( - expected_model_names, - helper_target_model_names, - &fixtures, - &patterns, - ); + let fixtures = classify_fixtures(authored_ctes, expected_ctes, assertion_ctes); + let expected_names = chain_root_names(expected_model_names, &fixtures, &patterns); topo_sort_model_chain(TopoSortRequest { expected_names: &expected_names, models: &models, @@ -605,15 +599,13 @@ pub(crate) fn resolve_chains_json(request_json: &str) -> Result serde_json::to_string(&ChainBatchResponse { chains }).map_err(|error| error.to_string()) } -/// Models the test runs: expected models, assertion targets, and the compiler's helper targets. +/// Models the test runs: expected models and the unmocked models its test CTEs call `__ref()` on. fn chain_root_names( mut expected_names: Vec, - helper_target_names: Vec, fixtures: &TestFixtures, patterns: &SqlTestPatterns, ) -> Vec { - expected_names.extend(assertion_ref_targets(&fixtures.assertions, patterns)); - expected_names.extend(helper_target_names); + expected_names.extend(reader_ref_targets(fixtures, patterns)); dedupe(expected_names) } @@ -747,7 +739,6 @@ fn plan_test(test: TestInput, context: &ProjectContext) -> Result plan_model_test( ModelTestPlan { test_name: test.name, @@ -757,7 +748,6 @@ fn plan_test(test: TestInput, context: &ProjectContext) -> Result = Vec::new(); for (assertion_name, assertion_sql) in &fixtures.assertions { let placed_sql = fixtures.scope.reader_sql(assertion_sql, &context.patterns); - let resolved = readers.resolve(&placed_sql, &fixtures)?; + let resolved = readers.resolve( + &placed_sql, + &fixtures, + &format!("{ASSERT_PREFIX}{assertion_name}"), + )?; reachable_mocks.extend(resolved.chain_reached); reachable_mocks.extend(resolved.reached); let mut resolved_sql = resolved.resolved_sql; @@ -1044,7 +1033,8 @@ fn resolve_referencing_helpers(request: HelperResolution<'_, '_>) -> Result<(), .read_helpers(&read_sql, reachable_mocks, patterns); let mut resolved_helpers: Vec<(usize, ResolvedHelper)> = Vec::new(); for (index, helper_sql) in fixtures.scope.referencing_helpers(&read_helpers, patterns) { - let resolved = readers.resolve(&helper_sql, fixtures)?; + let label = fixtures.scope.helper_name(index).to_string(); + let resolved = readers.resolve(&helper_sql, fixtures, &label)?; reachable_mocks.extend(resolved.chain_reached); resolved_helpers.push(( index, @@ -1055,18 +1045,32 @@ fn resolve_referencing_helpers(request: HelperResolution<'_, '_>) -> Result<(), }, )); } - if resolved_helpers.is_empty() { - return Ok(()); - } + let helpers_resolved = !resolved_helpers.is_empty(); for (index, resolved) in resolved_helpers { fixtures.scope.resolve_helper(index, resolved); } for step in chain { - if let Some(sql) = fixtures.expected.get(&step.model_name) { - let scope = helper_scope_ctes(sql, fixtures, patterns, readers.file_label)?; - reachable_mocks.extend(scope.reached_mocks); + let Some(sql) = fixtures.expected.get(&step.model_name) else { + continue; + }; + let placed_sql = fixtures.scope.reader_sql(sql, patterns); + let references = calls_reference(&placed_sql, patterns); + if !helpers_resolved && !references { + continue; + } + let scope = helper_scope_ctes(sql, fixtures, patterns, readers.file_label)?; + reachable_mocks.extend(scope.reached_mocks.iter().cloned()); + if references { + let label = format!("{EXPECTED_PREFIX}{}", step.model_name); + let resolved = readers.resolve(&placed_sql, fixtures, &label)?; + reachable_mocks.extend(resolved.chain_reached); + reachable_mocks.extend(resolved.reached); + step.expected_lifted_ctes = + merged_scoped_ctes(resolved.lifted_ctes, scope, readers.file_label)?; + step.expected_cte_sql = Some(resolved.body_sql); + } else { step.expected_lifted_ctes = scope.ctes; - step.expected_cte_sql = Some(fixtures.scope.reader_sql(sql, patterns)); + step.expected_cte_sql = Some(placed_sql); } } Ok(()) @@ -1093,8 +1097,31 @@ struct ResolvedReader { } impl ReaderContext<'_> { - /// Resolve an assertion or helper query against the planned chain, mocks and functions. - fn resolve(&mut self, sql: &str, fixtures: &TestFixtures) -> Result { + /// Resolve a test-authored query, failing rather than leaving a relation reference raw. + fn resolve( + &mut self, + sql: &str, + fixtures: &TestFixtures, + label: &str, + ) -> Result { + let resolved = self.resolve_references(sql, fixtures)?; + if let Some(call) = unresolved_relation_call(&resolved.body_sql, &self.context.patterns) { + return Err(compile_error(&format!( + "SQL test '{}' CTE '{label}' calls {call}, which the test query cannot resolve; \ + mock the relation with a CTE such as {} AS (SELECT ...), or read a model the \ + test runs with __ref(\"\")", + self.file_label, + mock_cte_example(&call) + ))); + } + Ok(resolved) + } + + fn resolve_references( + &mut self, + sql: &str, + fixtures: &TestFixtures, + ) -> Result { let context = self.context; let table_functions = resolve_table_function_fixtures(sql, fixtures, &context.patterns)?; let mut reached: HashSet = table_functions.reached.clone(); @@ -1854,17 +1881,6 @@ fn unreachable_mock_warnings( warnings } -fn assertion_ref_targets( - assertions: &[(String, String)], - patterns: &SqlTestPatterns, -) -> Vec { - let mut targets: Vec = Vec::new(); - for (_, sql) in assertions { - targets.extend(marker_names(&patterns.reference, &patterns.lexical, sql)); - } - dedupe(targets) -} - fn has_unresolved_test_reference(sql: &str, patterns: &SqlTestPatterns) -> bool { if !patterns.test_reference.is_match(sql) { return false; diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs new file mode 100644 index 0000000000..3b13a4e496 --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs @@ -0,0 +1,68 @@ +//! Relation references test-authored SQL-test CTEs call, and which models they make a test run. + +use crate::compiler::_helpers::sql_tests::helper_scope::read_helper_sql; +use crate::compiler::_helpers::sql_tests::markers::{ProtectedRanges, marker_names}; +use crate::compiler::_helpers::sql_tests::planning::{SqlTestPatterns, TestFixtures}; + +/// Whether test-authored SQL calls a reference or function SQLBuild resolves for the test query. +pub(crate) fn calls_reference(sql: &str, patterns: &SqlTestPatterns) -> bool { + patterns.test_reference.is_match(sql) || patterns.udf.is_match(sql) +} + +/// The first relation reference call left outside comments and quoted text, as written. +pub(crate) fn unresolved_relation_call(sql: &str, patterns: &SqlTestPatterns) -> Option { + if !patterns.test_reference.is_match(sql) { + return None; + } + let mut protected = ProtectedRanges::new(&patterns.lexical, sql); + for pattern in [ + &patterns.reference, + &patterns.source, + &patterns.seed, + &patterns.dbt_reference, + ] { + for found in pattern.find_iter(sql) { + if !protected.contains(found.start()) { + return Some(found.as_str().to_string()); + } + } + } + None +} + +/// The mock CTE that would stand in for one reference call, such as `__source__orders`. +pub(crate) fn mock_cte_example(call: &str) -> String { + let names: Vec<&str> = call.split('"').skip(1).step_by(2).collect(); + let function = call + .split('(') + .next() + .unwrap_or_default() + .to_ascii_lowercase(); + format!("{function}__{}", names.join("__")) +} + +/// Unmocked models the test's assertions, expected rows and the helpers they read call `__ref()` on. +pub(crate) fn reader_ref_targets( + fixtures: &TestFixtures, + patterns: &SqlTestPatterns, +) -> Vec { + let readers: Vec<&str> = fixtures + .assertions + .iter() + .map(|(_, sql)| sql.as_str()) + .chain(fixtures.expected.values().map(String::as_str)) + .collect(); + let mut targets: Vec = Vec::new(); + for sql in readers + .iter() + .copied() + .chain(read_helper_sql(fixtures, &readers, patterns)) + { + targets.extend( + marker_names(&patterns.reference, &patterns.lexical, sql) + .into_iter() + .filter(|name| !fixtures.mock_refs.contains_key(name)), + ); + } + targets +} diff --git a/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs b/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs index 88b54d4943..ae2da50e57 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs @@ -1833,8 +1833,7 @@ fn helper_reference_test(case: &HelperReferenceTestCase) -> Value { .iter() .map(|(name, _)| name.trim_start_matches("__expected__")) .collect::>(), - "assertionCtes": named(case.assertions), - "helperTargetModelNames": case.helper_targets + "assertionCtes": named(case.assertions) } }) } @@ -1854,21 +1853,27 @@ fn helper_reference_models() -> Value { ]) } +/// Plan and render one helper-reference case through the native planner's JSON entry point. +pub(crate) fn plan_helper_reference_response( + case: &HelperReferenceTestCase, +) -> Result { + crate::compiler::main::sql_test_planning::plan_and_render_json( + &json!({ + "lexicalSyntax": generic_lexical_syntax(), + "models": helper_reference_models(), + "tests": [helper_reference_test(case)], + "sqlAnalysisEnabled": case.sql_analysis_enabled, + "sqlAnalysisDialect": "duckdb", + "setDifferenceOperator": "EXCEPT" + }) + .to_string(), + ) +} + /// Plan and render one helper-reference case, returning its SQL, chain and warnings. pub(crate) fn plan_helper_reference_case(case: &HelperReferenceTestCase) -> Value { let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( - &json!({ - "lexicalSyntax": generic_lexical_syntax(), - "models": helper_reference_models(), - "tests": [helper_reference_test(case)], - "sqlAnalysisEnabled": case.sql_analysis_enabled, - "sqlAnalysisDialect": "duckdb", - "setDifferenceOperator": "EXCEPT" - }) - .to_string(), - ) - .expect("test assumption must hold"), + &plan_helper_reference_response(case).expect("test assumption must hold"), ) .expect("test assumption must hold"); response["artifacts"][0].clone() diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_helper_references.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_helper_references.rs index f05fd17ccf..3979ae8fb1 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_helper_references.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_helper_references.rs @@ -2,13 +2,29 @@ use serde_json::{Value, json}; use crate::compiler::tests::helpers::{ chain_helper_reference_case, defined_before, plan_helper_reference_case, + plan_helper_reference_response, +}; +use crate::compiler::tests::test_types::{ + HelperReferenceTestCase, UnresolvedReaderReferenceTestCase, }; -use crate::compiler::tests::test_types::HelperReferenceTestCase; const HELPER_READS_MODEL: (&str, &str) = ( "doubled", "SELECT order_id, amount_doubled FROM __ref(\"orders\")", ); +const MOCK_STG_ORDERS: (&str, &str) = ( + "__ref__stg_orders", + "SELECT 5 AS order_id, 10 AS amount, 7 AS region_id", +); +const REAL_STG_ORDERS_BODY: &str = "SELECT order_id, amount, region_id FROM __source__raw_orders"; +const ASSERT_MOCK_ROWS: (&str, &str) = ( + "__assert__mock_rows", + "SELECT order_id FROM __ref(\"stg_orders\") WHERE order_id <> 5", +); +const EXPECTED_FROM_MOCK: (&str, &str) = ( + "__expected__orders", + "SELECT order_id, amount * 2 AS amount_doubled FROM __ref(\"stg_orders\")", +); const ASSERT_DOUBLED: (&str, &str) = ( "__assert__doubles_amount", "SELECT order_id FROM doubled WHERE amount_doubled <> 20", @@ -23,7 +39,6 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen helpers: &[HELPER_READS_MODEL], expected: &[], assertions: &[ASSERT_DOUBLED], - helper_targets: &["orders"], expected_chain: &["stg_orders", "orders"], expected_order: &[ ("__source__raw_orders", "__ref__stg_orders"), @@ -42,7 +57,6 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen helpers: &[HELPER_READS_MODEL], expected: &[], assertions: &[ASSERT_DOUBLED], - helper_targets: &["orders"], expected_chain: &["stg_orders", "orders"], expected_order: &[ ("__source__raw_orders", "__ref__orders"), @@ -73,7 +87,6 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "__assert__has_region", "SELECT order_id FROM with_region WHERE region_name IS NULL", )], - helper_targets: &["stg_orders", "orders"], expected_chain: &["stg_orders", "orders"], expected_order: &[ ("__ref__orders", "__helper__base"), @@ -101,7 +114,6 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "SELECT order_id, amount_doubled FROM expected_rows", )], assertions: &[], - helper_targets: &[], expected_chain: &["stg_orders", "orders"], expected_order: &[ ("__source__raw_orders", "__helper__expected_rows"), @@ -122,7 +134,6 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "SELECT order_id, amount_doubled FROM doubled", )], assertions: &[], - helper_targets: &["orders"], expected_chain: &["stg_orders", "orders"], expected_order: &[("__ref__orders", "__expected__orders")], expected_fragments: &[ @@ -150,8 +161,7 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "SELECT order_id FROM doubled JOIN __ref(\"stg_orders\") USING (order_id) \ WHERE amount_doubled <> 20", )], - helper_targets: &["orders"], - expected_chain: &["orders", "stg_orders"], + expected_chain: &["orders"], expected_order: &[ ("__helper__base_ids", "__ref__orders"), ("__ref__orders", "__helper__doubled"), @@ -169,12 +179,102 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "SELECT 1 AS order_id, 10 AS amount, 7 AS region_id", )], assertions: &[], - helper_targets: &[], expected_chain: &["stg_orders"], expected_order: &[("__source__raw_orders", "__expected__stg_orders")], expected_fragments: &[], expected_absent_fragments: &["unused_rows", "__ref__orders"], }, + HelperReferenceTestCase { + description: "assertion reading a mocked model reads the mock, not the model", + sql_analysis_enabled: true, + helpers: &[MOCK_STG_ORDERS], + expected: &[( + "__expected__orders", + "SELECT 5 AS order_id, 20 AS amount_doubled", + )], + assertions: &[ASSERT_MOCK_ROWS], + expected_chain: &["orders"], + expected_order: &[("__ref__stg_orders", "__assert__mock_rows")], + expected_fragments: &["FROM __ref__stg_orders WHERE order_id <> 5"], + expected_absent_fragments: &[REAL_STG_ORDERS_BODY, "__sqb_cte_"], + }, + HelperReferenceTestCase { + description: "assertion reading a mocked model reads the mock without sql analysis", + sql_analysis_enabled: false, + helpers: &[MOCK_STG_ORDERS], + expected: &[], + assertions: &[ + ASSERT_MOCK_ROWS, + ( + "__assert__doubled", + "SELECT order_id FROM __ref(\"orders\") WHERE amount_doubled <> 20", + ), + ], + expected_chain: &["orders"], + expected_order: &[ + ("__ref__stg_orders", "__ref__orders"), + ("__ref__orders", "__assert__doubled"), + ], + expected_fragments: &["FROM __ref__stg_orders WHERE order_id <> 5"], + expected_absent_fragments: &[REAL_STG_ORDERS_BODY, "__sqb_cte_"], + }, + HelperReferenceTestCase { + description: "expected rows reading a mocked model resolve to the mock", + sql_analysis_enabled: true, + helpers: &[MOCK_STG_ORDERS], + expected: &[EXPECTED_FROM_MOCK], + assertions: &[], + expected_chain: &["orders"], + expected_order: &[("__ref__stg_orders", "__expected__orders")], + expected_fragments: &[ + "__expected__orders AS (SELECT order_id, amount * 2 AS amount_doubled FROM __ref__stg_orders)", + ], + expected_absent_fragments: &[REAL_STG_ORDERS_BODY, "__sqb_cte_"], + }, + HelperReferenceTestCase { + description: "expected rows reading a mocked model resolve to the mock without sql analysis", + sql_analysis_enabled: false, + helpers: &[MOCK_STG_ORDERS], + expected: &[EXPECTED_FROM_MOCK], + assertions: &[], + expected_chain: &["orders"], + expected_order: &[("__ref__stg_orders", "__expected__orders")], + expected_fragments: &[ + "__expected__orders AS (SELECT order_id, amount * 2 AS amount_doubled FROM __ref__stg_orders)", + ], + expected_absent_fragments: &[REAL_STG_ORDERS_BODY, "__sqb_cte_"], + }, + HelperReferenceTestCase { + description: "expected rows reading an unmocked model run that model", + sql_analysis_enabled: true, + helpers: &[], + expected: &[EXPECTED_FROM_MOCK], + assertions: &[], + expected_chain: &["stg_orders", "orders"], + expected_order: &[("__ref__stg_orders", "__expected__orders")], + expected_fragments: &["amount * 2 AS amount_doubled FROM __ref__stg_orders)"], + expected_absent_fragments: &[], + }, + HelperReferenceTestCase { + description: "expected rows reading a source and a seed resolve to their mocks without sql analysis", + sql_analysis_enabled: false, + helpers: &[], + expected: &[( + "__expected__stg_orders", + "SELECT s.order_id, s.amount, r.region_id FROM __source(\"raw_orders\") AS s \ + JOIN __seed(\"regions\") AS r USING (region_id)", + )], + assertions: &[], + expected_chain: &["stg_orders"], + expected_order: &[ + ("__source__raw_orders", "__expected__stg_orders"), + ("__seed__regions", "__expected__stg_orders"), + ], + expected_fragments: &[ + "FROM __source__raw_orders AS s JOIN __seed__regions AS r USING (region_id)", + ], + expected_absent_fragments: &[], + }, ]; for test_case in test_cases { let artifact: Value = plan_helper_reference_case(&test_case); @@ -214,9 +314,87 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen test_case.description ); } - for marker in ["__ref(", "__source(", "__seed("] { + for marker in ["__ref(", "__source(", "__seed(", "__dbt_ref("] { assert!(!sql.contains(marker), "{}: {sql}", test_case.description); } assert!(errors.is_empty(), "{}: {errors:?}", test_case.description); } } + +#[test] +fn given_unresolvable_reader_references_when_planning_directly_then_planner_rejects_test() { + let test_cases = [ + UnresolvedReaderReferenceTestCase { + description: "assertion calling an unmocked source", + sql_analysis_enabled: true, + helpers: &[], + expected: &[( + "__expected__stg_orders", + "SELECT 1 AS order_id, 10 AS amount, 7 AS region_id", + )], + assertions: &[( + "__assert__no_returns", + "SELECT order_id FROM __source(\"returns\")", + )], + expected_error_fragments: &[ + "compile_input:", + "CTE '__assert__no_returns' calls __source(\"returns\")", + "__source__returns AS (SELECT ...)", + ], + }, + UnresolvedReaderReferenceTestCase { + description: "expected rows calling an unmocked seed without sql analysis", + sql_analysis_enabled: false, + helpers: &[], + expected: &[( + "__expected__stg_orders", + "SELECT order_id, amount, region_id FROM __seed(\"returns\")", + )], + assertions: &[], + expected_error_fragments: &[ + "CTE '__expected__stg_orders' calls __seed(\"returns\")", + "__seed__returns AS (SELECT ...)", + ], + }, + UnresolvedReaderReferenceTestCase { + description: "helper read by an assertion calling an unmocked dbt model", + sql_analysis_enabled: true, + helpers: &[( + "legacy_orders", + "SELECT order_id FROM __dbt_ref(\"warehouse\", \"orders\")", + )], + expected: &[( + "__expected__stg_orders", + "SELECT 1 AS order_id, 10 AS amount, 7 AS region_id", + )], + assertions: &[("__assert__no_legacy", "SELECT order_id FROM legacy_orders")], + expected_error_fragments: &[ + "CTE 'legacy_orders' calls __dbt_ref(\"warehouse\", \"orders\")", + "__dbt_ref__warehouse__orders AS (SELECT ...)", + ], + }, + ]; + for test_case in test_cases { + let error = plan_helper_reference_response(&HelperReferenceTestCase { + description: test_case.description, + sql_analysis_enabled: test_case.sql_analysis_enabled, + helpers: test_case.helpers, + expected: test_case.expected, + assertions: test_case.assertions, + expected_chain: &[], + expected_order: &[], + expected_fragments: &[], + expected_absent_fragments: &[], + }) + .expect_err(test_case.description); + + assert!( + test_case + .expected_error_fragments + .iter() + .all(|fragment| error.contains(fragment)), + "{}: {error}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs index d29dd82315..f76bbcf0fb 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs @@ -125,7 +125,6 @@ pub(crate) struct HelperReferenceTestCase { pub(crate) helpers: &'static [(&'static str, &'static str)], pub(crate) expected: &'static [(&'static str, &'static str)], pub(crate) assertions: &'static [(&'static str, &'static str)], - pub(crate) helper_targets: &'static [&'static str], pub(crate) expected_chain: &'static [&'static str], pub(crate) expected_order: &'static [(&'static str, &'static str)], pub(crate) expected_fragments: &'static [&'static str], @@ -137,3 +136,12 @@ pub(crate) struct ScenarioExtractionTestCase { pub(crate) sql: &'static str, pub(crate) expected_json: Option<&'static str>, } + +pub(crate) struct UnresolvedReaderReferenceTestCase { + pub(crate) description: &'static str, + pub(crate) sql_analysis_enabled: bool, + pub(crate) helpers: &'static [(&'static str, &'static str)], + pub(crate) expected: &'static [(&'static str, &'static str)], + pub(crate) assertions: &'static [(&'static str, &'static str)], + pub(crate) expected_error_fragments: &'static [&'static str], +} diff --git a/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py index 28a6eacca7..e29fef2b82 100644 --- a/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py @@ -271,6 +271,44 @@ def _reference_cases() -> tuple[FailureCase, ...]: ) }, ), + failure_case( + name="test-assertion-reads-unmocked-source", + expected_code="P013", + expected_message=( + "SQL test assertion CTE '__assert__no_negative_orders' calls " + '__source("raw_orders"), which the test does not mock' + ), + files={ + "tests/unit/test_customer_totals.sql": ( + "TEST();\n\nWITH\n__ref__stg_orders AS (\n" + " SELECT 1 AS order_id, 10 AS customer_id, CAST(5 AS DOUBLE) AS amount," + " 'placed' AS status\n),\n" + "__expected__customer_totals AS (\n" + " SELECT 10 AS customer_id, CAST(5 AS DOUBLE) AS total_amount\n),\n" + "__assert__no_negative_orders AS (\n" + ' SELECT order_id FROM __source("raw_orders") WHERE amount < 0\n' + ")\nSELECT 1\n" + ) + }, + ), + failure_case( + name="test-expected-reads-unknown-model", + expected_code="P013", + expected_message=( + "SQL test expected CTE '__expected__stg_orders' references unknown model " + "'archived_orders'" + ), + files={ + "tests/unit/test_stg_orders.sql": ( + "TEST();\n\nWITH\n__source__raw_orders AS (\n" + " SELECT 1 AS order_id, 10 AS customer_id, CAST(5 AS DOUBLE) AS amount," + " 'placed' AS status\n),\n" + "__expected__stg_orders AS (\n" + ' SELECT order_id FROM __ref("archived_orders")\n' + ")\nSELECT 1\n" + ) + }, + ), failure_case( name="dbt-reference-without-manifest", expected_code="C214", 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 5eadc2239a..2c136d719d 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md @@ -165,11 +165,18 @@ __assert__no_unexpected_order_ids AS ( SQLBuild emits each helper and mock a query needs once, in dependency order, including mocks that a helper reaches through another mock. -Helpers can also call the references an `__assert__` CTE can, such as `__ref()`, `__source()` and -`__seed()`, and they resolve exactly as there: a mocked relation reads its mock, and an unmocked -`__ref("")` reads the model's real SQL, so the test runs that model and -`sqb test --select ` selects the test even when only a helper reads it. SQLBuild emits a -helper after the mocks and models it reads: +`__expected__` and `__assert__` CTEs, and the helpers they read, can call `__ref()`, +`__source()`, `__seed()` and `__dbt_ref()`. They resolve the same way in every test CTE: + +- A mocked name always means the mock. `__ref("stg_orders")` reads `__ref__stg_orders` when the + test defines it, and a mocked model is neither run nor counted as a model the test covers, so + `sqb test --select stg_orders` does not select the test because of it. +- An unmocked `__ref("")` reads the model's real SQL, so the test runs that model and + `sqb test --select ` selects the test, even when only a helper or expected rows read it. +- `__source()`, `__seed()` and `__dbt_ref()` must be mocked, for example with + `__source__raw__orders AS (SELECT ...)`. + +SQLBuild emits a helper after the mocks and models it reads: ```sql TEST(); @@ -189,9 +196,9 @@ __assert__quantities_are_positive AS ( Only helpers the test reads count: those its `__expected__` and `__assert__` CTEs reach, directly or through other helpers and mocks. A helper nothing reads is left out of the test query, and its references are neither checked nor run. Compile rejects references the test query cannot resolve -with `P013`, at the helper or call that causes it: a `__ref()` to a model that does not exist, -helpers or mocks that read each other in a cycle, and a mock or `__table_fn__` fixture the test -uses that reads a helper calling a reference. Mocks are defined before the models a test runs, so +with `P013`, at the CTE or call that causes it: a `__ref()` to a model that does not exist, an +unmocked `__source()`, `__seed()` or `__dbt_ref()`, helpers or mocks that read each other in a +cycle, and a mock or `__table_fn__` fixture the test uses that reads a helper calling a reference. Mocks are defined before the models a test runs, so a mock reads other mocks by CTE name, such as `FROM __ref__stg_orders`, rather than through `__ref("stg_orders")`. @@ -446,6 +453,10 @@ __assert__order_ids_are_not_null AS ( ) ``` +The assertion reads the `__ref__stg_orders` mock, not the real `stg_orders` model: a mocked name +always means the mock. Earlier SQLBuild versions ran the model's real SQL for an assertion calling +`__ref()` on a mocked model. + Assertions can be mixed with `__expected__` CTEs in the same test, or used on their own. They are useful when the natural check is "no rows should violate this rule" rather than "the output should exactly equal these rows" - for example, duplicate checks, negative-value constraints, or conditional business rules. During `sqb test` and `sqb build`, assertion results appear as nested check rows alongside expected comparisons. diff --git a/src/sqlbuild/compiler/compile/_helpers/assembly/project.py b/src/sqlbuild/compiler/compile/_helpers/assembly/project.py index 0d8e1b8c9d..4b6595deb4 100644 --- a/src/sqlbuild/compiler/compile/_helpers/assembly/project.py +++ b/src/sqlbuild/compiler/compile/_helpers/assembly/project.py @@ -1752,7 +1752,7 @@ def _assemble_compiled_sql_test( ( *model_payload.expected_model_names, *model_payload.assertion_target_model_names, - *model_payload.helper_target_model_names, + *model_payload.reference_target_model_names, ) ) ) @@ -1841,11 +1841,6 @@ def _assemble_compiled_sql_test( if isinstance(test_input.payload, CompileModelSqlTestInputPayload) else () ), - helper_target_model_names=( - test_input.payload.helper_target_model_names - if isinstance(test_input.payload, CompileModelSqlTestInputPayload) - else () - ), target_model_names=target_model_names, tested_resources=tested_resources, ) diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py b/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py index a2ad27f925..356e3a8bb4 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py @@ -38,8 +38,8 @@ extract_sql_test_ctes, ) from sqlbuild.compiler.compile._helpers.sql_tests.helper_ctes import ( - helper_target_model_names, - report_unresolvable_helper_ctes, + reference_target_model_names, + report_unresolvable_test_references, ) from sqlbuild.compiler.compile._helpers.sql_tests.native import ( extract_expanded_sql_tests_cached, @@ -56,7 +56,6 @@ CompileSqlScenarioCte, CompileSqlScenarioCtes, CompileSqlScenarioInput, - CompileSqlTestCte, CompileSqlTestCtes, CompileSqlTestInput, DeclarationExpansionContext, @@ -321,10 +320,8 @@ def build_test_inputs( ) for test, test_ctes in zip(expanded_tests, test_ctes_batch, strict=True): assertion_target_model_names: tuple[str, ...] = ( - extract_assertion_target_model_names( - assertion_sql=tuple(cte.sql_body for cte in test_ctes.payload.assertion_ctes), - syntax=sql_lexical_syntax, - origin=_sql_file_reference_origin(test.test_file), + _unmocked_assertion_targets( + payload=test_ctes.payload, test=test, syntax=sql_lexical_syntax ) if isinstance(test_ctes.payload, CompileModelSqlTestCtes) else () @@ -340,27 +337,18 @@ def build_test_inputs( assertion_target_model_names=assertion_target_model_names, target_catalog=target_catalog, ) - helper_targets: tuple[str, ...] = () + reference_targets: tuple[str, ...] = () if isinstance(test_ctes.payload, CompileModelSqlTestCtes): - reader_ctes: tuple[CompileSqlTestCte, ...] = ( - *test_ctes.payload.expected_ctes, - *test_ctes.payload.assertion_ctes, - ) - if report_unresolvable_helper_ctes( - authored_ctes=test_ctes.payload.authored_ctes, - reader_ctes=reader_ctes, - mock_model_names=test_ctes.payload.mock_model_names, + if report_unresolvable_test_references( + payload=test_ctes.payload, test_file=test.test_file, test_block=test.test_block, known_model_names=known_model_names, syntax=sql_lexical_syntax, ): continue - helper_targets = helper_target_model_names( - authored_ctes=test_ctes.payload.authored_ctes, - reader_ctes=reader_ctes, - mock_model_names=test_ctes.payload.mock_model_names, - syntax=sql_lexical_syntax, + reference_targets = reference_target_model_names( + payload=test_ctes.payload, syntax=sql_lexical_syntax ) test_inputs.append( CompileSqlTestInput( @@ -372,7 +360,7 @@ def build_test_inputs( test_ctes=test_ctes, tested_resource_names=test.tested_resource_names, assertion_target_model_names=assertion_target_model_names, - helper_target_model_names=helper_targets, + reference_target_model_names=reference_targets, ), declaration_usages=test.declaration_usages, parent_name=test.parent_name, @@ -385,6 +373,19 @@ def build_test_inputs( return tuple(test_inputs) +def _unmocked_assertion_targets( + *, payload: CompileModelSqlTestCtes, test: _ExpandedSqlTest, syntax: SqlLexicalSyntax +) -> tuple[str, ...]: + """Return models the assertions call ``__ref()`` on that the test does not mock.""" + + targets: tuple[str, ...] = extract_assertion_target_model_names( + assertion_sql=tuple(cte.sql_body for cte in payload.assertion_ctes), + syntax=syntax, + origin=_sql_file_reference_origin(test.test_file), + ) + return tuple(name for name in targets if name not in payload.mock_model_names) + + def _project_test_declaration_consumer( *, context: DeclarationExpansionContext, consumer: ResourceIdentity ) -> DeclarationExpansionContext: @@ -434,7 +435,7 @@ def _build_test_input_payload( test_ctes: CompileSqlTestCtes, tested_resource_names: tuple[str, ...], assertion_target_model_names: tuple[str, ...], - helper_target_model_names: tuple[str, ...], + reference_target_model_names: tuple[str, ...], ) -> CompileModelSqlTestInputPayload | CompileDirectLogicSqlTestInputPayload: match test_ctes.payload: case CompileModelSqlTestCtes() as model_payload: @@ -451,7 +452,7 @@ def _build_test_input_payload( assertion_ctes=model_payload.assertion_ctes, assertion_names=model_payload.assertion_names, assertion_target_model_names=assertion_target_model_names, - helper_target_model_names=helper_target_model_names, + reference_target_model_names=reference_target_model_names, ) case CompileDirectLogicSqlTestCtes() as direct_logic_payload: return CompileDirectLogicSqlTestInputPayload( diff --git a/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py b/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py index 1563f60c57..0d23325661 100644 --- a/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py +++ b/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py @@ -21,6 +21,7 @@ ) from sqlbuild.compiler.compile.models import ( CompileModelInput, + CompileModelSqlTestCtes, CompilerDiagnostic, CompileSqlReference, CompileSqlTestCte, @@ -110,71 +111,100 @@ def reachable_cte_keys(*, graph: SqlTestCteGraph, roots: Iterable[str]) -> tuple return tuple(visited) -def helper_target_model_names( - *, - authored_ctes: tuple[CompileSqlTestCte, ...], - reader_ctes: tuple[CompileSqlTestCte, ...], - mock_model_names: tuple[str, ...], - syntax: SqlLexicalSyntax, +def reference_target_model_names( + *, payload: CompileModelSqlTestCtes, syntax: SqlLexicalSyntax ) -> tuple[str, ...]: - """Return unmocked models that helpers the test reads call ``__ref()`` on.""" + """Return unmocked models the test's expected rows and the helpers it reads ``__ref()``.""" graph: SqlTestCteGraph = sql_test_cte_graph( - authored_ctes=authored_ctes, reader_ctes=reader_ctes + authored_ctes=payload.authored_ctes, reader_ctes=_reader_ctes(payload) + ) + mocked: frozenset[str] = frozenset(payload.mock_model_names) + sqls: tuple[str, ...] = ( + *(cte.sql_body for cte in payload.expected_ctes), + *(graph.ctes[key].sql_body for key in _read_helper_keys(graph)), ) - mocked: frozenset[str] = frozenset(mock_model_names) targets: list[str] = [] - for key in _read_helper_keys(graph): + for sql in sqls: targets.extend( reference.ref_name - for reference in _references(sql=graph.ctes[key].sql_body, syntax=syntax) + for reference in _references(sql=sql, syntax=syntax) if reference.ref_kind == SqlReferenceKind.REF and reference.ref_name not in mocked ) return tuple(dict.fromkeys(targets)) -def report_unresolvable_helper_ctes( +def report_unresolvable_test_references( *, - authored_ctes: tuple[CompileSqlTestCte, ...], - reader_ctes: tuple[CompileSqlTestCte, ...], - mock_model_names: tuple[str, ...], + payload: CompileModelSqlTestCtes, test_file: DiscoveredSqlTestFile, test_block: DiscoveredSqlTestBlock, known_model_names: set[str], syntax: SqlLexicalSyntax, ) -> bool: - """Report read helpers with unknown models or read cycles; return whether any was found.""" + """Report unresolvable references in expected rows, assertions and read helpers, or cycles.""" graph: SqlTestCteGraph = sql_test_cte_graph( - authored_ctes=authored_ctes, reader_ctes=reader_ctes + authored_ctes=payload.authored_ctes, reader_ctes=_reader_ctes(payload) ) read_helpers: tuple[str, ...] = _read_helper_keys(graph) - if not read_helpers: - return False + readers: tuple[tuple[str, CompileSqlTestCte], ...] = ( + *(("helper CTE", graph.ctes[key]) for key in read_helpers), + *(("expected CTE", cte) for cte in payload.expected_ctes), + *(("assertion CTE", cte) for cte in payload.assertion_ctes), + ) reporter: _HelperDiagnostics = _HelperDiagnostics(test_file=test_file, test_block=test_block) - mocked: frozenset[str] = frozenset(mock_model_names) - for key in read_helpers: - cte: CompileSqlTestCte = graph.ctes[key] + mocked: dict[SqlReferenceKind, frozenset[str]] = { + SqlReferenceKind.REF: frozenset(payload.mock_model_names), + SqlReferenceKind.SOURCE: frozenset(payload.mock_source_names), + SqlReferenceKind.SEED: frozenset(payload.mock_seed_names), + SqlReferenceKind.DBT_REF: frozenset(payload.mock_dbt_ref_names), + } + for label, cte in readers: for reference in _references(sql=cte.sql_body, syntax=syntax): - if ( - reference.ref_kind == SqlReferenceKind.REF - and reference.ref_name not in mocked - and reference.ref_name not in known_model_names - ): + kind: SqlReferenceKind = SqlReferenceKind(reference.ref_kind) + if kind not in mocked: + continue + mock_name: str = ( + f"{reference.ref_package}__{reference.ref_name}" + if reference.ref_package is not None + else reference.ref_name + ) + if mock_name in mocked[kind]: + continue + if kind is SqlReferenceKind.REF: + if reference.ref_name in known_model_names: + continue reporter.report( cte_name=cte.name, - call=SqlReferenceKind.REF.example_call(reference.ref_name, quote='"'), + call=kind.example_call(reference.ref_name, quote='"'), message=( - f"SQL test helper CTE '{cte.name}' references unknown model " + f"SQL test {label} '{cte.name}' references unknown model " f"'{reference.ref_name}'" ), help=( 'Read a model defined in the project, for example __ref(""), ' - f"or remove the reference from helper CTE '{cte.name}'." + f"or remove the reference from {label} '{cte.name}'." ), ) - cycle: tuple[str, ...] | None = _first_cycle( - graph=graph, keys=reachable_cte_keys(graph=graph, roots=graph.reader_reads) + continue + call: str = _reference_call(reference) + reporter.report( + cte_name=cte.name, + call=call, + message=( + f"SQL test {label} '{cte.name}' calls {call}, which the test does not " + "mock, so the test query cannot resolve it" + ), + help=( + f"Mock it in the test, for example {kind.fixture_cte_prefix}{mock_name} " + f"AS (SELECT ...), or remove the call from {label} '{cte.name}'." + ), + ) + cycle: tuple[str, ...] | None = ( + _first_cycle(graph=graph, keys=reachable_cte_keys(graph=graph, roots=graph.reader_reads)) + if read_helpers + else None ) if cycle is not None: names: tuple[str, ...] = tuple(graph.ctes[key].name for key in cycle) @@ -264,6 +294,10 @@ def report_mocks_reading_referencing_helpers( ) +def _reader_ctes(payload: CompileModelSqlTestCtes) -> tuple[CompileSqlTestCte, ...]: + return (*payload.expected_ctes, *payload.assertion_ctes) + + def _read_helper_keys(graph: SqlTestCteGraph) -> tuple[str, ...]: return tuple( key diff --git a/src/sqlbuild/compiler/compile/models.py b/src/sqlbuild/compiler/compile/models.py index 91315721e6..aa92850760 100644 --- a/src/sqlbuild/compiler/compile/models.py +++ b/src/sqlbuild/compiler/compile/models.py @@ -1245,7 +1245,7 @@ class CompileModelSqlTestInputPayload: assertion_ctes: tuple[CompileSqlTestCte, ...] = field(default_factory=tuple) assertion_names: tuple[str, ...] = field(default_factory=tuple) assertion_target_model_names: tuple[str, ...] = field(default_factory=tuple) - helper_target_model_names: tuple[str, ...] = field(default_factory=tuple) + reference_target_model_names: tuple[str, ...] = field(default_factory=tuple) @dataclass(frozen=True) @@ -1394,7 +1394,6 @@ class CompiledSqlTest: expected_model_names: tuple[str, ...] = field(default_factory=tuple) assertion_names: tuple[str, ...] = field(default_factory=tuple) assertion_target_model_names: tuple[str, ...] = field(default_factory=tuple) - helper_target_model_names: tuple[str, ...] = field(default_factory=tuple) target_model_names: tuple[str, ...] = field(default_factory=tuple) tested_resources: tuple[CompiledSqlTestResource, ...] = field(default_factory=tuple) diff --git a/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py b/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py index d9ed8ff3c3..87c199d9ab 100644 --- a/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py +++ b/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py @@ -413,7 +413,6 @@ def _test_request( "expectedCtes": [_cte_request(cte=cte) for cte in payload.expected_ctes], "expectedModelNames": list(payload.expected_model_names), "assertionCtes": [_cte_request(cte=cte) for cte in payload.assertion_ctes], - "helperTargetModelNames": list(test.helper_target_model_names), } return { "name": test.name, 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 c33535881d..4b6ab57058 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py @@ -1805,6 +1805,34 @@ def prepare_optional_ceremony_project(*, tmp_path: Path, test_files: dict[str, s ' SELECT item_id FROM __ref("item_totals") JOIN missing_rows USING (item_id)\n' ")\n" ), + "mocked_model_read_by_expected_and_assertion": ( + "__ref__items AS (\n SELECT 7 AS item_id, 5 AS amount\n),\n" + "__expected__item_totals AS (\n" + ' SELECT item_id, amount * 2 AS amount_doubled FROM __ref("items")\n' + "),\n" + "__assert__reads_mocked_items AS (\n" + ' SELECT item_id FROM __ref("items") WHERE item_id <> 7 OR amount <> 5\n' + ")\n" + ), + "expected_reads_unmocked_model": ( + "__ref__items AS (\n SELECT 3 AS item_id, 4 AS amount\n),\n" + "__expected__item_totals AS (\n" + ' SELECT item_id, amount * 2 AS amount_doubled FROM __ref("item_extras")\n' + ")\n" + ), + "assertion_reads_unmocked_source": ( + "__ref__items AS (\n SELECT 1 AS item_id, 10 AS amount\n),\n" + "__expected__item_totals AS (\n SELECT 1 AS item_id, 20 AS amount_doubled\n),\n" + "__assert__no_returns AS (\n" + ' SELECT item_id FROM __source("item_returns")\n' + ")\n" + ), + "expected_reads_unknown_model": ( + "__ref__items AS (\n SELECT 1 AS item_id, 10 AS amount\n),\n" + "__expected__item_totals AS (\n" + ' SELECT item_id, amount_doubled FROM __ref("item_archive")\n' + ")\n" + ), "mock_reads_referencing_helper": ( "__ref__items AS (\n SELECT item_id, 10 AS amount FROM base_rows\n),\n" 'base_rows AS (SELECT item_id FROM __ref("item_totals")),\n' @@ -1829,6 +1857,12 @@ def build_helper_reference_project_files( "[settings]\n" f"sql_analysis = {str(sql_analysis).lower()}\n" ), + "sources/item_returns.yml": ( + "sources:\n" + " - name: item_returns\n" + " description: Returned items\n" + " expression: SELECT 1 AS item_id\n" + ), "models/item_extras.sql": ( "MODEL (description 'Extra item amounts', materialized table);\n\n" 'SELECT item_id, amount FROM __ref("items")\n' diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py index badcfe9370..9d03494a95 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py @@ -181,6 +181,189 @@ expected_compiled_fragments=(), expected_absent_compiled_fragments=("archived_rows", "other_rows"), ), + HelperReferenceE2ETestCase( + description="expected rows and assertion read a mock on the python engine", + tests=("mocked_model_read_by_expected_and_assertion",), + command=("--no-color", "test"), + engine="python", + sql_analysis=True, + expected_output_fragments=( + "PASS=1 FAIL=0 TOTAL=1", + "assertion reads_mocked_items", + ), + expected_compiled_fragments=( + "SELECT item_id, amount * 2 AS amount_doubled FROM __ref__items", + "SELECT item_id FROM __ref__items WHERE item_id <> 7 OR amount <> 5", + ), + expected_absent_compiled_fragments=( + "__ref(", + "SELECT 1 AS item_id, 10 AS amount", + ), + ), + HelperReferenceE2ETestCase( + description="expected rows and assertion read a mock on the python engine without sql analysis", + tests=("mocked_model_read_by_expected_and_assertion",), + command=("--no-color", "test"), + engine="python", + sql_analysis=False, + expected_output_fragments=( + "PASS=1 FAIL=0 TOTAL=1", + "assertion reads_mocked_items", + ), + expected_compiled_fragments=( + "SELECT item_id, amount * 2 AS amount_doubled FROM __ref__items", + "SELECT item_id FROM __ref__items WHERE item_id <> 7 OR amount <> 5", + ), + expected_absent_compiled_fragments=( + "__ref(", + "SELECT 1 AS item_id, 10 AS amount", + ), + ), + HelperReferenceE2ETestCase( + description="expected rows and assertion read a mock on the native engine", + tests=("mocked_model_read_by_expected_and_assertion",), + command=("--no-color", "test"), + engine="native", + sql_analysis=True, + expected_output_fragments=( + "PASS=1 FAIL=0 TOTAL=1", + "assertion reads_mocked_items", + ), + expected_compiled_fragments=( + "SELECT item_id, amount * 2 AS amount_doubled FROM __ref__items", + "SELECT item_id FROM __ref__items WHERE item_id <> 7 OR amount <> 5", + ), + expected_absent_compiled_fragments=( + "__ref(", + "SELECT 1 AS item_id, 10 AS amount", + ), + ), + HelperReferenceE2ETestCase( + description="expected rows and assertion read a mock on the native engine without sql analysis", + tests=("mocked_model_read_by_expected_and_assertion",), + command=("--no-color", "test"), + engine="native", + sql_analysis=False, + expected_output_fragments=( + "PASS=1 FAIL=0 TOTAL=1", + "assertion reads_mocked_items", + ), + expected_compiled_fragments=( + "SELECT item_id, amount * 2 AS amount_doubled FROM __ref__items", + "SELECT item_id FROM __ref__items WHERE item_id <> 7 OR amount <> 5", + ), + expected_absent_compiled_fragments=( + "__ref(", + "SELECT 1 AS item_id, 10 AS amount", + ), + ), + HelperReferenceE2ETestCase( + description="expected rows and assertion read a mock on the native-preview engine", + tests=("mocked_model_read_by_expected_and_assertion",), + command=("--no-color", "test"), + engine="native-preview", + sql_analysis=True, + expected_output_fragments=( + "PASS=1 FAIL=0 TOTAL=1", + "assertion reads_mocked_items", + ), + expected_compiled_fragments=( + "SELECT item_id, amount * 2 AS amount_doubled FROM __ref__items", + "SELECT item_id FROM __ref__items WHERE item_id <> 7 OR amount <> 5", + ), + expected_absent_compiled_fragments=( + "__ref(", + "SELECT 1 AS item_id, 10 AS amount", + ), + ), + HelperReferenceE2ETestCase( + description="expected rows and assertion read a mock on the native-preview engine without sql analysis", + tests=("mocked_model_read_by_expected_and_assertion",), + command=("--no-color", "test"), + engine="native-preview", + sql_analysis=False, + expected_output_fragments=( + "PASS=1 FAIL=0 TOTAL=1", + "assertion reads_mocked_items", + ), + expected_compiled_fragments=( + "SELECT item_id, amount * 2 AS amount_doubled FROM __ref__items", + "SELECT item_id FROM __ref__items WHERE item_id <> 7 OR amount <> 5", + ), + expected_absent_compiled_fragments=( + "__ref(", + "SELECT 1 AS item_id, 10 AS amount", + ), + ), + HelperReferenceE2ETestCase( + description="selecting a mocked model does not select its test on the python engine", + tests=("mocked_model_read_by_expected_and_assertion",), + command=("--no-color", "test", "--select", "items"), + engine="python", + sql_analysis=True, + expected_output_fragments=("0 selected",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(",), + ), + HelperReferenceE2ETestCase( + description="expected rows read an unmocked model the test runs on the python engine", + tests=("expected_reads_unmocked_model",), + command=("--no-color", "test", "--select", "item_extras"), + engine="python", + sql_analysis=True, + expected_output_fragments=( + "1 selected", + "PASS=1 FAIL=0 TOTAL=1", + ), + expected_compiled_fragments=("amount * 2 AS amount_doubled FROM __ref__item_extras",), + expected_absent_compiled_fragments=("__ref(",), + ), + HelperReferenceE2ETestCase( + description="selecting a mocked model does not select its test on the native engine", + tests=("mocked_model_read_by_expected_and_assertion",), + command=("--no-color", "test", "--select", "items"), + engine="native", + sql_analysis=True, + expected_output_fragments=("0 selected",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(",), + ), + HelperReferenceE2ETestCase( + description="expected rows read an unmocked model the test runs on the native engine", + tests=("expected_reads_unmocked_model",), + command=("--no-color", "test", "--select", "item_extras"), + engine="native", + sql_analysis=True, + expected_output_fragments=( + "1 selected", + "PASS=1 FAIL=0 TOTAL=1", + ), + expected_compiled_fragments=("amount * 2 AS amount_doubled FROM __ref__item_extras",), + expected_absent_compiled_fragments=("__ref(",), + ), + HelperReferenceE2ETestCase( + description="selecting a mocked model does not select its test on the native-preview engine", + tests=("mocked_model_read_by_expected_and_assertion",), + command=("--no-color", "test", "--select", "items"), + engine="native-preview", + sql_analysis=True, + expected_output_fragments=("0 selected",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(",), + ), + HelperReferenceE2ETestCase( + description="expected rows read an unmocked model the test runs on the native-preview engine", + tests=("expected_reads_unmocked_model",), + command=("--no-color", "test", "--select", "item_extras"), + engine="native-preview", + sql_analysis=True, + expected_output_fragments=( + "1 selected", + "PASS=1 FAIL=0 TOTAL=1", + ), + expected_compiled_fragments=("amount * 2 AS amount_doubled FROM __ref__item_extras",), + expected_absent_compiled_fragments=("__ref(",), + ), ), ids=lambda case: case.description, ) @@ -304,6 +487,87 @@ def test_given_helper_cte_references_when_testing_then_helpers_read_the_tested_m expected_column=35, expected_help_fragment='FROM __ref__item_totals rather than FROM __ref("item_totals")', ), + HelperReferenceErrorE2ETestCase( + description="assertion reading an unmocked source on the python engine", + test_name="assertion_reads_unmocked_source", + engine="python", + expected_code="P013", + expected_message=( + "SQL test assertion CTE '__assert__no_returns' calls " + '__source("item_returns"), which the test does not mock, so the test query ' + "cannot resolve it" + ), + expected_line=11, + expected_column=23, + expected_help_fragment="__source__item_returns AS (SELECT ...)", + ), + HelperReferenceErrorE2ETestCase( + description="expected rows reading an unknown model on the python engine", + test_name="expected_reads_unknown_model", + engine="python", + expected_code="P013", + expected_message=( + "SQL test expected CTE '__expected__item_totals' references unknown model " + "'item_archive'" + ), + expected_line=8, + expected_column=39, + expected_help_fragment='__ref("")', + ), + HelperReferenceErrorE2ETestCase( + description="assertion reading an unmocked source on the native engine", + test_name="assertion_reads_unmocked_source", + engine="native", + expected_code="P013", + expected_message=( + "SQL test assertion CTE '__assert__no_returns' calls " + '__source("item_returns"), which the test does not mock, so the test query ' + "cannot resolve it" + ), + expected_line=11, + expected_column=23, + expected_help_fragment="__source__item_returns AS (SELECT ...)", + ), + HelperReferenceErrorE2ETestCase( + description="expected rows reading an unknown model on the native engine", + test_name="expected_reads_unknown_model", + engine="native", + expected_code="P013", + expected_message=( + "SQL test expected CTE '__expected__item_totals' references unknown model " + "'item_archive'" + ), + expected_line=8, + expected_column=39, + expected_help_fragment='__ref("")', + ), + HelperReferenceErrorE2ETestCase( + description="assertion reading an unmocked source on the native-preview engine", + test_name="assertion_reads_unmocked_source", + engine="native-preview", + expected_code="P013", + expected_message=( + "SQL test assertion CTE '__assert__no_returns' calls " + '__source("item_returns"), which the test does not mock, so the test query ' + "cannot resolve it" + ), + expected_line=11, + expected_column=23, + expected_help_fragment="__source__item_returns AS (SELECT ...)", + ), + HelperReferenceErrorE2ETestCase( + description="expected rows reading an unknown model on the native-preview engine", + test_name="expected_reads_unknown_model", + engine="native-preview", + expected_code="P013", + expected_message=( + "SQL test expected CTE '__expected__item_totals' references unknown model " + "'item_archive'" + ), + expected_line=8, + expected_column=39, + expected_help_fragment='__ref("")', + ), ), ids=lambda case: case.description, ) diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py index 75a983f290..6338bde708 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py @@ -538,9 +538,7 @@ def test_given_test_and_project_when_planning_then_produces_expected_chain( expected_warning_count=1, expected_warning_severity=WarningSeverity.ERROR, expected_error_fragments=("__dbt_ref__stripe__payments which has no mock",), - expected_cte_bodies={ - "payments": 'SELECT payment_id FROM __dbt_ref("stripe", "payments")', - }, + expected_cte_bodies={"payments": "SELECT 1 AS payment_id"}, ), PlanTestChainTestCase( description=("unresolved ref produces error warning"), diff --git a/website/src/content/docs/docs/concepts/testing.mdx b/website/src/content/docs/docs/concepts/testing.mdx index a416ce8a5c..0b712e2dd1 100644 --- a/website/src/content/docs/docs/concepts/testing.mdx +++ b/website/src/content/docs/docs/concepts/testing.mdx @@ -141,11 +141,18 @@ __assert__no_unexpected_order_ids AS ( SQLBuild emits each helper and mock a query needs once, in dependency order, including mocks that a helper reaches through another mock. -Helpers can also call the references an `__assert__` CTE can, such as `__ref()`, `__source()` and -`__seed()`, and they resolve exactly as there: a mocked relation reads its mock, and an unmocked -`__ref("")` reads the model's real SQL, so the test runs that model and -`sqb test --select ` selects the test even when only a helper reads it. SQLBuild emits a -helper after the mocks and models it reads: +`__expected__` and `__assert__` CTEs, and the helpers they read, can call `__ref()`, +`__source()`, `__seed()` and `__dbt_ref()`. They resolve the same way in every test CTE: + +- A mocked name always means the mock. `__ref("stg_orders")` reads `__ref__stg_orders` when the + test defines it, and a mocked model is neither run nor counted as a model the test covers, so + `sqb test --select stg_orders` does not select the test because of it. +- An unmocked `__ref("")` reads the model's real SQL, so the test runs that model and + `sqb test --select ` selects the test, even when only a helper or expected rows read it. +- `__source()`, `__seed()` and `__dbt_ref()` must be mocked, for example with + `__source__raw__orders AS (SELECT ...)`. + +SQLBuild emits a helper after the mocks and models it reads: ```sql TEST(); @@ -165,9 +172,9 @@ __assert__quantities_are_positive AS ( Only helpers the test reads count: those its `__expected__` and `__assert__` CTEs reach, directly or through other helpers and mocks. A helper nothing reads is left out of the test query, and its references are neither checked nor run. Compile rejects references the test query cannot resolve -with `P013`, at the helper or call that causes it: a `__ref()` to a model that does not exist, -helpers or mocks that read each other in a cycle, and a mock or `__table_fn__` fixture the test -uses that reads a helper calling a reference. Mocks are defined before the models a test runs, so +with `P013`, at the CTE or call that causes it: a `__ref()` to a model that does not exist, an +unmocked `__source()`, `__seed()` or `__dbt_ref()`, helpers or mocks that read each other in a +cycle, and a mock or `__table_fn__` fixture the test uses that reads a helper calling a reference. Mocks are defined before the models a test runs, so a mock reads other mocks by CTE name, such as `FROM __ref__stg_orders`, rather than through `__ref("stg_orders")`. @@ -422,6 +429,10 @@ __assert__order_ids_are_not_null AS ( ) ``` +The assertion reads the `__ref__stg_orders` mock, not the real `stg_orders` model: a mocked name +always means the mock. Earlier SQLBuild versions ran the model's real SQL for an assertion calling +`__ref()` on a mocked model. + Assertions can be mixed with `__expected__` CTEs in the same test, or used on their own. They are useful when the natural check is "no rows should violate this rule" rather than "the output should exactly equal these rows" - for example, duplicate checks, negative-value constraints, or conditional business rules. During `sqb test` and `sqb build`, assertion results appear as nested check rows alongside expected comparisons. From 5e97f21c9d532f7b46e966df9c4534464248b61f Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 8 Oct 2026 07:24:02 +0100 Subject: [PATCH 2/4] fix(test): reject SQL tests whose checks read only mocks A model-mode SQL test whose expected rows and assertions read only mocked names runs no model and used to be skipped silently. Compile now reports it with a located P013 diagnostic showing how to call __ref() on the model under test. Macro, UDF and table-function tests and scenarios keep their own targets. --- .../_helpers/corpus/render_failure_cases.py | 18 +++++++ .../references/docs/concepts/testing.md | 14 +++--- .../compile/_helpers/attachment/sql_tests.py | 14 ++++++ .../compile/_helpers/sql_tests/helper_ctes.py | 49 +++++++++++++++++++ .../cli/commands/main/test/helpers.py | 6 +++ .../main/test/test_helper_references.py | 39 +++++++++++++++ .../_helpers/test_declaration_scopes.py | 12 +++-- .../content/docs/docs/concepts/testing.mdx | 14 +++--- 8 files changed, 151 insertions(+), 15 deletions(-) diff --git a/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py index e29fef2b82..e46f035289 100644 --- a/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py @@ -291,6 +291,24 @@ def _reference_cases() -> tuple[FailureCase, ...]: ) }, ), + failure_case( + name="test-reads-only-mocks", + expected_code="P013", + expected_message=( + "SQL test 'test_stg_orders' reads only mocks (__ref__stg_orders), so it tests " + "no model" + ), + files={ + "tests/unit/test_stg_orders.sql": ( + "TEST();\n\nWITH\n__ref__stg_orders AS (\n" + " SELECT 1 AS order_id, 10 AS customer_id, CAST(5 AS DOUBLE) AS amount," + " 'placed' AS status\n),\n" + "__assert__no_negative_amounts AS (\n" + ' SELECT order_id FROM __ref("stg_orders") WHERE amount < 0\n' + ")\nSELECT 1\n" + ) + }, + ), failure_case( name="test-expected-reads-unknown-model", expected_code="P013", 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 2c136d719d..4cca3f45c2 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md @@ -444,18 +444,20 @@ Unit tests can include `__assert__` CTEs for property-based checks. An ass TEST(); WITH -__ref__stg_orders AS ( - SELECT 1 AS order_id, 100 AS customer_id, 3 AS quantity, - CAST('2026-04-01 10:00:00' AS TIMESTAMP) AS ordered_at, 'completed' AS status +__source__raw__orders AS ( + SELECT 1 AS id, 100 AS customer_id, 2 AS waffle_type_id, 3 AS quantity, + '2026-04-01 10:00:00' AS ordered_at, 'completed' AS status ), __assert__order_ids_are_not_null AS ( SELECT * FROM __ref("stg_orders") WHERE order_id IS NULL ) ``` -The assertion reads the `__ref__stg_orders` mock, not the real `stg_orders` model: a mocked name -always means the mock. Earlier SQLBuild versions ran the model's real SQL for an assertion calling -`__ref()` on a mocked model. +An assertion must read a model the test runs. A mocked name always means the mock, so a test whose +`__expected__` and `__assert__` CTEs read only mocks, such as `__ref("stg_orders")` in a test that +also defines `__ref__stg_orders`, tests no model. Compile rejects it with `P013` instead of +skipping it; call `__ref()` on the model under test, or remove the test. Earlier SQLBuild versions +ran the model's real SQL for an assertion calling `__ref()` on a mocked model. Assertions can be mixed with `__expected__` CTEs in the same test, or used on their own. They are useful when the natural check is "no rows should violate this rule" rather than "the output should exactly equal these rows" - for example, duplicate checks, negative-value constraints, or conditional business rules. diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py b/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py index 356e3a8bb4..c90152fc52 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py @@ -39,6 +39,7 @@ ) from sqlbuild.compiler.compile._helpers.sql_tests.helper_ctes import ( reference_target_model_names, + report_test_without_target_model, report_unresolvable_test_references, ) from sqlbuild.compiler.compile._helpers.sql_tests.native import ( @@ -350,6 +351,19 @@ def build_test_inputs( reference_targets = reference_target_model_names( payload=test_ctes.payload, syntax=sql_lexical_syntax ) + if ( + test.mode is SqlTestMode.MODEL + and not test_ctes.payload.expected_model_names + and not assertion_target_model_names + and not reference_targets + ): + report_test_without_target_model( + payload=test_ctes.payload, + test_file=test.test_file, + test_block=test.test_block, + syntax=sql_lexical_syntax, + ) + continue test_inputs.append( CompileSqlTestInput( test_file=test.test_file, diff --git a/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py b/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py index 0d23325661..3a56b787ff 100644 --- a/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py +++ b/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py @@ -224,6 +224,55 @@ def report_unresolvable_test_references( return reporter.reported +def report_test_without_target_model( + *, + payload: CompileModelSqlTestCtes, + test_file: DiscoveredSqlTestFile, + test_block: DiscoveredSqlTestBlock, + syntax: SqlLexicalSyntax, +) -> None: + """Report a model test whose checks read only mocks, so it would run no model.""" + + mocked: dict[SqlReferenceKind, frozenset[str]] = { + SqlReferenceKind.REF: frozenset(payload.mock_model_names), + SqlReferenceKind.SOURCE: frozenset(payload.mock_source_names), + SqlReferenceKind.SEED: frozenset(payload.mock_seed_names), + SqlReferenceKind.DBT_REF: frozenset(payload.mock_dbt_ref_names), + SqlReferenceKind.TABLE_FUNCTION: frozenset(payload.mock_table_function_names), + } + graph: SqlTestCteGraph = sql_test_cte_graph( + authored_ctes=payload.authored_ctes, reader_ctes=_reader_ctes(payload) + ) + read_mocks: list[str] = [ + graph.ctes[key].name + for key in reachable_cte_keys(graph=graph, roots=graph.reader_reads) + if graph.ctes[key].name.startswith(_MOCK_CTE_PREFIXES) + ] + first_call: str | None = None + for cte in _reader_ctes(payload): + for reference in _references(sql=cte.sql_body, syntax=syntax): + kind: SqlReferenceKind = SqlReferenceKind(reference.ref_kind) + if reference.ref_name not in mocked.get(kind, frozenset()): + continue + read_mocks.append(f"{kind.fixture_cte_prefix}{reference.ref_name}") + if first_call is None and kind is not SqlReferenceKind.TABLE_FUNCTION: + first_call = _reference_call(reference) + mocks: tuple[str, ...] = tuple(dict.fromkeys(read_mocks)) + reads: str = "reads only mocks (" + ", ".join(mocks) + ")" if mocks else "reads no model" + reader: CompileSqlTestCte | None = next(iter(_reader_ctes(payload)), None) + test_name: str = test_block.name or test_file.relative_path.stem + _HelperDiagnostics(test_file=test_file, test_block=test_block).report( + cte_name=reader.name if reader is not None else test_name, + call=first_call, + message=f"SQL test '{test_name}' {reads}, so it tests no model and would never run", + help=( + 'Call __ref("") in an __assert__ or __expected__ CTE, for example ' + '__expected__ AS (SELECT ...) or SELECT ... FROM __ref(""), so the test ' + "runs that model with its mocks, or remove the test." + ), + ) + + def report_mocks_reading_referencing_helpers( *, authored_ctes: tuple[CompileSqlTestCte, ...], 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 4b6ab57058..5a547ff593 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py @@ -1827,6 +1827,12 @@ def prepare_optional_ceremony_project(*, tmp_path: Path, test_files: dict[str, s ' SELECT item_id FROM __source("item_returns")\n' ")\n" ), + "assertion_reads_only_mocks": ( + "__ref__items AS (\n SELECT 7 AS item_id, 5 AS amount\n),\n" + "__assert__only_mocked_items AS (\n" + ' SELECT item_id FROM __ref("items") WHERE item_id <> 7\n' + ")\n" + ), "expected_reads_unknown_model": ( "__ref__items AS (\n SELECT 1 AS item_id, 10 AS amount\n),\n" "__expected__item_totals AS (\n" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py index 9d03494a95..b623b0ae89 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py @@ -501,6 +501,19 @@ def test_given_helper_cte_references_when_testing_then_helpers_read_the_tested_m expected_column=23, expected_help_fragment="__source__item_returns AS (SELECT ...)", ), + HelperReferenceErrorE2ETestCase( + description="test reading only mocks on the python engine", + test_name="assertion_reads_only_mocks", + engine="python", + expected_code="P013", + expected_message=( + "SQL test 'assertion_reads_only_mocks' reads only mocks (__ref__items), so it " + "tests no model and would never run" + ), + expected_line=8, + expected_column=23, + expected_help_fragment='Call __ref("") in an __assert__ or __expected__ CTE', + ), HelperReferenceErrorE2ETestCase( description="expected rows reading an unknown model on the python engine", test_name="expected_reads_unknown_model", @@ -528,6 +541,19 @@ def test_given_helper_cte_references_when_testing_then_helpers_read_the_tested_m expected_column=23, expected_help_fragment="__source__item_returns AS (SELECT ...)", ), + HelperReferenceErrorE2ETestCase( + description="test reading only mocks on the native engine", + test_name="assertion_reads_only_mocks", + engine="native", + expected_code="P013", + expected_message=( + "SQL test 'assertion_reads_only_mocks' reads only mocks (__ref__items), so it " + "tests no model and would never run" + ), + expected_line=8, + expected_column=23, + expected_help_fragment='Call __ref("") in an __assert__ or __expected__ CTE', + ), HelperReferenceErrorE2ETestCase( description="expected rows reading an unknown model on the native engine", test_name="expected_reads_unknown_model", @@ -555,6 +581,19 @@ def test_given_helper_cte_references_when_testing_then_helpers_read_the_tested_m expected_column=23, expected_help_fragment="__source__item_returns AS (SELECT ...)", ), + HelperReferenceErrorE2ETestCase( + description="test reading only mocks on the native-preview engine", + test_name="assertion_reads_only_mocks", + engine="native-preview", + expected_code="P013", + expected_message=( + "SQL test 'assertion_reads_only_mocks' reads only mocks (__ref__items), so it " + "tests no model and would never run" + ), + expected_line=8, + expected_column=23, + expected_help_fragment='Call __ref("") in an __assert__ or __expected__ CTE', + ), HelperReferenceErrorE2ETestCase( description="expected rows reading an unknown model on the native-preview engine", test_name="expected_reads_unknown_model", diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_declaration_scopes.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_declaration_scopes.py index a3781441e2..e2fe64e774 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_declaration_scopes.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_declaration_scopes.py @@ -509,10 +509,16 @@ def test_given_scoped_declaration_when_compiling_sql_surface_then_uses_authored_ ExpectedModelDeclarationGrantTestCase( description="filename resemblance and no expected model grant nothing", files={ - "models/domain/orders.sql": "MODEL (description 'Test model orders.');\nSELECT 1 AS value", + "models/domain/customers.sql": ( + "MODEL (description 'Test model customers.');\nSELECT 1 AS value" + ), + "models/domain/orders.sql": ( + "MODEL (description 'Test model orders.');\n" + 'SELECT value FROM __ref("customers")' + ), "tests/unit/orders_test.sql": ( - "TEST ();\nWITH __ref__orders AS (SELECT 1 AS value), " - "__assert__valid AS (SELECT 1 WHERE FALSE) SELECT 1" + "TEST ();\nWITH __ref__customers AS (SELECT 1 AS value), __assert__valid AS " + '(SELECT value FROM __ref("orders") WHERE FALSE) SELECT 1' ), }, expected_sql_fragments=("__assert__valid",), diff --git a/website/src/content/docs/docs/concepts/testing.mdx b/website/src/content/docs/docs/concepts/testing.mdx index 0b712e2dd1..87ad9c6246 100644 --- a/website/src/content/docs/docs/concepts/testing.mdx +++ b/website/src/content/docs/docs/concepts/testing.mdx @@ -420,18 +420,20 @@ Unit tests can include `__assert__` CTEs for property-based checks. An ass TEST(); WITH -__ref__stg_orders AS ( - SELECT 1 AS order_id, 100 AS customer_id, 3 AS quantity, - CAST('2026-04-01 10:00:00' AS TIMESTAMP) AS ordered_at, 'completed' AS status +__source__raw__orders AS ( + SELECT 1 AS id, 100 AS customer_id, 2 AS waffle_type_id, 3 AS quantity, + '2026-04-01 10:00:00' AS ordered_at, 'completed' AS status ), __assert__order_ids_are_not_null AS ( SELECT * FROM __ref("stg_orders") WHERE order_id IS NULL ) ``` -The assertion reads the `__ref__stg_orders` mock, not the real `stg_orders` model: a mocked name -always means the mock. Earlier SQLBuild versions ran the model's real SQL for an assertion calling -`__ref()` on a mocked model. +An assertion must read a model the test runs. A mocked name always means the mock, so a test whose +`__expected__` and `__assert__` CTEs read only mocks, such as `__ref("stg_orders")` in a test that +also defines `__ref__stg_orders`, tests no model. Compile rejects it with `P013` instead of +skipping it; call `__ref()` on the model under test, or remove the test. Earlier SQLBuild versions +ran the model's real SQL for an assertion calling `__ref()` on a mocked model. Assertions can be mixed with `__expected__` CTEs in the same test, or used on their own. They are useful when the natural check is "no rows should violate this rule" rather than "the output should exactly equal these rows" - for example, duplicate checks, negative-value constraints, or conditional business rules. From 82c25be37ab4ee3a9231f1442e3bfaa796cd0a84 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 8 Oct 2026 08:21:53 +0100 Subject: [PATCH 3/4] fix(test): plan SQL-test helpers from the compiler's reads and clarify mock-only errors The native planner now takes the helpers a test reads and the models its expected rows and helpers reference from the compiler, fails when a request omits them, and leaves unread helpers that call references out of the test query, so a helper named like a column no longer runs or fails a test. A reference the planner still cannot resolve is a located P013 instead of an unlocated P001. The error for a test that mocks the model it tests now explains how to mock the model's inputs instead. --- .../_helpers/sql_tests/helper_scope.rs | 109 +++----------- .../compiler/_helpers/sql_tests/planning.rs | 141 ++++++++++++------ .../_helpers/sql_tests/reader_references.rs | 133 ++++++++++++----- .../src/compiler/tests/helpers.rs | 91 +++++++---- .../tests/test_sql_test_helper_references.rs | 129 +++++++++++++--- .../src/compiler/tests/test_types.rs | 5 + .../_helpers/corpus/render_failure_cases.py | 4 +- .../references/docs/concepts/testing.md | 7 +- .../cli/commands/_helpers/compile/pipeline.py | 4 +- .../_helpers/refactor/compile_facts.py | 4 +- .../compile/_helpers/assembly/project.py | 10 ++ .../compile/_helpers/attachment/sql_tests.py | 18 +-- .../compile/_helpers/sql_tests/helper_ctes.py | 104 +++++++++---- .../compile/classes/sql_test_cte_locator.py | 25 ++++ src/sqlbuild/compiler/compile/exceptions.py | 14 ++ src/sqlbuild/compiler/compile/models.py | 11 ++ .../_helpers/sql_tests/native_planning.py | 50 ++++++- .../cli/commands/main/test/helpers.py | 10 ++ .../main/test/test_helper_references.py | 117 +++++++++++++-- .../test_native_attachment_project.py | 6 +- .../sql_test_assembly/test_plan_test.py | 46 +++++- .../content/docs/docs/concepts/testing.mdx | 7 +- 22 files changed, 761 insertions(+), 284 deletions(-) create mode 100644 src/sqlbuild/compiler/compile/classes/sql_test_cte_locator.py diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs index 6e22e9c62e..613dfe929c 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs @@ -15,6 +15,7 @@ use crate::compiler::_helpers::sql_tests::planning::{ DBT_REF_PREFIX, REF_PREFIX, SEED_PREFIX, SOURCE_PREFIX, SqlTestPatterns, TABLE_FUNCTION_PREFIX, TestFixtures, compile_error, }; +use crate::compiler::_helpers::sql_tests::reader_references::calls_reference; use crate::constants::{ QUOTED_IDENTIFIER_DELIMITER_BYTES, SQL_TEST_ACTUAL_CTE, SQL_TEST_ACTUAL_CTE_PREFIX, SQL_TEST_EXPECTED_CTE, @@ -80,6 +81,8 @@ pub(crate) struct ScopeRequest<'a> { pub(crate) rename_dialect: Option<&'a Dialect>, pub(crate) flat_dialect: bool, pub(crate) slice_dialect: SliceDialect, + /// Helpers the compiler found the test reads; an unread helper calling a reference is left out. + pub(crate) read_helper_names: &'a [String], } #[derive(Clone, Copy, PartialEq, Eq, Hash)] @@ -160,13 +163,17 @@ impl ScopeGraph { .mocks_by_generated_name .insert(mock.generated_name.clone(), index); } - if fixtures.helpers.is_empty() { - return Ok(graph); - } - for (index, cte) in fixtures.helpers.iter().enumerate() { - graph - .helper_indexes - .insert(cte.name.to_ascii_lowercase(), index); + let read: HashSet = request + .read_helper_names + .iter() + .map(|name| name.to_ascii_lowercase()) + .collect(); + for cte in &fixtures.helpers { + let key = cte.name.to_ascii_lowercase(); + if !read.contains(&key) && calls_reference(&cte.sql_body, patterns) { + continue; + } + graph.helper_indexes.insert(key, graph.helpers.len()); graph.helpers.push(ScopeHelper { name: cte.name.clone(), original: cte.sql_body.clone(), @@ -176,6 +183,9 @@ impl ScopeGraph { resolved: None, }); } + if graph.helpers.is_empty() { + return Ok(graph); + } let readers = test_readers(fixtures); graph.reject_shadowed_helpers(&readers, request)?; if let Some(dialect) = request.rename_dialect @@ -294,36 +304,12 @@ impl ScopeGraph { order.ordered } - /// Helpers the test's expected rows and assertions read, except those a mock it uses reads. - pub(crate) fn read_helpers( - &self, - readers: &[&str], - used_mocks: &HashSet, - patterns: &SqlTestPatterns, - ) -> HashSet { - let helpers = |nodes: Vec| -> HashSet { - nodes - .into_iter() - .filter_map(|node| match node { - ScopeNode::Helper(index) => Some(index), - ScopeNode::Mock(_) => None, - }) - .collect() - }; - let mut read: HashSet = HashSet::new(); - for sql in readers { - read.extend(helpers( - self.closure(&identifier_tokens(sql, patterns), true), - )); - } - for mock in &self.mocks { - if used_mocks.contains(&mock.mock_name) { - for index in helpers(self.closure(&mock.tokens, true)) { - read.remove(&index); - } - } - } - read + /// Indexes of the helpers the compiler found the test's expected rows and assertions read. + pub(crate) fn read_helper_indexes(&self, names: &[String]) -> HashSet { + names + .iter() + .filter_map(|name| self.helper_indexes.get(&name.to_ascii_lowercase()).copied()) + .collect() } /// Read helpers whose SQL, as placed in the test query, calls a reference such as `__ref()`. @@ -525,55 +511,6 @@ fn test_readers(fixtures: &TestFixtures) -> Vec<(String, &str)> { readers } -/// Authored SQL of the helpers `readers` reach, directly or through other helpers and mocks. -pub(crate) fn read_helper_sql<'f>( - fixtures: &'f TestFixtures, - readers: &[&str], - patterns: &SqlTestPatterns, -) -> Vec<&'f str> { - if fixtures.helpers.is_empty() { - return Vec::new(); - } - let mut bodies: HashMap = fixtures - .helpers - .iter() - .map(|cte| (cte.name.to_ascii_lowercase(), (cte.sql_body.as_str(), true))) - .collect(); - for (prefix, group) in [ - (REF_PREFIX, &fixtures.mock_refs), - (SOURCE_PREFIX, &fixtures.mock_sources), - (SEED_PREFIX, &fixtures.mock_seeds), - (DBT_REF_PREFIX, &fixtures.mock_dbt_refs), - (TABLE_FUNCTION_PREFIX, &fixtures.mock_table_functions), - ] { - for (name, sql) in group { - bodies.insert( - format!("{prefix}{name}").to_ascii_lowercase(), - (sql.as_str(), false), - ); - } - } - let mut pending: Vec = readers - .iter() - .flat_map(|sql| identifier_tokens(sql, patterns)) - .collect(); - let mut visited: HashSet = HashSet::new(); - let mut helpers: Vec<&'f str> = Vec::new(); - while let Some(token) = pending.pop() { - let Some((sql, is_helper)) = bodies.get(&token).copied() else { - continue; - }; - if !visited.insert(token) { - continue; - } - if is_helper { - helpers.push(sql); - } - pending.extend(identifier_tokens(sql, patterns)); - } - helpers -} - /// Return the helper and mock CTEs `sql` reads, transitively and dependencies first. pub(crate) fn helper_scope_ctes( sql: &str, diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs index 976a955912..3bf4a939ff 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs @@ -24,7 +24,8 @@ use crate::compiler::_helpers::sql_tests::markers::{ replace_dbt_ref_markers, replace_named_markers, }; use crate::compiler::_helpers::sql_tests::reader_references::{ - calls_reference, mock_cte_example, reader_ref_targets, unresolved_relation_call, + CompilerReads, assertion_ref_targets, calls_reference, unresolvable_call, + unresolved_reference_error, }; use crate::compiler::_helpers::sql_tests::relation_markers::relation_marker_calls; use crate::compiler::_helpers::sql_tests::rendering::{ @@ -157,6 +158,10 @@ enum TestPayload { expected_model_names: Vec, #[serde(default)] assertion_ctes: Vec, + #[serde(default)] + read_helper_names: Option>, + #[serde(default)] + reference_target_model_names: Option>, }, Direct { mode: String, @@ -303,6 +308,7 @@ struct ModelTestPlan { expected_ctes: Vec, expected_model_names: Vec, assertion_ctes: Vec, + reads: CompilerReads, } struct TextualChainRequest<'a> { @@ -576,36 +582,46 @@ pub(crate) fn resolve_chains_json(request_json: &str) -> Result .tests .into_iter() .map(|test| match test.payload { - TestPayload::Direct { .. } => Vec::new(), + TestPayload::Direct { .. } => Ok(Vec::new()), TestPayload::Model { authored_ctes, model_query_overrides, expected_ctes, expected_model_names, assertion_ctes, + read_helper_names, + reference_target_model_names, } => { + let reads = CompilerReads::required( + &test.file_label, + read_helper_names, + reference_target_model_names, + )?; let fixtures = classify_fixtures(authored_ctes, expected_ctes, assertion_ctes); - let expected_names = chain_root_names(expected_model_names, &fixtures, &patterns); - topo_sort_model_chain(TopoSortRequest { + let expected_names = + chain_root_names(expected_model_names, &reads, &fixtures, &patterns); + Ok(topo_sort_model_chain(TopoSortRequest { expected_names: &expected_names, models: &models, overrides: &model_query_overrides, mock_refs: &fixtures.mock_refs, patterns: &patterns, - }) + })) } }) - .collect(); + .collect::>()?; serde_json::to_string(&ChainBatchResponse { chains }).map_err(|error| error.to_string()) } -/// Models the test runs: expected models and the unmocked models its test CTEs call `__ref()` on. +/// Models the test runs: expected models, assertion targets and the compiler's reference targets. fn chain_root_names( mut expected_names: Vec, + reads: &CompilerReads, fixtures: &TestFixtures, patterns: &SqlTestPatterns, ) -> Vec { - expected_names.extend(reader_ref_targets(fixtures, patterns)); + expected_names.extend(assertion_ref_targets(fixtures, patterns)); + expected_names.extend(reads.reference_target_model_names.iter().cloned()); dedupe(expected_names) } @@ -739,18 +755,28 @@ fn plan_test(test: TestInput, context: &ProjectContext) -> Result plan_model_test( - ModelTestPlan { - test_name: test.name, - file_label: test.file_label, - authored_ctes, - model_query_overrides, - expected_ctes, - expected_model_names, - assertion_ctes, - }, - context, - ), + read_helper_names, + reference_target_model_names, + } => { + let reads = CompilerReads::required( + &test.file_label, + read_helper_names, + reference_target_model_names, + )?; + plan_model_test( + ModelTestPlan { + test_name: test.name, + file_label: test.file_label, + authored_ctes, + model_query_overrides, + expected_ctes, + expected_model_names, + assertion_ctes, + reads, + }, + context, + ) + } } } @@ -826,8 +852,14 @@ fn plan_model_test( .then_some(context.render_dialect.as_ref()), flat_dialect: context.rejects_nested_with, slice_dialect: SliceDialect::new(Some(&context.dialect)), + read_helper_names: &plan.reads.read_helper_names, })?; - let expected_names = chain_root_names(plan.expected_model_names, &fixtures, &context.patterns); + let expected_names = chain_root_names( + plan.expected_model_names, + &plan.reads, + &fixtures, + &context.patterns, + ); let ordered_names = topo_sort_model_chain(TopoSortRequest { expected_names: &expected_names, models: &context.models, @@ -938,6 +970,7 @@ fn plan_model_test( context, ordered_names: &ordered_names, overrides: &plan.model_query_overrides, + test_name: &plan.test_name, file_label: &plan.file_label, analysis_resolved: &analysis_resolved, textual_chain: None, @@ -945,6 +978,7 @@ fn plan_model_test( resolve_referencing_helpers(HelperResolution { readers: &mut readers, fixtures: &mut fixtures, + read_helper_names: &plan.reads.read_helper_names, chain: &mut chain, reachable_mocks: &mut reachable_mocks, })?; @@ -955,7 +989,10 @@ fn plan_model_test( let resolved = readers.resolve( &placed_sql, &fixtures, - &format!("{ASSERT_PREFIX}{assertion_name}"), + &AuthoredReader { + cte_name: &format!("{ASSERT_PREFIX}{assertion_name}"), + sql: assertion_sql, + }, )?; reachable_mocks.extend(resolved.chain_reached); reachable_mocks.extend(resolved.reached); @@ -1009,6 +1046,7 @@ fn plan_model_test( struct HelperResolution<'r, 'a> { readers: &'r mut ReaderContext<'a>, fixtures: &'r mut TestFixtures, + read_helper_names: &'r [String], chain: &'r mut [ChainStep], reachable_mocks: &'r mut HashSet, } @@ -1018,23 +1056,20 @@ fn resolve_referencing_helpers(request: HelperResolution<'_, '_>) -> Result<(), let HelperResolution { readers, fixtures, + read_helper_names, chain, reachable_mocks, } = request; let patterns = &readers.context.patterns; - let read_sql: Vec<&str> = fixtures - .expected - .values() - .chain(fixtures.assertions.iter().map(|(_, sql)| sql)) - .map(String::as_str) - .collect(); - let read_helpers = fixtures - .scope - .read_helpers(&read_sql, reachable_mocks, patterns); + let read_helpers = fixtures.scope.read_helper_indexes(read_helper_names); let mut resolved_helpers: Vec<(usize, ResolvedHelper)> = Vec::new(); for (index, helper_sql) in fixtures.scope.referencing_helpers(&read_helpers, patterns) { let label = fixtures.scope.helper_name(index).to_string(); - let resolved = readers.resolve(&helper_sql, fixtures, &label)?; + let authored = AuthoredReader { + cte_name: &label, + sql: &helper_sql, + }; + let resolved = readers.resolve(&helper_sql, fixtures, &authored)?; reachable_mocks.extend(resolved.chain_reached); resolved_helpers.push(( index, @@ -1054,7 +1089,7 @@ fn resolve_referencing_helpers(request: HelperResolution<'_, '_>) -> Result<(), continue; }; let placed_sql = fixtures.scope.reader_sql(sql, patterns); - let references = calls_reference(&placed_sql, patterns); + let references = calls_reference(sql, patterns); if !helpers_resolved && !references { continue; } @@ -1062,7 +1097,11 @@ fn resolve_referencing_helpers(request: HelperResolution<'_, '_>) -> Result<(), reachable_mocks.extend(scope.reached_mocks.iter().cloned()); if references { let label = format!("{EXPECTED_PREFIX}{}", step.model_name); - let resolved = readers.resolve(&placed_sql, fixtures, &label)?; + let authored = AuthoredReader { + cte_name: &label, + sql, + }; + let resolved = readers.resolve(&placed_sql, fixtures, &authored)?; reachable_mocks.extend(resolved.chain_reached); reachable_mocks.extend(resolved.reached); step.expected_lifted_ctes = @@ -1081,11 +1120,18 @@ struct ReaderContext<'a> { context: &'a ProjectContext, ordered_names: &'a [String], overrides: &'a BTreeMap, + test_name: &'a str, file_label: &'a str, analysis_resolved: &'a HashMap, textual_chain: Option, } +/// One test CTE as authored, whose own reference calls must all resolve. +struct AuthoredReader<'s> { + cte_name: &'s str, + sql: &'s str, +} + /// One assertion or helper query with its references resolved to the CTEs standing in for them. struct ResolvedReader { resolved_sql: String, @@ -1097,24 +1143,27 @@ struct ResolvedReader { } impl ReaderContext<'_> { - /// Resolve a test-authored query, failing rather than leaving a relation reference raw. + /// Resolve a test-authored query, failing if its own CTE calls a reference nothing resolves. fn resolve( &mut self, sql: &str, fixtures: &TestFixtures, - label: &str, + authored: &AuthoredReader<'_>, ) -> Result { - let resolved = self.resolve_references(sql, fixtures)?; - if let Some(call) = unresolved_relation_call(&resolved.body_sql, &self.context.patterns) { - return Err(compile_error(&format!( - "SQL test '{}' CTE '{label}' calls {call}, which the test query cannot resolve; \ - mock the relation with a CTE such as {} AS (SELECT ...), or read a model the \ - test runs with __ref(\"\")", + if let Some(call) = unresolvable_call( + authored.sql, + fixtures, + self.ordered_names, + &self.context.patterns, + ) { + return Err(unresolved_reference_error( + self.test_name, self.file_label, - mock_cte_example(&call) - ))); + authored.cte_name, + &call, + )); } - Ok(resolved) + self.resolve_references(sql, fixtures) } fn resolve_references( @@ -1942,6 +1991,6 @@ pub(crate) fn compile_error(message: &str) -> String { format!("compile_input:{message}") } -fn planner_error(message: &str) -> String { +pub(crate) fn planner_error(message: &str) -> String { format!("planner_input:{message}") } diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs index 3b13a4e496..103845aa15 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs @@ -1,37 +1,94 @@ -//! Relation references test-authored SQL-test CTEs call, and which models they make a test run. +//! Relation references test-authored SQL-test CTEs call, and which of them a test can resolve. + +use serde_json::json; -use crate::compiler::_helpers::sql_tests::helper_scope::read_helper_sql; use crate::compiler::_helpers::sql_tests::markers::{ProtectedRanges, marker_names}; -use crate::compiler::_helpers::sql_tests::planning::{SqlTestPatterns, TestFixtures}; +use crate::compiler::_helpers::sql_tests::planning::{ + SqlTestPatterns, TestFixtures, planner_error, +}; + +/// Prefix of the planner error for a reference a test CTE calls that the test cannot resolve. +pub(crate) const UNRESOLVED_REFERENCE_ERROR_PREFIX: &str = "sql_test_reference:"; /// Whether test-authored SQL calls a reference or function SQLBuild resolves for the test query. pub(crate) fn calls_reference(sql: &str, patterns: &SqlTestPatterns) -> bool { patterns.test_reference.is_match(sql) || patterns.udf.is_match(sql) } -/// The first relation reference call left outside comments and quoted text, as written. -pub(crate) fn unresolved_relation_call(sql: &str, patterns: &SqlTestPatterns) -> Option { +/// Unmocked models the test's assertions call `__ref()` on. +pub(crate) fn assertion_ref_targets( + fixtures: &TestFixtures, + patterns: &SqlTestPatterns, +) -> Vec { + let mut targets: Vec = Vec::new(); + for (_, sql) in &fixtures.assertions { + targets.extend( + marker_names(&patterns.reference, &patterns.lexical, sql) + .into_iter() + .filter(|name| !fixtures.mock_refs.contains_key(name)), + ); + } + targets +} + +/// The first relation reference call, outside comments and quoted text, that neither a mock nor a model the test runs resolves. +pub(crate) fn unresolvable_call( + sql: &str, + fixtures: &TestFixtures, + chain: &[String], + patterns: &SqlTestPatterns, +) -> Option { if !patterns.test_reference.is_match(sql) { return None; } let mut protected = ProtectedRanges::new(&patterns.lexical, sql); - for pattern in [ - &patterns.reference, - &patterns.source, - &patterns.seed, - &patterns.dbt_reference, - ] { - for found in pattern.find_iter(sql) { - if !protected.contains(found.start()) { - return Some(found.as_str().to_string()); + let groups = [ + (&patterns.reference, &fixtures.mock_refs, true), + (&patterns.source, &fixtures.mock_sources, false), + (&patterns.seed, &fixtures.mock_seeds, false), + (&patterns.dbt_reference, &fixtures.mock_dbt_refs, false), + ]; + for (pattern, mocks, runs_models) in groups { + for captures in pattern.captures_iter(sql) { + let Some(full) = captures.get(0) else { + continue; + }; + if protected.contains(full.start()) { + continue; + } + let name = match (captures.get(1), captures.get(2)) { + (Some(first), Some(second)) => format!("{}__{}", first.as_str(), second.as_str()), + (Some(first), None) => first.as_str().to_string(), + _ => continue, + }; + let runs = runs_models && chain.iter().any(|model| *model == name); + if !runs && !mocks.contains_key(&name) { + return Some(full.as_str().to_string()); } } } None } +/// The planner error for a reference call one test CTE makes that the test cannot resolve. +pub(crate) fn unresolved_reference_error( + test_name: &str, + file_label: &str, + cte_name: &str, + call: &str, +) -> String { + let payload = json!({ + "testName": test_name, + "fileLabel": file_label, + "cteName": cte_name, + "call": call, + "mockCte": mock_cte_example(call), + }); + format!("{UNRESOLVED_REFERENCE_ERROR_PREFIX}{payload}") +} + /// The mock CTE that would stand in for one reference call, such as `__source__orders`. -pub(crate) fn mock_cte_example(call: &str) -> String { +fn mock_cte_example(call: &str) -> String { let names: Vec<&str> = call.split('"').skip(1).step_by(2).collect(); let function = call .split('(') @@ -41,28 +98,28 @@ pub(crate) fn mock_cte_example(call: &str) -> String { format!("{function}__{}", names.join("__")) } -/// Unmocked models the test's assertions, expected rows and the helpers they read call `__ref()` on. -pub(crate) fn reader_ref_targets( - fixtures: &TestFixtures, - patterns: &SqlTestPatterns, -) -> Vec { - let readers: Vec<&str> = fixtures - .assertions - .iter() - .map(|(_, sql)| sql.as_str()) - .chain(fixtures.expected.values().map(String::as_str)) - .collect(); - let mut targets: Vec = Vec::new(); - for sql in readers - .iter() - .copied() - .chain(read_helper_sql(fixtures, &readers, patterns)) - { - targets.extend( - marker_names(&patterns.reference, &patterns.lexical, sql) - .into_iter() - .filter(|name| !fixtures.mock_refs.contains_key(name)), - ); +/// Which helpers a test reads and which models its test CTEs make it run, as the compiler found. +pub(crate) struct CompilerReads { + pub(crate) read_helper_names: Vec, + pub(crate) reference_target_model_names: Vec, +} + +impl CompilerReads { + /// Require the compiler's reads; planning without them could leave references raw. + pub(crate) fn required( + file_label: &str, + read_helper_names: Option>, + reference_target_model_names: Option>, + ) -> Result { + match (read_helper_names, reference_target_model_names) { + (Some(read_helper_names), Some(reference_target_model_names)) => Ok(Self { + read_helper_names, + reference_target_model_names, + }), + _ => Err(planner_error(&format!( + "SQL test '{file_label}' planning request omits readHelperNames or \ + referenceTargetModelNames, which the compiler computes for every model test" + ))), + } } - targets } diff --git a/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs b/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs index ae2da50e57..3c1d87c438 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs @@ -201,7 +201,7 @@ pub(crate) fn concurrent_requests_initialize_shared_template_once() -> bool { pub(crate) fn model_test_batch_returns_ordered_artifact() -> bool { let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( + &plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": [ @@ -264,7 +264,7 @@ pub(crate) fn model_test_batch_returns_ordered_artifact() -> bool { pub(crate) fn unicode_cte_after_leading_with_preserves_identifier() -> bool { let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( + &plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": [{ @@ -309,7 +309,7 @@ pub(crate) fn unicode_cte_after_leading_with_preserves_identifier() -> bool { pub(crate) fn unresolved_reference_fast_rejection_preserves_warning() -> bool { let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( + &plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": [{ @@ -399,7 +399,7 @@ pub(crate) fn shared_textual_chain_renders_each_model_once() -> bool { } let top: String = format!("orders_{LAYERS:02}_left"); let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( + &plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": models, @@ -482,7 +482,7 @@ fn plan_deep_diamond_with_missing_mock(sql_analysis_enabled: bool) -> Value { const LAYERS: usize = 12; let top: String = format!("orders_{LAYERS:02}_left"); serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( + &plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": deep_diamond_models(LAYERS), @@ -531,7 +531,7 @@ pub(crate) fn deep_shared_graph_reports_missing_mock_once() -> bool { pub(crate) fn plan_without_rendering_returns_executable_steps() -> bool { let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( + &plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": [ @@ -614,7 +614,7 @@ pub(crate) fn plan_without_rendering_returns_executable_steps() -> bool { pub(crate) fn textual_assertion_with_clause_merges_lifted_ctes() -> bool { let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( + &plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": [ @@ -698,11 +698,9 @@ pub(crate) fn upstream_fallback_resolves() -> bool { }], "sqlAnalysisEnabled": true, "sqlAnalysisDialect": "duckdb" }); - let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json(&request.to_string()) - .expect("planning succeeds"), - ) - .expect("valid JSON"); + let response: Value = + serde_json::from_str(&plan_json(&request.to_string()).expect("planning succeeds")) + .expect("valid JSON"); let artifact = &response["artifacts"][0]; assert_eq!(artifact["warnings"], json!([])); let sql = artifact["sql"].as_str().expect("rendered SQL"); @@ -712,10 +710,7 @@ pub(crate) fn upstream_fallback_resolves() -> bool { let mut artifact_request = request; artifact_request["includePlan"] = json!(false); let compact: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( - &artifact_request.to_string(), - ) - .expect("artifact planning succeeds"), + &plan_json(&artifact_request.to_string()).expect("artifact planning succeeds"), ) .expect("valid artifact JSON"); assert_eq!(compact["artifacts"][0]["sql"], artifact["sql"]); @@ -731,7 +726,7 @@ pub(crate) fn upstream_fallback_resolves() -> bool { pub(crate) fn chain_resolution_orders_unmocked_models() -> bool { let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_chain_resolution::resolve_chains_json( + &chain_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": deep_diamond_models(2), @@ -856,7 +851,7 @@ pub(crate) fn long_chain_plan_output_stays_linear() -> bool { let tip: String = format!("orders_{:03}", MODELS - 1); for sql_analysis_enabled in [true, false] { let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( + &plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": models, @@ -1016,7 +1011,7 @@ pub(crate) fn sqlserver_difference_sample_projects_bracketed_columns() -> bool { pub(crate) fn snowflake_plan_keeps_quoted_expected_columns() -> bool { let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( + &plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": [{ @@ -1064,7 +1059,7 @@ fn plan_helper_scope_request( sql_analysis_enabled: bool, helper_name: &str, ) -> Result { - crate::compiler::main::sql_test_planning::plan_and_render_json( + plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": [ @@ -1288,7 +1283,7 @@ pub(crate) fn mock_read_through_helper_brings_its_mock_dependencies_into_scope() ]; for shape in shapes { let response: Value = serde_json::from_str( - &crate::compiler::main::sql_test_planning::plan_and_render_json( + &plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": [ @@ -1760,7 +1755,7 @@ pub(crate) fn plan_shape_refusal(shape: &PlanShape<'_>) -> String { } fn plan_shape(shape: &PlanShape<'_>) -> Result { - let response = crate::compiler::main::sql_test_planning::plan_and_render_json( + let response = plan_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": [{"name": "orders", "querySql": shape.model_sql, "modelDependencies": []}], @@ -1804,7 +1799,7 @@ fn assert_names_isolated(sql: &str, dialect: &str, helper_name: &str, label: &st } } -fn helper_reference_test(case: &HelperReferenceTestCase) -> Value { +fn helper_reference_test(case: &HelperReferenceTestCase, sends_compiler_reads: bool) -> Value { let named = |ctes: &[(&str, &str)]| -> Vec { ctes.iter() .map(|(name, sql)| json!({"name": name, "sqlBody": sql})) @@ -1821,7 +1816,7 @@ fn helper_reference_test(case: &HelperReferenceTestCase) -> Value { ), ]); authored.extend(named(case.helpers)); - json!({ + let mut test = json!({ "name": "orders_case", "fileLabel": "tests/orders.sql", "payload": { @@ -1835,7 +1830,15 @@ fn helper_reference_test(case: &HelperReferenceTestCase) -> Value { .collect::>(), "assertionCtes": named(case.assertions) } - }) + }); + let reads = [ + ("readHelperNames", json!(case.read_helpers)), + ("referenceTargetModelNames", json!(case.reference_targets)), + ]; + for (key, value) in reads.into_iter().filter(|_| sends_compiler_reads) { + test["payload"][key] = value; + } + test } fn helper_reference_models() -> Value { @@ -1856,12 +1859,13 @@ fn helper_reference_models() -> Value { /// Plan and render one helper-reference case through the native planner's JSON entry point. pub(crate) fn plan_helper_reference_response( case: &HelperReferenceTestCase, + sends_compiler_reads: bool, ) -> Result { crate::compiler::main::sql_test_planning::plan_and_render_json( &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": helper_reference_models(), - "tests": [helper_reference_test(case)], + "tests": [helper_reference_test(case, sends_compiler_reads)], "sqlAnalysisEnabled": case.sql_analysis_enabled, "sqlAnalysisDialect": "duckdb", "setDifferenceOperator": "EXCEPT" @@ -1873,7 +1877,7 @@ pub(crate) fn plan_helper_reference_response( /// Plan and render one helper-reference case, returning its SQL, chain and warnings. pub(crate) fn plan_helper_reference_case(case: &HelperReferenceTestCase) -> Value { let response: Value = serde_json::from_str( - &plan_helper_reference_response(case).expect("test assumption must hold"), + &plan_helper_reference_response(case, true).expect("test assumption must hold"), ) .expect("test assumption must hold"); response["artifacts"][0].clone() @@ -1886,7 +1890,7 @@ pub(crate) fn chain_helper_reference_case(case: &HelperReferenceTestCase) -> Val &json!({ "lexicalSyntax": generic_lexical_syntax(), "models": helper_reference_models(), - "tests": [helper_reference_test(case)] + "tests": [helper_reference_test(case, true)] }) .to_string(), ) @@ -1900,3 +1904,34 @@ pub(crate) fn chain_helper_reference_case(case: &HelperReferenceTestCase) -> Val pub(crate) fn defined_before(sql: &str, first: &str, second: &str) -> bool { top_level_cte_position(sql, first) < top_level_cte_position(sql, second) } + +/// Plan a request whose model tests predate the compiler's reads, sending empty reads for them. +pub(crate) fn plan_json(request_json: &str) -> Result { + crate::compiler::main::sql_test_planning::plan_and_render_json(&with_empty_compiler_reads( + request_json, + )) +} + +/// Resolve chains for a request whose model tests predate the compiler's reads. +pub(crate) fn chain_json(request_json: &str) -> Result { + crate::compiler::main::sql_test_chain_resolution::resolve_chains_json( + &with_empty_compiler_reads(request_json), + ) +} + +fn with_empty_compiler_reads(request_json: &str) -> String { + let mut request: Value = serde_json::from_str(request_json).expect("test assumption must hold"); + let model_payloads = request["tests"] + .as_array_mut() + .into_iter() + .flatten() + .map(|test| &mut test["payload"]) + .filter(|payload| payload["kind"] == json!("model")) + .filter_map(Value::as_object_mut); + for payload in model_payloads { + for key in ["readHelperNames", "referenceTargetModelNames"] { + payload.entry(key).or_insert_with(|| json!([])); + } + } + request.to_string() +} diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_helper_references.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_helper_references.rs index 3979ae8fb1..a3b311b682 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_helper_references.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_helper_references.rs @@ -39,6 +39,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen helpers: &[HELPER_READS_MODEL], expected: &[], assertions: &[ASSERT_DOUBLED], + read_helpers: &["doubled"], + reference_targets: &["orders"], expected_chain: &["stg_orders", "orders"], expected_order: &[ ("__source__raw_orders", "__ref__stg_orders"), @@ -57,6 +59,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen helpers: &[HELPER_READS_MODEL], expected: &[], assertions: &[ASSERT_DOUBLED], + read_helpers: &["doubled"], + reference_targets: &["orders"], expected_chain: &["stg_orders", "orders"], expected_order: &[ ("__source__raw_orders", "__ref__orders"), @@ -87,6 +91,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "__assert__has_region", "SELECT order_id FROM with_region WHERE region_name IS NULL", )], + read_helpers: &["with_region", "base"], + reference_targets: &["stg_orders", "orders"], expected_chain: &["stg_orders", "orders"], expected_order: &[ ("__ref__orders", "__helper__base"), @@ -114,6 +120,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "SELECT order_id, amount_doubled FROM expected_rows", )], assertions: &[], + read_helpers: &["expected_rows"], + reference_targets: &[], expected_chain: &["stg_orders", "orders"], expected_order: &[ ("__source__raw_orders", "__helper__expected_rows"), @@ -134,6 +142,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "SELECT order_id, amount_doubled FROM doubled", )], assertions: &[], + read_helpers: &["doubled"], + reference_targets: &["orders"], expected_chain: &["stg_orders", "orders"], expected_order: &[("__ref__orders", "__expected__orders")], expected_fragments: &[ @@ -161,6 +171,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "SELECT order_id FROM doubled JOIN __ref(\"stg_orders\") USING (order_id) \ WHERE amount_doubled <> 20", )], + read_helpers: &["doubled"], + reference_targets: &["orders"], expected_chain: &["orders"], expected_order: &[ ("__helper__base_ids", "__ref__orders"), @@ -179,6 +191,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "SELECT 1 AS order_id, 10 AS amount, 7 AS region_id", )], assertions: &[], + read_helpers: &[], + reference_targets: &[], expected_chain: &["stg_orders"], expected_order: &[("__source__raw_orders", "__expected__stg_orders")], expected_fragments: &[], @@ -193,6 +207,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "SELECT 5 AS order_id, 20 AS amount_doubled", )], assertions: &[ASSERT_MOCK_ROWS], + read_helpers: &[], + reference_targets: &[], expected_chain: &["orders"], expected_order: &[("__ref__stg_orders", "__assert__mock_rows")], expected_fragments: &["FROM __ref__stg_orders WHERE order_id <> 5"], @@ -210,6 +226,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen "SELECT order_id FROM __ref(\"orders\") WHERE amount_doubled <> 20", ), ], + read_helpers: &[], + reference_targets: &[], expected_chain: &["orders"], expected_order: &[ ("__ref__stg_orders", "__ref__orders"), @@ -224,6 +242,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen helpers: &[MOCK_STG_ORDERS], expected: &[EXPECTED_FROM_MOCK], assertions: &[], + read_helpers: &[], + reference_targets: &[], expected_chain: &["orders"], expected_order: &[("__ref__stg_orders", "__expected__orders")], expected_fragments: &[ @@ -237,6 +257,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen helpers: &[MOCK_STG_ORDERS], expected: &[EXPECTED_FROM_MOCK], assertions: &[], + read_helpers: &[], + reference_targets: &[], expected_chain: &["orders"], expected_order: &[("__ref__stg_orders", "__expected__orders")], expected_fragments: &[ @@ -250,6 +272,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen helpers: &[], expected: &[EXPECTED_FROM_MOCK], assertions: &[], + read_helpers: &[], + reference_targets: &["stg_orders"], expected_chain: &["stg_orders", "orders"], expected_order: &[("__ref__stg_orders", "__expected__orders")], expected_fragments: &["amount * 2 AS amount_doubled FROM __ref__stg_orders)"], @@ -265,6 +289,8 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen JOIN __seed(\"regions\") AS r USING (region_id)", )], assertions: &[], + read_helpers: &[], + reference_targets: &[], expected_chain: &["stg_orders"], expected_order: &[ ("__source__raw_orders", "__expected__stg_orders"), @@ -275,6 +301,44 @@ fn given_helper_cte_references_when_planning_then_references_resolve_in_dependen ], expected_absent_fragments: &[], }, + HelperReferenceTestCase { + description: "helper sharing a column name in expected rows is not read", + sql_analysis_enabled: true, + helpers: &[( + "amount_doubled", + "SELECT order_id FROM __ref(\"archived_orders\")", + )], + expected: &[( + "__expected__orders", + "SELECT 1 AS order_id, 20 AS amount_doubled", + )], + assertions: &[], + read_helpers: &[], + reference_targets: &[], + expected_chain: &["stg_orders", "orders"], + expected_order: &[("__ref__orders", "__expected__orders")], + expected_fragments: &["SELECT 1 AS order_id, 20 AS amount_doubled"], + expected_absent_fragments: &["__ref__archived_orders"], + }, + HelperReferenceTestCase { + description: "helper sharing a column name in expected rows is not read without sql analysis", + sql_analysis_enabled: false, + helpers: &[( + "amount_doubled", + "SELECT order_id FROM __ref(\"archived_orders\")", + )], + expected: &[( + "__expected__orders", + "SELECT 1 AS order_id, 20 AS amount_doubled", + )], + assertions: &[], + read_helpers: &[], + reference_targets: &[], + expected_chain: &["stg_orders", "orders"], + expected_order: &[("__ref__orders", "__expected__orders")], + expected_fragments: &["SELECT 1 AS order_id, 20 AS amount_doubled"], + expected_absent_fragments: &["__ref__archived_orders"], + }, ]; for test_case in test_cases { let artifact: Value = plan_helper_reference_case(&test_case); @@ -336,10 +400,14 @@ fn given_unresolvable_reader_references_when_planning_directly_then_planner_reje "__assert__no_returns", "SELECT order_id FROM __source(\"returns\")", )], + read_helpers: &[], + reference_targets: &[], + sends_compiler_reads: true, expected_error_fragments: &[ - "compile_input:", - "CTE '__assert__no_returns' calls __source(\"returns\")", - "__source__returns AS (SELECT ...)", + "sql_test_reference:", + "\"cteName\":\"__assert__no_returns\"", + "\"call\":\"__source(\\\"returns\\\")\"", + "\"mockCte\":\"__source__returns\"", ], }, UnresolvedReaderReferenceTestCase { @@ -351,9 +419,12 @@ fn given_unresolvable_reader_references_when_planning_directly_then_planner_reje "SELECT order_id, amount, region_id FROM __seed(\"returns\")", )], assertions: &[], + read_helpers: &[], + reference_targets: &[], + sends_compiler_reads: true, expected_error_fragments: &[ - "CTE '__expected__stg_orders' calls __seed(\"returns\")", - "__seed__returns AS (SELECT ...)", + "\"cteName\":\"__expected__stg_orders\"", + "\"mockCte\":\"__seed__returns\"", ], }, UnresolvedReaderReferenceTestCase { @@ -368,24 +439,46 @@ fn given_unresolvable_reader_references_when_planning_directly_then_planner_reje "SELECT 1 AS order_id, 10 AS amount, 7 AS region_id", )], assertions: &[("__assert__no_legacy", "SELECT order_id FROM legacy_orders")], + read_helpers: &["legacy_orders"], + reference_targets: &[], + sends_compiler_reads: true, + expected_error_fragments: &[ + "\"cteName\":\"legacy_orders\"", + "\"mockCte\":\"__dbt_ref__warehouse__orders\"", + ], + }, + UnresolvedReaderReferenceTestCase { + description: "request without the compiler's reads", + sql_analysis_enabled: true, + helpers: &[HELPER_READS_MODEL], + expected: &[], + assertions: &[ASSERT_DOUBLED], + read_helpers: &[], + reference_targets: &[], + sends_compiler_reads: false, expected_error_fragments: &[ - "CTE 'legacy_orders' calls __dbt_ref(\"warehouse\", \"orders\")", - "__dbt_ref__warehouse__orders AS (SELECT ...)", + "planner_input:", + "omits readHelperNames or referenceTargetModelNames", ], }, ]; for test_case in test_cases { - let error = plan_helper_reference_response(&HelperReferenceTestCase { - description: test_case.description, - sql_analysis_enabled: test_case.sql_analysis_enabled, - helpers: test_case.helpers, - expected: test_case.expected, - assertions: test_case.assertions, - expected_chain: &[], - expected_order: &[], - expected_fragments: &[], - expected_absent_fragments: &[], - }) + let error = plan_helper_reference_response( + &HelperReferenceTestCase { + description: test_case.description, + sql_analysis_enabled: test_case.sql_analysis_enabled, + helpers: test_case.helpers, + expected: test_case.expected, + assertions: test_case.assertions, + read_helpers: test_case.read_helpers, + reference_targets: test_case.reference_targets, + expected_chain: &[], + expected_order: &[], + expected_fragments: &[], + expected_absent_fragments: &[], + }, + test_case.sends_compiler_reads, + ) .expect_err(test_case.description); assert!( diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs index f76bbcf0fb..e08180e949 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs @@ -125,6 +125,8 @@ pub(crate) struct HelperReferenceTestCase { pub(crate) helpers: &'static [(&'static str, &'static str)], pub(crate) expected: &'static [(&'static str, &'static str)], pub(crate) assertions: &'static [(&'static str, &'static str)], + pub(crate) read_helpers: &'static [&'static str], + pub(crate) reference_targets: &'static [&'static str], pub(crate) expected_chain: &'static [&'static str], pub(crate) expected_order: &'static [(&'static str, &'static str)], pub(crate) expected_fragments: &'static [&'static str], @@ -143,5 +145,8 @@ pub(crate) struct UnresolvedReaderReferenceTestCase { pub(crate) helpers: &'static [(&'static str, &'static str)], pub(crate) expected: &'static [(&'static str, &'static str)], pub(crate) assertions: &'static [(&'static str, &'static str)], + pub(crate) read_helpers: &'static [&'static str], + pub(crate) reference_targets: &'static [&'static str], + pub(crate) sends_compiler_reads: bool, pub(crate) expected_error_fragments: &'static [&'static str], } diff --git a/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py index e46f035289..f74df90b55 100644 --- a/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py @@ -295,8 +295,8 @@ def _reference_cases() -> tuple[FailureCase, ...]: name="test-reads-only-mocks", expected_code="P013", expected_message=( - "SQL test 'test_stg_orders' reads only mocks (__ref__stg_orders), so it tests " - "no model" + "SQL test 'test_stg_orders' mocks the model it tests (__ref__stg_orders), so " + "the test has no model to run against" ), files={ "tests/unit/test_stg_orders.sql": ( 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 4cca3f45c2..4d31211a34 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md @@ -455,9 +455,10 @@ __assert__order_ids_are_not_null AS ( An assertion must read a model the test runs. A mocked name always means the mock, so a test whose `__expected__` and `__assert__` CTEs read only mocks, such as `__ref("stg_orders")` in a test that -also defines `__ref__stg_orders`, tests no model. Compile rejects it with `P013` instead of -skipping it; call `__ref()` on the model under test, or remove the test. Earlier SQLBuild versions -ran the model's real SQL for an assertion calling `__ref()` on a mocked model. +also defines `__ref__stg_orders`, has no model to run against. Compile rejects it with `P013` +instead of skipping it. Mock the model's inputs instead, such as `__source__raw__orders` above, and +keep `__ref("stg_orders")` in the assertion, or remove the test. Earlier SQLBuild versions ran the +model's real SQL for an assertion calling `__ref()` on a mocked model, so such tests used to run. Assertions can be mixed with `__expected__` CTEs in the same test, or used on their own. They are useful when the natural check is "no rows should violate this rule" rather than "the output should exactly equal these rows" - for example, duplicate checks, negative-value constraints, or conditional business rules. diff --git a/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py b/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py index 1db8f83d16..45cbc47e1b 100644 --- a/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py +++ b/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py @@ -35,7 +35,7 @@ WrittenTarget, ) from sqlbuild.compiler.compile.constants import COMPILE_RENDER_REUSE -from sqlbuild.compiler.compile.exceptions import CompileInputError +from sqlbuild.compiler.compile.exceptions import CompileInputError, SqlTestReferenceError from sqlbuild.compiler.compile.models import ( CompileAnalysisSelection, CompiledObjectKey, @@ -362,6 +362,8 @@ def compile_sql_test_planning_diagnostics( severity=DiagnosticSeverity.ERROR, code=error.code, message=str(error), + help=error.help, + location=error.location if isinstance(error, SqlTestReferenceError) else None, ), ) diff --git a/src/sqlbuild/cli/commands/_helpers/refactor/compile_facts.py b/src/sqlbuild/cli/commands/_helpers/refactor/compile_facts.py index 6c66786e36..f4c9ec2639 100644 --- a/src/sqlbuild/cli/commands/_helpers/refactor/compile_facts.py +++ b/src/sqlbuild/cli/commands/_helpers/refactor/compile_facts.py @@ -12,7 +12,7 @@ from sqlbuild.cli.commands.types import CompileLineageMode from sqlbuild.cli.compile.models import CompileAnalysis from sqlbuild.cli.entry.models import CompileProfileFlags -from sqlbuild.compiler.compile.exceptions import CompileInputError +from sqlbuild.compiler.compile.exceptions import CompileInputError, SqlTestReferenceError from sqlbuild.compiler.compile.models import CompileAnalysisSelection, CompilerDiagnostic from sqlbuild.compiler.compile.types import DiagnosticPhase, DiagnosticSeverity from sqlbuild.compiler.planner.exceptions import PlannerInputError @@ -68,6 +68,8 @@ def compile_for_refactor(*, project_dir: Path, no_cache: bool = False) -> Refact severity=DiagnosticSeverity.ERROR, code=error.code, message=str(error), + help=error.help, + location=error.location if isinstance(error, SqlTestReferenceError) else None, ) ) return RefactorCompile( diff --git a/src/sqlbuild/compiler/compile/_helpers/assembly/project.py b/src/sqlbuild/compiler/compile/_helpers/assembly/project.py index 4b6595deb4..338e8bc868 100644 --- a/src/sqlbuild/compiler/compile/_helpers/assembly/project.py +++ b/src/sqlbuild/compiler/compile/_helpers/assembly/project.py @@ -1841,6 +1841,16 @@ def _assemble_compiled_sql_test( if isinstance(test_input.payload, CompileModelSqlTestInputPayload) else () ), + read_helper_names=( + test_input.payload.read_helper_names + if isinstance(test_input.payload, CompileModelSqlTestInputPayload) + else () + ), + reference_target_model_names=( + test_input.payload.reference_target_model_names + if isinstance(test_input.payload, CompileModelSqlTestInputPayload) + else () + ), target_model_names=target_model_names, tested_resources=tested_resources, ) diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py b/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py index c90152fc52..ca3fd36208 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py @@ -38,9 +38,9 @@ extract_sql_test_ctes, ) from sqlbuild.compiler.compile._helpers.sql_tests.helper_ctes import ( - reference_target_model_names, report_test_without_target_model, report_unresolvable_test_references, + sql_test_reads, ) from sqlbuild.compiler.compile._helpers.sql_tests.native import ( extract_expanded_sql_tests_cached, @@ -65,6 +65,7 @@ LoadedMacro, MacroContext, SqlReferenceOrigin, + SqlTestReads, ) from sqlbuild.compiler.compile.types import ( SqlTestMode, @@ -338,7 +339,7 @@ def build_test_inputs( assertion_target_model_names=assertion_target_model_names, target_catalog=target_catalog, ) - reference_targets: tuple[str, ...] = () + reads: SqlTestReads = SqlTestReads(read_helper_names=(), reference_target_model_names=()) if isinstance(test_ctes.payload, CompileModelSqlTestCtes): if report_unresolvable_test_references( payload=test_ctes.payload, @@ -348,14 +349,12 @@ def build_test_inputs( syntax=sql_lexical_syntax, ): continue - reference_targets = reference_target_model_names( - payload=test_ctes.payload, syntax=sql_lexical_syntax - ) + reads = sql_test_reads(payload=test_ctes.payload, syntax=sql_lexical_syntax) if ( test.mode is SqlTestMode.MODEL and not test_ctes.payload.expected_model_names and not assertion_target_model_names - and not reference_targets + and not reads.reference_target_model_names ): report_test_without_target_model( payload=test_ctes.payload, @@ -374,7 +373,7 @@ def build_test_inputs( test_ctes=test_ctes, tested_resource_names=test.tested_resource_names, assertion_target_model_names=assertion_target_model_names, - reference_target_model_names=reference_targets, + reads=reads, ), declaration_usages=test.declaration_usages, parent_name=test.parent_name, @@ -449,7 +448,7 @@ def _build_test_input_payload( test_ctes: CompileSqlTestCtes, tested_resource_names: tuple[str, ...], assertion_target_model_names: tuple[str, ...], - reference_target_model_names: tuple[str, ...], + reads: SqlTestReads, ) -> CompileModelSqlTestInputPayload | CompileDirectLogicSqlTestInputPayload: match test_ctes.payload: case CompileModelSqlTestCtes() as model_payload: @@ -466,7 +465,8 @@ def _build_test_input_payload( assertion_ctes=model_payload.assertion_ctes, assertion_names=model_payload.assertion_names, assertion_target_model_names=assertion_target_model_names, - reference_target_model_names=reference_target_model_names, + read_helper_names=reads.read_helper_names, + reference_target_model_names=reads.reference_target_model_names, ) case CompileDirectLogicSqlTestCtes() as direct_logic_payload: return CompileDirectLogicSqlTestInputPayload( diff --git a/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py b/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py index 3a56b787ff..917b3c6641 100644 --- a/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py +++ b/src/sqlbuild/compiler/compile/_helpers/sql_tests/helper_ctes.py @@ -26,6 +26,7 @@ CompileSqlReference, CompileSqlTestCte, SqlTestCteGraph, + SqlTestReads, ) from sqlbuild.compiler.compile.types import ( CompiledResourceType, @@ -111,18 +112,17 @@ def reachable_cte_keys(*, graph: SqlTestCteGraph, roots: Iterable[str]) -> tuple return tuple(visited) -def reference_target_model_names( - *, payload: CompileModelSqlTestCtes, syntax: SqlLexicalSyntax -) -> tuple[str, ...]: - """Return unmocked models the test's expected rows and the helpers it reads ``__ref()``.""" +def sql_test_reads(*, payload: CompileModelSqlTestCtes, syntax: SqlLexicalSyntax) -> SqlTestReads: + """Return the helpers a test reads and the unmocked models they and expected rows read.""" graph: SqlTestCteGraph = sql_test_cte_graph( authored_ctes=payload.authored_ctes, reader_ctes=_reader_ctes(payload) ) + read_helpers: tuple[str, ...] = _read_helper_keys(graph) mocked: frozenset[str] = frozenset(payload.mock_model_names) sqls: tuple[str, ...] = ( *(cte.sql_body for cte in payload.expected_ctes), - *(graph.ctes[key].sql_body for key in _read_helper_keys(graph)), + *(graph.ctes[key].sql_body for key in read_helpers), ) targets: list[str] = [] for sql in sqls: @@ -131,7 +131,10 @@ def reference_target_model_names( for reference in _references(sql=sql, syntax=syntax) if reference.ref_kind == SqlReferenceKind.REF and reference.ref_name not in mocked ) - return tuple(dict.fromkeys(targets)) + return SqlTestReads( + read_helper_names=tuple(graph.ctes[key].name for key in read_helpers), + reference_target_model_names=tuple(dict.fromkeys(targets)), + ) def report_unresolvable_test_references( @@ -258,18 +261,43 @@ def report_test_without_target_model( if first_call is None and kind is not SqlReferenceKind.TABLE_FUNCTION: first_call = _reference_call(reference) mocks: tuple[str, ...] = tuple(dict.fromkeys(read_mocks)) - reads: str = "reads only mocks (" + ", ".join(mocks) + ")" if mocks else "reads no model" + mocked_models: tuple[str, ...] = tuple( + mock.removeprefix(REF_TEST_CTE_PREFIX) + for mock in mocks + if mock.startswith(REF_TEST_CTE_PREFIX) + ) reader: CompileSqlTestCte | None = next(iter(_reader_ctes(payload)), None) test_name: str = test_block.name or test_file.relative_path.stem + message: str + help_text: str + if mocked_models: + plural: str = "s" if len(mocked_models) > 1 else "" + owner: str = "models'" if plural else "model's" + mocked_ctes: str = ", ".join(f"{REF_TEST_CTE_PREFIX}{name}" for name in mocked_models) + calls: str = " and ".join( + SqlReferenceKind.REF.example_call(name, quote='"') for name in mocked_models + ) + message = ( + f"SQL test '{test_name}' mocks the model{plural} it tests ({mocked_ctes}), so the " + "test has no model to run against" + ) + help_text = ( + f"Mock the {owner} inputs instead (for example __source__ or " + f"__ref__) and keep {calls} in the __assert__ or __expected__ CTE, " + "or remove the test." + ) + else: + reads: str = f"reads only mocks ({', '.join(mocks)})" if mocks else "reads no model" + message = f"SQL test '{test_name}' {reads}, so the test has no model to run against" + help_text = ( + 'Call __ref("") in an __assert__ or __expected__ CTE so the test ' + "runs that model with its mocks, or remove the test." + ) _HelperDiagnostics(test_file=test_file, test_block=test_block).report( cte_name=reader.name if reader is not None else test_name, call=first_call, - message=f"SQL test '{test_name}' {reads}, so it tests no model and would never run", - help=( - 'Call __ref("") in an __assert__ or __expected__ CTE, for example ' - '__expected__ AS (SELECT ...) or SELECT ... FROM __ref(""), so the test ' - "runs that model with its mocks, or remove the test." - ), + message=message, + help=help_text, ) @@ -490,6 +518,33 @@ def _first_cycle(*, graph: SqlTestCteGraph, keys: tuple[str, ...]) -> tuple[str, return None +def sql_test_cte_location( + *, + test_file: DiscoveredSqlTestFile, + test_block: DiscoveredSqlTestBlock, + cte_name: str, + call: str | None, +) -> SourceLocation: + """Locate a call inside one CTE of a SQL test block, else the CTE header, else the block.""" + + contents: str = test_file.contents + block_offset: int = max(contents.find(test_block.sql_body), 0) + header: re.Match[str] | None = re.compile( + rf"(? None: self._test_file: DiscoveredSqlTestFile = test_file - self._block_offset: int = max(test_file.contents.find(test_block.sql_body), 0) + self._test_block: DiscoveredSqlTestBlock = test_block self._test_name: str = test_block.name or test_file.relative_path.stem self.reported: bool = False def report(self, *, cte_name: str, call: str | None, message: str, help: str) -> None: - location: SourceLocation = self._location(cte_name=cte_name, call=call) + location: SourceLocation = sql_test_cte_location( + test_file=self._test_file, test_block=self._test_block, cte_name=cte_name, call=call + ) self.reported = True report_compile_diagnostic( key=( @@ -522,20 +579,3 @@ def report(self, *, cte_name: str, call: str | None, message: str, help: str) -> help=help, ), ) - - def _location(self, *, cte_name: str, call: str | None) -> SourceLocation: - contents: str = self._test_file.contents - header: re.Match[str] | None = re.compile( - rf"(? SourceLocation: + """Locate a call inside one CTE of a SQL test block, else the CTE header, else the block.""" + + return sql_test_cte_location( + test_file=test_file, test_block=test_block, cte_name=cte_name, call=call + ) diff --git a/src/sqlbuild/compiler/compile/exceptions.py b/src/sqlbuild/compiler/compile/exceptions.py index 5d849a9b26..7416ff7e68 100644 --- a/src/sqlbuild/compiler/compile/exceptions.py +++ b/src/sqlbuild/compiler/compile/exceptions.py @@ -2,6 +2,8 @@ from __future__ import annotations +from sqlbuild.spec.contracts.models import SourceLocation + class CompileInputError(ValueError): """Raised when discovered inputs cannot be attached into a compile view.""" @@ -15,6 +17,18 @@ def __init__(self, message: str, *, code: str | None = None, help: str | None = self.help = help +class SqlTestReferenceError(CompileInputError): + """Raised when a SQL-test CTE calls a reference the test query cannot resolve.""" + + code: str = "P013" + + def __init__(self, message: str, *, location: SourceLocation, help: str) -> None: + super().__init__( + f"{location.path.as_posix()}:{location.line}:{location.column}: {message}", help=help + ) + self.location: SourceLocation = location + + class MacroDeclarationLookupError(CompileInputError, KeyError): """Retain authored diagnostics while honoring the Mapping lookup contract.""" diff --git a/src/sqlbuild/compiler/compile/models.py b/src/sqlbuild/compiler/compile/models.py index aa92850760..edaca84ad9 100644 --- a/src/sqlbuild/compiler/compile/models.py +++ b/src/sqlbuild/compiler/compile/models.py @@ -1245,9 +1245,18 @@ class CompileModelSqlTestInputPayload: assertion_ctes: tuple[CompileSqlTestCte, ...] = field(default_factory=tuple) assertion_names: tuple[str, ...] = field(default_factory=tuple) assertion_target_model_names: tuple[str, ...] = field(default_factory=tuple) + read_helper_names: tuple[str, ...] = field(default_factory=tuple) reference_target_model_names: tuple[str, ...] = field(default_factory=tuple) +@dataclass(frozen=True) +class SqlTestReads: + """Helpers one SQL test reads and the unmocked models they and its expected rows reference.""" + + read_helper_names: tuple[str, ...] + reference_target_model_names: tuple[str, ...] + + @dataclass(frozen=True) class SqlTestCteGraph: """Which authored helper and mock CTEs one SQL test reads, keyed by case-folded name.""" @@ -1394,6 +1403,8 @@ class CompiledSqlTest: expected_model_names: tuple[str, ...] = field(default_factory=tuple) assertion_names: tuple[str, ...] = field(default_factory=tuple) assertion_target_model_names: tuple[str, ...] = field(default_factory=tuple) + read_helper_names: tuple[str, ...] = field(default_factory=tuple) + reference_target_model_names: tuple[str, ...] = field(default_factory=tuple) target_model_names: tuple[str, ...] = field(default_factory=tuple) tested_resources: tuple[CompiledSqlTestResource, ...] = field(default_factory=tuple) diff --git a/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py b/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py index 87c199d9ab..89a301f85a 100644 --- a/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py +++ b/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py @@ -9,7 +9,9 @@ import sqlbuild._native as _native from sqlbuild.adapter.contract.classes.base_adapter import BaseAdapter -from sqlbuild.compiler.compile.exceptions import CompileInputError +from sqlbuild.compiler.compile.classes.sql_test_cte_locator import SqlTestCteLocator +from sqlbuild.compiler.compile.constants import SQL_TEST_HELPER_REFERENCE_CODE +from sqlbuild.compiler.compile.exceptions import CompileInputError, SqlTestReferenceError from sqlbuild.compiler.compile.models import ( CompiledDirectLogicSqlTestPayload, CompiledModel, @@ -38,6 +40,8 @@ from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.executor.testing.types import NativeSqlTestRenderingModule +_UNRESOLVED_REFERENCE_PREFIX: str = "sql_test_reference:" + _CALL_SUFFIX_SENTINEL: str = "__SQLBUILD_CALL_SUFFIX__" _NATIVE_WORKERS: int = 4 @@ -116,7 +120,7 @@ def plan_sql_tests_natively( orjson.dumps(request, option=orjson.OPT_SORT_KEYS).decode() ) except ValueError as error: - raise _planning_error(error=error) from None + raise _planning_error(error=error, tests=tests) from None response_payload: object = orjson.loads(native_response) if not isinstance(response_payload, dict): raise NativeSqlTestPlanningError( @@ -168,7 +172,7 @@ def resolve_sql_test_model_chains( NativeSqlTestRenderingModule, _native ).resolve_sql_test_chains_json(orjson.dumps(request, option=orjson.OPT_SORT_KEYS).decode()) except ValueError as error: - raise _planning_error(error=error) from None + raise _planning_error(error=error, tests=tests) from None response_payload: object = orjson.loads(native_response) chains: object = response_payload.get("chains") if isinstance(response_payload, dict) else None if not isinstance(chains, list) or len(chains) != len(tests): @@ -178,8 +182,12 @@ def resolve_sql_test_model_chains( return tuple(_string_tuple(value=chain, context="chain") for chain in chains) -def _planning_error(*, error: ValueError) -> Exception: +def _planning_error(*, error: ValueError, tests: tuple[CompiledSqlTest, ...]) -> Exception: message: str = str(error) + if message.startswith(_UNRESOLVED_REFERENCE_PREFIX): + return _unresolved_reference_error( + payload=orjson.loads(message.removeprefix(_UNRESOLVED_REFERENCE_PREFIX)), tests=tests + ) if message.startswith("compile_input:"): return CompileInputError(message.removeprefix("compile_input:")) if message.startswith("planner_input:"): @@ -187,6 +195,38 @@ def _planning_error(*, error: ValueError) -> Exception: return NativeSqlTestPlanningError(f"native SQL-test planning failed: {message}") +def _unresolved_reference_error( + *, payload: dict[str, str], tests: tuple[CompiledSqlTest, ...] +) -> Exception: + """Locate a reference call the native planner could not resolve in its test file.""" + + test: CompiledSqlTest | None = next( + ( + test + for test in tests + if test.name == payload["testName"] + and str(test.test_file.relative_path) == payload["fileLabel"] + ), + None, + ) + cte_name: str = payload["cteName"] + call: str = payload["call"] + message: str = f"SQL test CTE '{cte_name}' calls {call}, which the test query cannot resolve" + help_text: str = ( + f"Mock it in the test, for example {payload['mockCte']} AS (SELECT ...), or call " + '__ref("") on a model the test runs.' + ) + if test is None: + return CompileInputError(message, code=SQL_TEST_HELPER_REFERENCE_CODE, help=help_text) + return SqlTestReferenceError( + message, + location=SqlTestCteLocator.locate( + test_file=test.test_file, test_block=test.test_block, cte_name=cte_name, call=call + ), + help=help_text, + ) + + def _plan_from_payload(*, value: object) -> NativeSqlTestPlan: payload: dict[str, Any] = _object(value=value, context="result") sql: object = payload.get("sql") @@ -413,6 +453,8 @@ def _test_request( "expectedCtes": [_cte_request(cte=cte) for cte in payload.expected_ctes], "expectedModelNames": list(payload.expected_model_names), "assertionCtes": [_cte_request(cte=cte) for cte in payload.assertion_ctes], + "readHelperNames": list(test.read_helper_names), + "referenceTargetModelNames": list(test.reference_target_model_names), } return { "name": test.name, 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 5a547ff593..7c0439a645 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py @@ -1827,6 +1827,16 @@ def prepare_optional_ceremony_project(*, tmp_path: Path, test_files: dict[str, s ' SELECT item_id FROM __source("item_returns")\n' ")\n" ), + "helper_named_like_column_reads_unknown_model": ( + "__ref__items AS (\n SELECT 7 AS item_id, 5 AS amount\n),\n" + 'amount_doubled AS (SELECT item_id FROM __ref("item_archive")),\n' + "__expected__item_totals AS (\n SELECT 7 AS item_id, 10 AS amount_doubled\n)\n" + ), + "helper_named_like_column_reads_model": ( + "__ref__items AS (\n SELECT 7 AS item_id, 5 AS amount\n),\n" + 'amount_doubled AS (SELECT item_id FROM __ref("item_extras")),\n' + "__expected__item_totals AS (\n SELECT 7 AS item_id, 10 AS amount_doubled\n)\n" + ), "assertion_reads_only_mocks": ( "__ref__items AS (\n SELECT 7 AS item_id, 5 AS amount\n),\n" "__assert__only_mocked_items AS (\n" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py index b623b0ae89..dae4e50795 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_references.py @@ -364,6 +364,96 @@ expected_compiled_fragments=("amount * 2 AS amount_doubled FROM __ref__item_extras",), expected_absent_compiled_fragments=("__ref(",), ), + HelperReferenceE2ETestCase( + description="unread helper named like a column reads an unknown model on the python engine", + tests=("helper_named_like_column_reads_unknown_model",), + command=("--no-color", "test"), + engine="python", + sql_analysis=True, + expected_output_fragments=("PASS=1 FAIL=0 TOTAL=1",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(", "item_archive"), + ), + HelperReferenceE2ETestCase( + description="unread helper named like a column reads an unknown model on the python engine without sql analysis", + tests=("helper_named_like_column_reads_unknown_model",), + command=("--no-color", "test"), + engine="python", + sql_analysis=False, + expected_output_fragments=("PASS=1 FAIL=0 TOTAL=1",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(", "item_archive"), + ), + HelperReferenceE2ETestCase( + description="unread helper named like a column selects no model on the python engine", + tests=("helper_named_like_column_reads_model",), + command=("--no-color", "test", "--select", "item_extras"), + engine="python", + sql_analysis=True, + expected_output_fragments=("0 selected",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(", "__ref__item_extras"), + ), + HelperReferenceE2ETestCase( + description="unread helper named like a column reads an unknown model on the native engine", + tests=("helper_named_like_column_reads_unknown_model",), + command=("--no-color", "test"), + engine="native", + sql_analysis=True, + expected_output_fragments=("PASS=1 FAIL=0 TOTAL=1",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(", "item_archive"), + ), + HelperReferenceE2ETestCase( + description="unread helper named like a column reads an unknown model on the native engine without sql analysis", + tests=("helper_named_like_column_reads_unknown_model",), + command=("--no-color", "test"), + engine="native", + sql_analysis=False, + expected_output_fragments=("PASS=1 FAIL=0 TOTAL=1",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(", "item_archive"), + ), + HelperReferenceE2ETestCase( + description="unread helper named like a column selects no model on the native engine", + tests=("helper_named_like_column_reads_model",), + command=("--no-color", "test", "--select", "item_extras"), + engine="native", + sql_analysis=True, + expected_output_fragments=("0 selected",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(", "__ref__item_extras"), + ), + HelperReferenceE2ETestCase( + description="unread helper named like a column reads an unknown model on the native-preview engine", + tests=("helper_named_like_column_reads_unknown_model",), + command=("--no-color", "test"), + engine="native-preview", + sql_analysis=True, + expected_output_fragments=("PASS=1 FAIL=0 TOTAL=1",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(", "item_archive"), + ), + HelperReferenceE2ETestCase( + description="unread helper named like a column reads an unknown model on the native-preview engine without sql analysis", + tests=("helper_named_like_column_reads_unknown_model",), + command=("--no-color", "test"), + engine="native-preview", + sql_analysis=False, + expected_output_fragments=("PASS=1 FAIL=0 TOTAL=1",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(", "item_archive"), + ), + HelperReferenceE2ETestCase( + description="unread helper named like a column selects no model on the native-preview engine", + tests=("helper_named_like_column_reads_model",), + command=("--no-color", "test", "--select", "item_extras"), + engine="native-preview", + sql_analysis=True, + expected_output_fragments=("0 selected",), + expected_compiled_fragments=(), + expected_absent_compiled_fragments=("__ref(", "__ref__item_extras"), + ), ), ids=lambda case: case.description, ) @@ -507,12 +597,15 @@ def test_given_helper_cte_references_when_testing_then_helpers_read_the_tested_m engine="python", expected_code="P013", expected_message=( - "SQL test 'assertion_reads_only_mocks' reads only mocks (__ref__items), so it " - "tests no model and would never run" + "SQL test 'assertion_reads_only_mocks' mocks the model it tests (__ref__items), " + "so the test has no model to run against" ), expected_line=8, expected_column=23, - expected_help_fragment='Call __ref("") in an __assert__ or __expected__ CTE', + expected_help_fragment=( + "Mock the model's inputs instead (for example __source__ or " + '__ref__) and keep __ref("items")' + ), ), HelperReferenceErrorE2ETestCase( description="expected rows reading an unknown model on the python engine", @@ -547,12 +640,15 @@ def test_given_helper_cte_references_when_testing_then_helpers_read_the_tested_m engine="native", expected_code="P013", expected_message=( - "SQL test 'assertion_reads_only_mocks' reads only mocks (__ref__items), so it " - "tests no model and would never run" + "SQL test 'assertion_reads_only_mocks' mocks the model it tests (__ref__items), " + "so the test has no model to run against" ), expected_line=8, expected_column=23, - expected_help_fragment='Call __ref("") in an __assert__ or __expected__ CTE', + expected_help_fragment=( + "Mock the model's inputs instead (for example __source__ or " + '__ref__) and keep __ref("items")' + ), ), HelperReferenceErrorE2ETestCase( description="expected rows reading an unknown model on the native engine", @@ -587,12 +683,15 @@ def test_given_helper_cte_references_when_testing_then_helpers_read_the_tested_m engine="native-preview", expected_code="P013", expected_message=( - "SQL test 'assertion_reads_only_mocks' reads only mocks (__ref__items), so it " - "tests no model and would never run" + "SQL test 'assertion_reads_only_mocks' mocks the model it tests (__ref__items), " + "so the test has no model to run against" ), expected_line=8, expected_column=23, - expected_help_fragment='Call __ref("") in an __assert__ or __expected__ CTE', + expected_help_fragment=( + "Mock the model's inputs instead (for example __source__ or " + '__ref__) and keep __ref("items")' + ), ), HelperReferenceErrorE2ETestCase( description="expected rows reading an unknown model on the native-preview engine", 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 5e7d8ca6d5..30db6f6ad9 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 @@ -103,7 +103,7 @@ + ",\n__assert__has_rows AS (SELECT 1 FROM built_orders WHERE id IS NULL)\n" }, expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), - expected_outcome_fragment="helper_target_model_names=('orders',)", + expected_outcome_fragment="reference_target_model_names=('orders',)", ), AttachmentProjectTestCase( description="a helper chain where one helper reads another", @@ -115,7 +115,7 @@ + ",\n__assert__has_ids AS (SELECT 1 FROM built_ids WHERE id IS NULL)\n" }, expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), - expected_outcome_fragment="helper_target_model_names=('orders',)", + expected_outcome_fragment="reference_target_model_names=('orders',)", ), AttachmentProjectTestCase( description="an unread helper referencing an unknown model", @@ -126,7 +126,7 @@ + "\n" }, expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), - expected_outcome_fragment="helper_target_model_names=()", + expected_outcome_fragment="read_helper_names=(), reference_target_model_names=()", ), AttachmentProjectTestCase( description="a read helper referencing an unknown model reports P013", diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py index 6338bde708..be518b4b4f 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py @@ -8,7 +8,7 @@ from sqlbuild.adapter.contract.classes.base_adapter import BaseAdapter from sqlbuild.adapters.duckdb.classes.duckdb_adapter import DuckDbAdapter from sqlbuild.adapters.sqlserver.classes.sqlserver_adapter import SqlServerAdapter -from sqlbuild.compiler.compile.exceptions import CompileInputError +from sqlbuild.compiler.compile.exceptions import CompileInputError, SqlTestReferenceError from sqlbuild.compiler.compile.models import ( CompiledDirectLogicSqlTestPayload, CompiledFunction, @@ -1574,3 +1574,47 @@ def test_given_unflattened_with_when_building_assertion_ctes_then_raises_clear_e with pytest.raises(PlannerInputError, match=test_case.expected_error_fragment): plan_single_test(test=compiled_test, project=project, adapter=SqlServerAdapter()) + + +@pytest.mark.parametrize( + "test_case", + [ + PlanTestChainTestCase( + description="assertion calling an unmocked source fails planning with located P013", + model_queries={"orders": 'SELECT id FROM __source("raw")'}, + mock_ref_ctes={}, + mock_source_ctes={"raw": "SELECT 1 AS id"}, + helper_ctes={}, + expected_model_names=("orders",), + expected_chain_length=1, + expected_error_fragments=( + "tests/unit/test_chain.sql:1:1: SQL test CTE '__assert__no_returns' calls " + '__source("returns"), which the test query cannot resolve', + "__source__returns AS (SELECT ...)", + ), + expected_cte_bodies={"orders": "SELECT 1 AS id"}, + assertion_ctes={"no_returns": 'SELECT id FROM __source("returns")'}, + ) + ], + ids=lambda case: case.description, +) +def test_given_reference_the_compiler_missed_when_planning_then_it_fails_with_located_p013( + test_case: PlanTestChainTestCase, +) -> None: + compiled_test: CompiledSqlTest + project: CompiledProject + compiled_test, project = build_test_and_project(test_case) + + with pytest.raises(SqlTestReferenceError) as raised: + plan_and_render_sql_test_artifacts( + project=project, + tests=(compiled_test,), + adapter=DuckDbAdapter(), + sql_analysis_enabled=True, + ) + + assert raised.value.code == "P013" + assert raised.value.location.path == Path("tests/unit/test_chain.sql") + assert raised.value.message == test_case.expected_error_fragments[0] + assert raised.value.help is not None + assert test_case.expected_error_fragments[1] in raised.value.help diff --git a/website/src/content/docs/docs/concepts/testing.mdx b/website/src/content/docs/docs/concepts/testing.mdx index 87ad9c6246..127424023a 100644 --- a/website/src/content/docs/docs/concepts/testing.mdx +++ b/website/src/content/docs/docs/concepts/testing.mdx @@ -431,9 +431,10 @@ __assert__order_ids_are_not_null AS ( An assertion must read a model the test runs. A mocked name always means the mock, so a test whose `__expected__` and `__assert__` CTEs read only mocks, such as `__ref("stg_orders")` in a test that -also defines `__ref__stg_orders`, tests no model. Compile rejects it with `P013` instead of -skipping it; call `__ref()` on the model under test, or remove the test. Earlier SQLBuild versions -ran the model's real SQL for an assertion calling `__ref()` on a mocked model. +also defines `__ref__stg_orders`, has no model to run against. Compile rejects it with `P013` +instead of skipping it. Mock the model's inputs instead, such as `__source__raw__orders` above, and +keep `__ref("stg_orders")` in the assertion, or remove the test. Earlier SQLBuild versions ran the +model's real SQL for an assertion calling `__ref()` on a mocked model, so such tests used to run. Assertions can be mixed with `__expected__` CTEs in the same test, or used on their own. They are useful when the natural check is "no rows should violate this rule" rather than "the output should exactly equal these rows" - for example, duplicate checks, negative-value constraints, or conditional business rules. From d43c34157be15fcabea3bf15c778c1310416fd38 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 8 Oct 2026 09:07:40 +0100 Subject: [PATCH 4/4] fix(test): satisfy the manual_contains lint in mocked reference checks --- .../src/compiler/_helpers/sql_tests/reader_references.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs index 103845aa15..dc5d8e0091 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/reader_references.rs @@ -61,7 +61,7 @@ pub(crate) fn unresolvable_call( (Some(first), None) => first.as_str().to_string(), _ => continue, }; - let runs = runs_models && chain.iter().any(|model| *model == name); + let runs = runs_models && chain.contains(&name); if !runs && !mocks.contains_key(&name) { return Some(full.as_str().to_string()); }