Skip to content

Commit 82c25be

Browse files
committed
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.
1 parent 5e97f21 commit 82c25be

22 files changed

Lines changed: 761 additions & 284 deletions

File tree

‎crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs‎

Lines changed: 23 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use crate::compiler::_helpers::sql_tests::planning::{
1515
DBT_REF_PREFIX, REF_PREFIX, SEED_PREFIX, SOURCE_PREFIX, SqlTestPatterns, TABLE_FUNCTION_PREFIX,
1616
TestFixtures, compile_error,
1717
};
18+
use crate::compiler::_helpers::sql_tests::reader_references::calls_reference;
1819
use crate::constants::{
1920
QUOTED_IDENTIFIER_DELIMITER_BYTES, SQL_TEST_ACTUAL_CTE, SQL_TEST_ACTUAL_CTE_PREFIX,
2021
SQL_TEST_EXPECTED_CTE,
@@ -80,6 +81,8 @@ pub(crate) struct ScopeRequest<'a> {
8081
pub(crate) rename_dialect: Option<&'a Dialect>,
8182
pub(crate) flat_dialect: bool,
8283
pub(crate) slice_dialect: SliceDialect,
84+
/// Helpers the compiler found the test reads; an unread helper calling a reference is left out.
85+
pub(crate) read_helper_names: &'a [String],
8386
}
8487

8588
#[derive(Clone, Copy, PartialEq, Eq, Hash)]
@@ -160,13 +163,17 @@ impl ScopeGraph {
160163
.mocks_by_generated_name
161164
.insert(mock.generated_name.clone(), index);
162165
}
163-
if fixtures.helpers.is_empty() {
164-
return Ok(graph);
165-
}
166-
for (index, cte) in fixtures.helpers.iter().enumerate() {
167-
graph
168-
.helper_indexes
169-
.insert(cte.name.to_ascii_lowercase(), index);
166+
let read: HashSet<String> = request
167+
.read_helper_names
168+
.iter()
169+
.map(|name| name.to_ascii_lowercase())
170+
.collect();
171+
for cte in &fixtures.helpers {
172+
let key = cte.name.to_ascii_lowercase();
173+
if !read.contains(&key) && calls_reference(&cte.sql_body, patterns) {
174+
continue;
175+
}
176+
graph.helper_indexes.insert(key, graph.helpers.len());
170177
graph.helpers.push(ScopeHelper {
171178
name: cte.name.clone(),
172179
original: cte.sql_body.clone(),
@@ -176,6 +183,9 @@ impl ScopeGraph {
176183
resolved: None,
177184
});
178185
}
186+
if graph.helpers.is_empty() {
187+
return Ok(graph);
188+
}
179189
let readers = test_readers(fixtures);
180190
graph.reject_shadowed_helpers(&readers, request)?;
181191
if let Some(dialect) = request.rename_dialect
@@ -294,36 +304,12 @@ impl ScopeGraph {
294304
order.ordered
295305
}
296306

297-
/// Helpers the test's expected rows and assertions read, except those a mock it uses reads.
298-
pub(crate) fn read_helpers(
299-
&self,
300-
readers: &[&str],
301-
used_mocks: &HashSet<String>,
302-
patterns: &SqlTestPatterns,
303-
) -> HashSet<usize> {
304-
let helpers = |nodes: Vec<ScopeNode>| -> HashSet<usize> {
305-
nodes
306-
.into_iter()
307-
.filter_map(|node| match node {
308-
ScopeNode::Helper(index) => Some(index),
309-
ScopeNode::Mock(_) => None,
310-
})
311-
.collect()
312-
};
313-
let mut read: HashSet<usize> = HashSet::new();
314-
for sql in readers {
315-
read.extend(helpers(
316-
self.closure(&identifier_tokens(sql, patterns), true),
317-
));
318-
}
319-
for mock in &self.mocks {
320-
if used_mocks.contains(&mock.mock_name) {
321-
for index in helpers(self.closure(&mock.tokens, true)) {
322-
read.remove(&index);
323-
}
324-
}
325-
}
326-
read
307+
/// Indexes of the helpers the compiler found the test's expected rows and assertions read.
308+
pub(crate) fn read_helper_indexes(&self, names: &[String]) -> HashSet<usize> {
309+
names
310+
.iter()
311+
.filter_map(|name| self.helper_indexes.get(&name.to_ascii_lowercase()).copied())
312+
.collect()
327313
}
328314

329315
/// 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)> {
525511
readers
526512
}
527513

528-
/// Authored SQL of the helpers `readers` reach, directly or through other helpers and mocks.
529-
pub(crate) fn read_helper_sql<'f>(
530-
fixtures: &'f TestFixtures,
531-
readers: &[&str],
532-
patterns: &SqlTestPatterns,
533-
) -> Vec<&'f str> {
534-
if fixtures.helpers.is_empty() {
535-
return Vec::new();
536-
}
537-
let mut bodies: HashMap<String, (&'f str, bool)> = fixtures
538-
.helpers
539-
.iter()
540-
.map(|cte| (cte.name.to_ascii_lowercase(), (cte.sql_body.as_str(), true)))
541-
.collect();
542-
for (prefix, group) in [
543-
(REF_PREFIX, &fixtures.mock_refs),
544-
(SOURCE_PREFIX, &fixtures.mock_sources),
545-
(SEED_PREFIX, &fixtures.mock_seeds),
546-
(DBT_REF_PREFIX, &fixtures.mock_dbt_refs),
547-
(TABLE_FUNCTION_PREFIX, &fixtures.mock_table_functions),
548-
] {
549-
for (name, sql) in group {
550-
bodies.insert(
551-
format!("{prefix}{name}").to_ascii_lowercase(),
552-
(sql.as_str(), false),
553-
);
554-
}
555-
}
556-
let mut pending: Vec<String> = readers
557-
.iter()
558-
.flat_map(|sql| identifier_tokens(sql, patterns))
559-
.collect();
560-
let mut visited: HashSet<String> = HashSet::new();
561-
let mut helpers: Vec<&'f str> = Vec::new();
562-
while let Some(token) = pending.pop() {
563-
let Some((sql, is_helper)) = bodies.get(&token).copied() else {
564-
continue;
565-
};
566-
if !visited.insert(token) {
567-
continue;
568-
}
569-
if is_helper {
570-
helpers.push(sql);
571-
}
572-
pending.extend(identifier_tokens(sql, patterns));
573-
}
574-
helpers
575-
}
576-
577514
/// Return the helper and mock CTEs `sql` reads, transitively and dependencies first.
578515
pub(crate) fn helper_scope_ctes(
579516
sql: &str,

0 commit comments

Comments
 (0)