Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 54 additions & 13 deletions src/adapters/explore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use crate::adapters::asset_embed::{
};
use crate::adapters::render::{DagPayload, ReportPayload};
use crate::domain::{
GrainKind, Manifest, ModelInScopeSet, Node, NodeId, model_grain_signals, resolve_target_model,
GrainKind, Manifest, ModelInScopeSet, Node, NodeId, model_grain_signals, resolve_tested_model,
};

/// The explorer's external-drive contract version (cute-dbt#105).
Expand Down Expand Up @@ -93,8 +93,8 @@ pub struct LineageNode {
/// `data_test_counts` helper below).
pub data_tests: usize,
/// Unit tests targeting this model (cute-dbt#103) — manifest
/// `unit_tests` entries whose bare `model:` reference resolves here
/// ([`resolve_target_model`], the same bridge the report uses).
/// `unit_tests` entries whose target resolves here
/// ([`resolve_tested_model`], the same bridge the report uses).
pub unit_tests: usize,
}

Expand Down Expand Up @@ -128,15 +128,16 @@ fn data_test_counts(current: &Manifest) -> HashMap<&NodeId, usize> {
}

/// Count the unit tests per target model (cute-dbt#103): each manifest
/// `unit_tests` entry stores the BARE model name, bridged to its node
/// by [`resolve_target_model`] (the report renderer's exact resolution
/// — the two surfaces cannot disagree on a test's target). An
/// unresolvable `model:` reference contributes nothing (skipped, not
/// failed — the explore fail-open posture).
/// `unit_tests` entry is bridged to its node by [`resolve_tested_model`]
/// (the engine-resolved id when present, the bare `model:` name
/// otherwise — the report renderer's exact resolution, so the two
/// surfaces cannot disagree on a test's target). An unresolvable
/// `model:` reference contributes nothing (skipped, not failed — the
/// explore fail-open posture).
fn unit_test_counts(current: &Manifest) -> HashMap<NodeId, usize> {
let mut counts: HashMap<NodeId, usize> = HashMap::new();
for unit_test in current.unit_tests().values() {
if let Some(model) = resolve_target_model(current, unit_test.model()) {
if let Some(model) = resolve_tested_model(current, unit_test) {
*counts.entry(model.id().clone()).or_insert(0) += 1;
}
}
Expand Down Expand Up @@ -479,8 +480,8 @@ fn model_detail(current: &Manifest, node: Option<&Node>) -> ModelDetailPayload {
}

/// Collect each model's unit-test file paths (cute-dbt#105), keyed by
/// resolved target model: each `unit_tests` entry stores the BARE model
/// name, bridged by [`resolve_target_model`] (the [`unit_test_counts`]
/// resolved target model: each `unit_tests` entry is bridged by
/// [`resolve_tested_model`] (the [`unit_test_counts`]
/// twin — the badge count and the paths list cannot disagree on a
/// test's target). Entries are name-ordered per model (the manifest
/// `unit_tests` map iterates non-deterministically). An unresolvable
Expand All @@ -489,7 +490,7 @@ fn model_detail(current: &Manifest, node: Option<&Node>) -> ModelDetailPayload {
fn unit_test_paths_by_model(current: &Manifest) -> HashMap<NodeId, Vec<UnitTestPathsPayload>> {
let mut by_model: HashMap<NodeId, Vec<UnitTestPathsPayload>> = HashMap::new();
for unit_test in current.unit_tests().values() {
let Some(model) = resolve_target_model(current, unit_test.model()) else {
let Some(model) = resolve_tested_model(current, unit_test) else {
continue;
};
// given-order fixture refs, then the expect's — verbatim off
Expand Down Expand Up @@ -961,6 +962,46 @@ mod tests {
])
}

// ----- unit_test_counts (cute-dbt#254) ---------------------------

#[test]
fn unit_test_counts_bind_a_versioned_model_via_engine_resolved_id() {
// A versioned model's leaf segment is its version suffix
// (`…dim_customers.v2` → `"v2"`), so bare-name resolution can
// never bind it — the engine-resolved `tested_node_unique_id`
// must carry the badge attribution (cute-dbt#254).
use crate::domain::{UnitTest, UnitTestExpect};
let versioned = model("model.shop.dim_customers.v2", Some("select 1"), &[]);
let ut = UnitTest::new(
"t1",
NodeId::new("dim_customers"),
Vec::new(),
UnitTestExpect::new(serde_json::Value::Null, None, None),
None,
DependsOn::default(),
None,
None,
None,
)
.with_tested_node_unique_id(Some(NodeId::new("model.shop.dim_customers.v2")));
let current = Manifest::new(
ManifestMetadata::new("v12"),
[(versioned.id().clone(), versioned)].into_iter().collect(),
[("unit_test.shop.t1.v2".to_owned(), ut)]
.into_iter()
.collect(),
StdHashMap::new(),
);
let counts = unit_test_counts(&current);
assert_eq!(
counts
.get(&NodeId::new("model.shop.dim_customers.v2"))
.copied(),
Some(1),
"the versioned model's badge must count its unit test",
);
}

// ----- build_lineage --------------------------------------------

#[test]
Expand Down Expand Up @@ -1410,7 +1451,7 @@ mod tests {
}

/// A minimal unit test targeting `target_bare` (the manifest stores
/// the BARE model name — resolution is `resolve_target_model`).
/// the BARE model name — resolution is `resolve_tested_model`).
fn unit_test_on(target_bare: &str) -> crate::domain::UnitTest {
crate::domain::UnitTest::new(
format!("test_{target_bare}"),
Expand Down
93 changes: 93 additions & 0 deletions src/adapters/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,24 @@ struct WireUnitTest {
/// case — a bare struct field rejects `null` and fails the parse.
#[serde(default)]
overrides: Option<WireUnitTestOverrides>,
/// Engine-resolved `unique_id` of the tested model (cute-dbt#254).
/// Top-level sibling of `model` (`ManifestUnitTest
/// .tested_node_unique_id`, `dbt-schemas`
/// `manifest/manifest_nodes.rs:273` @ `9977b6cb…`). For versioned
/// models this is the `.vN`-suffixed `unique_id` — the only handle
/// that binds them (the bare `model:` name never leaf-matches a
/// versioned id). `Option` + `#[serde(default)]` tolerate both the
/// absent key (every committed fixture predates the field) and an
/// explicit `null` (the engine null-fill shape).
#[serde(default)]
tested_node_unique_id: Option<NodeId>,
/// Engine-resolved `unique_id` of the node backing a `this` given
/// input — the [`Self::tested_node_unique_id`] twin
/// (`manifest_nodes.rs:274` @ `9977b6cb…`; fusion never populates
/// it as of that SHA — dbt-core parity field). Same tolerance
/// contract.
#[serde(default)]
this_input_node_unique_id: Option<NodeId>,
}

/// Tolerant wire projection of the `config` sub-object on a dbt unit-test
Expand Down Expand Up @@ -453,6 +471,8 @@ impl WireUnitTest {
)
.with_incremental_mode(is_incremental_mode)
.with_overrides(overrides)
.with_tested_node_unique_id(self.tested_node_unique_id)
.with_this_input_node_unique_id(self.this_input_node_unique_id)
}
}

Expand Down Expand Up @@ -764,6 +784,79 @@ mod tests {
);
}

// ----- cute-dbt#254 — engine-resolved unit-test target ids ------

#[test]
fn parse_manifest_threads_engine_resolved_target_ids() {
// The versioned-model wire shape: fusion resolves each per-version
// unit test's target to the `.vN`-suffixed unique_id
// (`tested_node_unique_id` / `this_input_node_unique_id`,
// `dbt-schemas` `manifest/manifest_nodes.rs:273-274` @ `9977b6cb…`).
let json = format!(
r#"{{
"metadata": {{ "dbt_schema_version": "{V12_URL}" }},
"nodes": {{
"model.shop.dim_customers.v2": {{
"resource_type": "model",
"checksum": {{ "name": "sha256", "checksum": "deadbeef" }},
"compiled_code": "select 1"
}}
}},
"unit_tests": {{
"unit_test.shop.dim_customers.t1.v2": {{
"name": "t1",
"model": "dim_customers",
"expect": {{ "rows": [{{"id":1}}], "format": "dict" }},
"tested_node_unique_id": "model.shop.dim_customers.v2",
"this_input_node_unique_id": "model.shop.dim_customers.v2"
}}
}}
}}"#
);
let manifest = parse_manifest(&json).expect("versioned manifest parses");
let ut = manifest
.unit_test("unit_test.shop.dim_customers.t1.v2")
.expect("unit test ingested");
assert_eq!(
ut.tested_node_unique_id().map(NodeId::as_str),
Some("model.shop.dim_customers.v2"),
);
assert_eq!(
ut.this_input_node_unique_id().map(NodeId::as_str),
Some("model.shop.dim_customers.v2"),
);
}

#[test]
fn parse_manifest_tolerates_absent_and_null_resolved_ids() {
// Absent keys: every committed fixture predates the fields.
let manifest = parse_manifest(&minimal_v12_manifest()).expect("valid v12 manifest");
let ut = manifest.unit_test("unit_test.shop.t1").expect("ingested");
assert!(ut.tested_node_unique_id().is_none());
assert!(ut.this_input_node_unique_id().is_none());

// Explicit nulls: the engine null-fill shape (the cute-dbt#145
// `"overrides": null` precedent) must not fail the parse.
let json = format!(
r#"{{
"metadata": {{ "dbt_schema_version": "{V12_URL}" }},
"unit_tests": {{
"unit_test.shop.t1": {{
"name": "t1",
"model": "stg_orders",
"expect": {{ "rows": [] }},
"tested_node_unique_id": null,
"this_input_node_unique_id": null
}}
}}
}}"#
);
let manifest = parse_manifest(&json).expect("null-filled ids parse");
let ut = manifest.unit_test("unit_test.shop.t1").expect("ingested");
assert!(ut.tested_node_unique_id().is_none());
assert!(ut.this_input_node_unique_id().is_none());
}

// ----- parse_manifest: happy path + translation -----------------

#[test]
Expand Down
9 changes: 5 additions & 4 deletions src/adapters/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ use crate::domain::{
HeuristicId, InScopeSet, Instrument, Manifest, ModelInScopeSet, ModelYamlOutcome, Node, NodeId,
SourceNode, TestMetadata, Tier, UnitTest, UnitTestDataDiff, UnitTestGiven, UnitTestOverrides,
UnitTestYamlBlock, apply_check_policy, model_findings, resolve_target_model,
table_from_manifest_rows,
resolve_tested_model, table_from_manifest_rows,
};

/// Snake-case wire key for an [`EdgeType`] — the exact JSON-serde string
Expand Down Expand Up @@ -2276,8 +2276,9 @@ fn is_leaf_binding_candidate(graph: &CteGraph, index: usize) -> bool {
/// Build a map from in-scope model id to **all** unit tests targeting it
/// in the current manifest (cute-dbt#91 widening).
///
/// Resolved via [`resolve_target_model`] (the bare `model:` name → full
/// node id mapping). Unlike the prior in-scope-only indexer, this
/// Resolved via [`resolve_tested_model`] (the engine-resolved
/// `tested_node_unique_id` when present, the bare `model:` name
/// otherwise — cute-dbt#254). Unlike the prior in-scope-only indexer, this
/// enumerates every unit test whose resolved target is one of
/// `models_in_scope`, regardless of whether the test is itself in scope —
/// so a model that entered scope solely via a changed test also carries
Expand All @@ -2295,7 +2296,7 @@ pub fn index_tests_for_models<'m>(
) -> HashMap<NodeId, Vec<(&'m str, &'m UnitTest)>> {
let mut map: HashMap<NodeId, Vec<(&'m str, &'m UnitTest)>> = HashMap::new();
for (test_id, unit_test) in current.unit_tests() {
let Some(model) = resolve_target_model(current, unit_test.model()) else {
let Some(model) = resolve_tested_model(current, unit_test) else {
continue;
};
if models_in_scope.contains(model.id()) {
Expand Down
7 changes: 3 additions & 4 deletions src/domain/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ use crate::domain::cte::{
};
use crate::domain::grain::test_is_enabled;
use crate::domain::manifest::{Manifest, Node, NodeConfig, NodeId};
use crate::domain::state::resolve_target_model;
use crate::domain::state::{resolve_target_model, resolve_tested_model};
use crate::domain::unit_test::{UnitTest, UnitTestGiven};
use crate::domain::unit_test_table::{CellValue, FixtureTable, table_from_manifest_rows};

Expand Down Expand Up @@ -1148,8 +1148,7 @@ fn detect_union_arm_coverage(ctx: &CheckContext<'_>) -> Vec<Finding<HeuristicId>
.unit_tests()
.iter()
.filter(|(_, ut)| {
resolve_target_model(ctx.manifest, ut.model())
.is_some_and(|model| model.id() == ctx.model.id())
resolve_tested_model(ctx.manifest, ut).is_some_and(|model| model.id() == ctx.model.id())
})
.collect();
tests.sort_by(|a, b| a.0.cmp(b.0));
Expand Down Expand Up @@ -1568,7 +1567,7 @@ fn tests_on_model<'a>(ctx: &'a CheckContext<'_>) -> Vec<(&'a String, &'a UnitTes
.unit_tests()
.iter()
.filter(|(_, unit_test)| {
resolve_target_model(ctx.manifest, unit_test.model())
resolve_tested_model(ctx.manifest, unit_test)
.is_some_and(|model| model.id() == ctx.model.id())
})
.collect();
Expand Down
2 changes: 1 addition & 1 deletion src/domain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pub use scope::{ScopeInput, ScopeSelection, all_models, changed_models, select_i
pub use state::{
BANNER_EMPTY_SCOPE, BodyChecksumModifier, ConfigsModifier, ContractModifier, InScopeSet,
MacrosModifier, ModelInScopeSet, ModifiedSet, ModifierKind, RelationModifier, StateComparator,
StateModifier, resolve_target_model,
StateModifier, resolve_target_model, resolve_tested_model,
};
pub use unit_test::{UnitTest, UnitTestExpect, UnitTestGiven, UnitTestOverrides};
pub use unit_test_table::{
Expand Down
4 changes: 2 additions & 2 deletions src/domain/preflight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
use thiserror::Error;

use crate::domain::manifest::Manifest;
use crate::domain::state::{InScopeSet, ModelInScopeSet, resolve_target_model};
use crate::domain::state::{InScopeSet, ModelInScopeSet, resolve_tested_model};

/// Domain-level fail-closed currency for the two-stage preflight check.
///
Expand Down Expand Up @@ -172,7 +172,7 @@ fn first_in_scope_test_for_model(
let Some(unit_test) = current.unit_test(test_id) else {
continue;
};
if let Some(resolved) = resolve_target_model(current, unit_test.model()) {
if let Some(resolved) = resolve_tested_model(current, unit_test) {
if resolved.id() == model_id {
return Some(unit_test.name().to_owned());
}
Expand Down
8 changes: 4 additions & 4 deletions src/domain/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use std::collections::{BTreeSet, HashMap};
use crate::domain::manifest::{Manifest, NodeId};
use crate::domain::pr_diff::NormalizedDiffIndex;
use crate::domain::state::{
InScopeSet, ModelInScopeSet, ModifierKind, StateComparator, resolve_target_model,
InScopeSet, ModelInScopeSet, ModifierKind, StateComparator, resolve_tested_model,
};

/// Source of the in-scope set: either a baseline manifest (dbt
Expand Down Expand Up @@ -209,7 +209,7 @@ fn select_in_scope_pr_diff(current: &Manifest, index: &NormalizedDiffIndex) -> S
let mut in_scope_ids: Vec<String> = Vec::new();
let mut changed_ids: Vec<String> = Vec::new();
for (test_id, ut) in current.unit_tests() {
let target_path_modified = resolve_target_model(current, ut.model())
let target_path_modified = resolve_tested_model(current, ut)
.is_some_and(|model| path_modified_models.contains(model.id()));
let test_yaml_changed = ut
.original_file_path()
Expand All @@ -232,7 +232,7 @@ fn select_in_scope_pr_diff(current: &Manifest, index: &NormalizedDiffIndex) -> S
let tests_per_model: HashMap<NodeId, usize> = current
.unit_tests()
.values()
.filter_map(|ut| resolve_target_model(current, ut.model()).map(|m| m.id().clone()))
.filter_map(|ut| resolve_tested_model(current, ut).map(|m| m.id().clone()))
.fold(HashMap::new(), |mut acc, id| {
*acc.entry(id).or_insert(0) += 1;
acc
Expand All @@ -241,7 +241,7 @@ fn select_in_scope_pr_diff(current: &Manifest, index: &NormalizedDiffIndex) -> S
let mut model_ids: BTreeSet<NodeId> = BTreeSet::new();
for test_id in in_scope.iter() {
if let Some(ut) = current.unit_test(test_id) {
if let Some(model) = resolve_target_model(current, ut.model()) {
if let Some(model) = resolve_tested_model(current, ut) {
model_ids.insert(model.id().clone());
}
}
Expand Down
Loading
Loading