Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CT-71] Restore previous behavior when a test depends on a disabled (vs. missing) resource #4594

Closed
jtcohen6 opened this issue Jan 19, 2022 · 2 comments · Fixed by #4647
Closed
Labels
dbt tests Issues related to built-in dbt testing functionality
Milestone

Comments

@jtcohen6
Copy link
Contributor

Prompted by Slack threads here + here

Folks who've newly upgraded to v1.0 are seeing a lot of warn-level (stdout) messages about tests defined on disabled models. This is what our internal analytics project looks like, and I don't think it's an uncommon experience:

Screenshot 2022-01-19 at 19 16 48

(There's many more below that)

It is desirable to warn about tests that have "missed" the correct model, such as due to a typo in a ref or yaml property (e.g. see #4532). It's also a very common pattern to disable unused/irrelevant models in packages, and the result is a quite cluttered terminal. There are some useful warnings in there (missing node for patch), lost as needles in haystack-traces.

What I proposed in one of the threads linked above:

If a test depends on a disabled model, log this as DEBUG-level (shows up in logs/dbt.log)
If a test depends on a missing model, log this as WARN-level (shows up in standard CLI output)

I looked quickly at the code, and turns out this was actually our previous behavior:

def invalid_ref_fail_unless_test(node, target_model_name,
target_model_package, disabled):
if node.resource_type == NodeType.Test:
msg = get_target_not_found_or_disabled_msg(
node, target_model_name, target_model_package, disabled
)
if disabled:
logger.debug(warning_tag(msg))
else:
warn_or_error(
msg,
log_fmt=warning_tag('{}')
)

We've since replaced the first of those with a call to the InvalidRefInTestNode event, so this would just look like changing:

@dataclass
class InvalidRefInTestNode(WarnLevel):
msg: str
code: str = "I051"
def message(self) -> str:
return ui.warning_tag(self.msg)

To:

@dataclass
class InvalidRefInTestNode(DebugLevel):
    msg: str
    code: str = "I051"

    def message(self) -> str:
        return self.msg

Voila, just the good stuff:

Screenshot 2022-01-19 at 19 19 35

@jtcohen6 jtcohen6 added dbt tests Issues related to built-in dbt testing functionality Team:Language labels Jan 19, 2022
@jtcohen6 jtcohen6 added this to the v1.0.2 milestone Jan 19, 2022
@github-actions github-actions bot changed the title Restore previous behavior when a test depends on a disabled (vs. missing) resource [CT-71] Restore previous behavior when a test depends on a disabled (vs. missing) resource Jan 19, 2022
@fivetran-sheringuyen
Copy link

I think this is a great update -- happy to be a tester if/when you need!

@leahwicz
Copy link
Contributor

leahwicz commented Feb 1, 2022

Backport here: #4656

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dbt tests Issues related to built-in dbt testing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants