dbt-labs / dbt-project-evaluator

This package contains macros and models to find DAG issues automatically
https://dbt-labs.github.io/dbt-project-evaluator/latest/
Apache License 2.0
453 stars 69 forks source link

Built-in unique test to not be explicitly added to "primary_key_test_macros" #497

Open PatFitzner opened 2 months ago

PatFitzner commented 2 months ago

In order to avoid name collisions on unique test definition overrides, the line that explicitly adds the builtin dbt.test_unique test to the "primary_key_test_macros" list has been replaced with a conditional block that will only add it if none of the tests already present in the "primary_key_test_macros" list would produce a is_test_unique column in the int_all_graph_resources model, which is required downstream by int_model_test_summary.

This is a:

Link to Issue

Closes 496

Description & motivation

Line 10 of models/marts/core/int_all_graph_resources.sql

{%- do test_macro_list.append("dbt.test_unique") -%}

Is supposed to ensure that the column is_test_unique is generated, but breaks the execution of the package when the default test dbt.test_unique has been overridden in a project.

Checklist

PatFitzner commented 2 months ago

I feel like CircleCI is out to get me. I've pushed the same commit 3 or 4 times with amended comments to trigger a new CI execution, and the failures are not always the same, as well as unrelated to the changes of this PR (non-exsiting tables, failed freshness / non-documented model tests ...) Could you please let me know what I may be missing here @graciegoheen @dave-connors-3 @b-per ?

Thanks <3

b-per commented 2 months ago

Thanks @PatFitzner Yes, CI is a bit flaky with some adapters unfortunately but we will have a look.

I think that the original piece of code on test_unique was added because we allow people to say that a model is PK-tested if it has a unique test and has a not null constraint on it.

I think that your approach is correct but I will need to do a few more checks.