Skip to content

Commit 8040a76

Browse files
MikaKermanclaudedevin-ai-integration[bot]haritamar
authored
Fix test result sample count differing from failure count (#933)
* Fix test result sample count exceeding failure count for non-deterministic queries Truncate result_rows to match dbt's failure count when re-execution of non-deterministic queries (e.g. TABLESAMPLE) returns more rows than the original run. Also add a description note when sample_percent is used. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove test_not_null_sampled macro and Change 2 tests, validate sample_percent value Remove the custom generic test macro and associated tests that were hard to exercise through the integration test framework. Also tighten the sample_percent check to validate it's a number between 0 and 100. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix leading whitespace in sampling description note Avoid prepending a space when test_results_description is empty/None. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Itamar Hartstein <haritamar@gmail.com>
1 parent 534afc6 commit 8040a76

File tree

3 files changed

+64
-1
lines changed

3 files changed

+64
-1
lines changed
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import json
2+
3+
import pytest
4+
from dbt_project import DbtProject
5+
6+
COLUMN_NAME = "some_column"
7+
8+
9+
@pytest.mark.skip_targets(["clickhouse"])
10+
def test_result_rows_do_not_exceed_failures(test_id: str, dbt_project: DbtProject):
11+
"""Result rows count should never exceed the dbt failure count."""
12+
null_count = 20
13+
data = [{COLUMN_NAME: None} for _ in range(null_count)]
14+
test_result = dbt_project.test(
15+
test_id,
16+
"not_null",
17+
dict(column_name=COLUMN_NAME),
18+
data=data,
19+
test_vars={
20+
"enable_elementary_test_materialization": True,
21+
"test_sample_row_count": 1000,
22+
},
23+
)
24+
assert test_result["status"] == "fail"
25+
26+
failures = int(test_result["failures"])
27+
assert failures == null_count
28+
29+
samples = [
30+
json.loads(row["result_row"])
31+
for row in dbt_project.run_query(dbt_project.samples_query(test_id))
32+
]
33+
assert len(samples) <= failures

macros/edr/materializations/test/test.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,16 @@
8484
{% set result_rows = elementary.query_test_result_rows(
8585
sample_limit=sample_limit, ignore_passed_tests=true
8686
) %}
87+
88+
{# Truncate result rows if they exceed dbt's failure count (can happen with non-deterministic queries) #}
89+
{% if result_rows | length > 0 %}
90+
{% set test_result = elementary.get_test_result() %}
91+
{% set dbt_failures = test_result.failures | int %}
92+
{% if dbt_failures > 0 and result_rows | length > dbt_failures %}
93+
{% set result_rows = result_rows[:dbt_failures] %}
94+
{% endif %}
95+
{% endif %}
96+
8797
{% set elementary_test_results_row = elementary.get_dbt_test_result_row(
8898
flattened_test, result_rows
8999
) %}

macros/edr/tests/on_run_end/handle_tests_results.sql

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,32 @@
123123
{% do elementary_test_results_row.setdefault(
124124
"test_results_description", result.message
125125
) %}
126+
{# Add note when test uses sampling #}
127+
{% set test_params = elementary_test_results_row.get(
128+
"test_params", {}
129+
) %}
130+
{% if test_params is mapping and test_params.get(
131+
"sample_percent"
132+
) is number and test_params.get(
133+
"sample_percent"
134+
) > 0 and test_params.get(
135+
"sample_percent"
136+
) < 100 %}
137+
{% set base_desc = elementary_test_results_row.get(
138+
"test_results_description"
139+
) %}
140+
{% set note = "Note: this test uses sample_percent, so result samples may not exactly match the failure count." %}
141+
{% set new_desc = (base_desc ~ " " ~ note) if base_desc else note %}
142+
{% do elementary_test_results_row.update(
143+
{"test_results_description": new_desc}
144+
) %}
145+
{% endif %}
126146
{% if render_result_rows %}
127147
{% do elementary_test_results_row.update(
128148
{
129149
"result_rows": elementary.render_result_rows(
130150
elementary_test_results_row.result_rows
131-
)
151+
),
132152
}
133153
) %}
134154
{% endif %}

0 commit comments

Comments
 (0)