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

Adap 821/unit tests infer wrong datatype for None values in fixtures #885

Merged
merged 11 commits into from
Jul 23, 2024
7 changes: 7 additions & 0 deletions .changes/unreleased/Fixes-20240625-170324.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Fixes
body: 'Handle unit test fixtures where typing goes wrong from first value in column
being Null. '
time: 2024-06-25T17:03:24.73937-07:00
custom:
Author: versusfacit
Issue: "821"
11 changes: 11 additions & 0 deletions dbt/include/redshift/macros/adapters/unit_testing.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{%- macro redshift__validate_fixture_rows(rows, row_number) -%}
{%- if rows is not none and rows|length > 0 -%}
{%- set row = rows[0] -%}
{%- for key, value in row.items() -%}
{%- if value is none -%}
{%- set fixture_name = "expected output" if model.resource_type == 'unit_test' else ("'" ~ model.name ~ "'") -%}
{{ exceptions.raise_compiler_error("Unit test fixture " ~ fixture_name ~ " in " ~ model.name ~ " does not have any row free of null values, which may cause type mismatch errors during unit test execution.") }}
mikealfare marked this conversation as resolved.
Show resolved Hide resolved
{%- endif -%}
{%- endfor -%}
{%- endif -%}
{%- endmacro -%}
2 changes: 1 addition & 1 deletion dev-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# install latest changes in dbt-core + dbt-postgres
git+https://github.com/dbt-labs/dbt-adapters.git
git+https://github.com/dbt-labs/dbt-adapters.git@ADAP-821/support_for_redshift_821
Copy link
Contributor

Choose a reason for hiding this comment

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

Friendly reminder to revert prior to merging.

git+https://github.com/dbt-labs/dbt-adapters.git#subdirectory=dbt-tests-adapter
git+https://github.com/dbt-labs/dbt-common.git
git+https://github.com/dbt-labs/dbt-core.git#subdirectory=core
Expand Down
73 changes: 73 additions & 0 deletions tests/functional/adapter/unit_testing/fixtures.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
model_none_value_base = """
{{ config(materialized="table") }}

select 1 as id, 'a' as col1
"""

model_none_value_model = """
{{config(materialized="table")}}

select * from {{ ref('none_value_base') }}
"""


test_none_column_value_doesnt_throw_error_csv = """
unit_tests:
- name: test_simple

model: none_value_model
given:
- input: ref('none_value_base')
format: csv
rows: |
id,col1
,d
,e
6,f

expect:
format: csv
rows: |
id,col1
,d
,e
6,f
"""

test_none_column_value_doesnt_throw_error_dct = """
unit_tests:
- name: test_simple

model: none_value_model
given:
- input: ref('none_value_base')
rows:
- { "id": , "col1": "d"}
- { "id": , "col1": "e"}
- { "id": 6, "col1": "f"}

expect:
rows:
- {id: , "col1": "d"}
- {id: , "col1": "e"}
- {id: 6, "col1": "f"}
"""

test_none_column_value_will_throw_error = """
unit_tests:
- name: test_simple

model: none_value_model
given:
- input: ref('none_value_base')
rows:
- { "id": , "col1": "d"}
- { "id": , "col1": "e"}
- { "id": 6, "col1": }

expect:
rows:
- {id: , "col1": "d"}
- {id: , "col1": "e"}
- {id: 6, "col1": }
"""
64 changes: 64 additions & 0 deletions tests/functional/adapter/unit_testing/test_unit_testing.py
mikealfare marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,7 +1,21 @@
import pytest

from dbt.artifacts.schemas.results import RunStatus
from dbt.tests.fixtures.project import write_project_files
from dbt.tests.util import run_dbt

from dbt.tests.adapter.unit_testing.test_types import BaseUnitTestingTypes
from dbt.tests.adapter.unit_testing.test_case_insensitivity import BaseUnitTestCaseInsensivity
from dbt.tests.adapter.unit_testing.test_invalid_input import BaseUnitTestInvalidInput
from tests.functional.adapter.unit_testing.fixtures import (
model_none_value_base,
model_none_value_model,
test_none_column_value_doesnt_throw_error_csv,
test_none_column_value_doesnt_throw_error_dct,
test_none_column_value_will_throw_error,
)

from dbt_common.exceptions import CompilationError


class TestRedshiftUnitTestingTypes(BaseUnitTestingTypes):
Expand Down Expand Up @@ -34,6 +48,56 @@ def data_types(self):
]


class RedshiftUnitTestingNone:
def test_nones_handled_dict(self, project):
run_dbt(["build"])


class TestRedshiftUnitTestCsvNone(RedshiftUnitTestingNone):
@pytest.fixture(scope="class")
def models(self):
return {
"none_value_base.sql": model_none_value_base,
"none_value_model.sql": model_none_value_model,
"__properties.yml": test_none_column_value_doesnt_throw_error_csv,
}


class TestRedshiftUnitTestDictNone(RedshiftUnitTestingNone):
@pytest.fixture(scope="class")
def models(self):
return {
"none_value_base.sql": model_none_value_base,
"none_value_model.sql": model_none_value_model,
"__properties.yml": test_none_column_value_doesnt_throw_error_dct,
}


class TestRedshiftUnitTestingTooManyNonesFails:
@pytest.fixture(scope="class")
def models(self):
return {
"__properties.yml": test_none_column_value_will_throw_error,
"none_value_base.sql": model_none_value_base,
"none_value_model.sql": model_none_value_model,
}

def test_invalid_input(self, project):
"""This is a user-facing exception, so we can't pytest.raise(CompilationError)"""

def _find_first_error(items):
return next((item for item in items if item.status == RunStatus.Error), None)

run_result = run_dbt(["build"], expect_pass=False)
first_item = _find_first_error(run_result)

assert first_item is not None
assert (
"does not have any row free of null values, which may cause type mismatch errors during unit test execution"
in str(first_item.message)
)


class TestRedshiftUnitTestCaseInsensitivity(BaseUnitTestCaseInsensivity):
pass

Expand Down
Loading