Skip to content

fix: raise correct deprecation when config key is top-level in generic test (#12572)#12618

Open
kalluripradeep wants to merge 1 commit intodbt-labs:mainfrom
kalluripradeep:fix-missing-args-deprecation-misleading-12572
Open

fix: raise correct deprecation when config key is top-level in generic test (#12572)#12618
kalluripradeep wants to merge 1 commit intodbt-labs:mainfrom
kalluripradeep:fix-missing-args-deprecation-misleading-12572

Conversation

@kalluripradeep
Copy link
Contributor

Problem

Fixes #12572
Parent: #12394

When a user defines a config property like where at the top level of a generic test:

data_tests:
  - unique:
      where: "valid_to is null"

dbt was raising MissingArgumentsPropertyInGenericTestDeprecation, which misleads the developer into moving where under arguments: — which is wrong. The correct fix is to move it under config:.

Root Cause

In TestBuilder.extract_test_args(), the check for missing arguments property did not distinguish between actual test arguments and known config properties (CONFIG_ARGS like where, severity, tags, etc.). Any top-level key that wasn't config, column_name, description, or name triggered the wrong deprecation.

Fix

  1. Exclude CONFIG_ARGS from the missing-arguments check in extract_test_args() so that known config properties don't trigger MissingArgumentsPropertyInGenericTestDeprecation

  2. Emit PropertyMovedToConfigDeprecation for each CONFIG_ARGS key found at the root level of the test definition, guiding the developer to move it under config:

Test

Added TestMissingArgsVsPropertyMovedToConfig in tests/functional/deprecations/test_deprecations.py which verifies:

  • PropertyMovedToConfigDeprecation fires for where at top level
  • MissingArgumentsPropertyInGenericTestDeprecation does NOT fire

cc @dbeatty10

@kalluripradeep kalluripradeep requested a review from a team as a code owner March 8, 2026 16:32
@cla-bot cla-bot bot added the cla:yes label Mar 8, 2026
@github-actions github-actions bot added the community This PR is from a community member label Mar 8, 2026
@graciegoheen graciegoheen added the ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering label Mar 9, 2026
Copy link
Contributor

@ash2shukla ash2shukla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this work !
Please address following comments and we can merge it !

config_keys_at_root = [
k for k in test_args.keys()
if k not in ("config", "column_name", "description", "name")
and k in TestBuilder.CONFIG_ARGS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the checks against CONFIG_ARGS here is redundant as CONFIG_ARGS and ("config", "column_name", "description", "name") are mutually exclusive.
Probably reword this block like following -

for k in test_args.keys():
    if k in TestBuilder.CONFIG_ARGS:
        deprecations.warn(
            "property-moved-to-config-deprecation",
            key=k,
            file=file_path,
            key_path=f"data_tests.{test_name}.{k}",
        )

top_level_keys = ("config", "column_name", "description", "name", *TestBuilder.CONFIG_ARGS)

if not arguments and any(k not in top_level_keys for k in test_args.keys()):
    resource = (
        f"'{resource_name}' in package '{package_name}'"
        if package_name
        else f"'{resource_name}'"
    )
    deprecations.warn(
        "missing-arguments-property-in-generic-test-deprecation",
        test_name=f"`{test_name}` defined on {resource} ({file_path})",
    )

PropertyMovedToConfigDeprecation, not MissingArgumentsPropertyInGenericTestDeprecation."""

@pytest.fixture(scope="class")
def dbt_profile_target(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We dont need to patch profile target for this test.

["parse", "--no-partial-parse", "--show-all-deprecations"],
callbacks=[moved_catcher.catch, missing_catcher.catch],
)
assert len(moved_catcher.caught_events) >= 1, (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be exactly one event in this case, check for equality please == 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla:yes community This PR is from a community member ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] MissingArgumentsPropertyInGenericTestDeprecation misleading on properties.

3 participants