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

empty unit test csv fixture values default to null #10117

Merged
merged 2 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20240509-091411.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Unit test fixture (csv) returns null for empty value
time: 2024-05-09T09:14:11.772709-04:00
custom:
Author: michelleark
Issue: "9881"
2 changes: 1 addition & 1 deletion core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1822,7 +1822,7 @@ def code(self) -> str:
return "Z026"

def message(self) -> str:
return f" compiled Code at {self.path}"
return f" compiled code at {self.path}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

small formatting issue, unrelated to the behaviour changes in this PR

cc @jtcohen6



class CheckNodeTestFailure(InfoLevel):
Expand Down
10 changes: 8 additions & 2 deletions core/dbt/parser/unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,17 @@
)

if ut_fixture.fixture:
ut_fixture.rows = self.get_fixture_file_rows(
csv_rows = self.get_fixture_file_rows(

Check warning on line 368 in core/dbt/parser/unit_tests.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/unit_tests.py#L368

Added line #L368 was not covered by tests
ut_fixture.fixture, self.project.project_name, unit_test_definition.unique_id
)
else:
ut_fixture.rows = self._convert_csv_to_list_of_dicts(ut_fixture.rows)
csv_rows = self._convert_csv_to_list_of_dicts(ut_fixture.rows)

Check warning on line 372 in core/dbt/parser/unit_tests.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/unit_tests.py#L372

Added line #L372 was not covered by tests

# Empty values (e.g. ,,) in a csv fixture should default to null, not ""
ut_fixture.rows = [

Check warning on line 375 in core/dbt/parser/unit_tests.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/unit_tests.py#L375

Added line #L375 was not covered by tests
{k: (None if v == "" else v) for k, v in row.items()} for row in csv_rows
]

elif ut_fixture.format == UnitTestFormat.SQL:
if not (isinstance(ut_fixture.rows, str) or isinstance(ut_fixture.fixture, str)):
raise ParsingError(
Expand Down
50 changes: 50 additions & 0 deletions tests/functional/unit_testing/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@
'b' as string_b
"""

my_model_check_null_sql = """
SELECT
CASE
WHEN a IS null THEN True
ELSE False
END a_is_null
FROM {{ ref('my_model_a') }}
"""

test_my_model_yml = """
unit_tests:
- name: test_my_model
Expand Down Expand Up @@ -507,6 +516,11 @@
1,a
"""

test_my_model_a_with_null_fixture_csv = """id,a
1,
2,3
"""

test_my_model_a_empty_fixture_csv = """
"""

Expand Down Expand Up @@ -1060,3 +1074,39 @@ def external_package():
rows:
- {id: 2}
"""


test_my_model_csv_null_yml = """
unit_tests:
- name: test_my_model_check_null
model: my_model_check_null
given:
- input: ref('my_model_a')
format: csv
rows: |
id,a
1,
2,3
expect:
format: csv
rows: |
a_is_null
True
False
"""

test_my_model_file_csv_null_yml = """
unit_tests:
- name: test_my_model_check_null
model: my_model_check_null
given:
- input: ref('my_model_a')
format: csv
fixture: test_my_model_a_with_null_fixture
expect:
format: csv
rows: |
a_is_null
True
False
"""
48 changes: 48 additions & 0 deletions tests/functional/unit_testing/test_csv_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,19 @@
datetime_test_invalid_format_key,
my_model_a_sql,
my_model_b_sql,
my_model_check_null_sql,
my_model_sql,
test_my_model_a_empty_fixture_csv,
test_my_model_a_fixture_csv,
test_my_model_a_numeric_fixture_csv,
test_my_model_a_with_null_fixture_csv,
test_my_model_b_fixture_csv,
test_my_model_basic_fixture_csv,
test_my_model_concat_fixture_csv,
test_my_model_csv_null_yml,
test_my_model_csv_yml,
test_my_model_duplicate_csv_yml,
test_my_model_file_csv_null_yml,
test_my_model_file_csv_yml,
test_my_model_fixture_csv,
test_my_model_missing_csv_yml,
Expand Down Expand Up @@ -208,6 +212,50 @@ def test_unit_test(self, project):
results = run_dbt(["test", "--select", "my_model"], expect_pass=False)


class TestUnitTestsInlineCSVEmptyValueIsNull:
@pytest.fixture(scope="class")
def models(self):
return {
"my_model_a.sql": my_model_a_sql,
"my_model_check_null.sql": my_model_check_null_sql,
"test_my_model_csv_null.yml": test_my_model_csv_null_yml,
}

def test_unit_test(self, project):
results = run_dbt(["run"])
assert len(results) == 2

# Select by model name
results = run_dbt(["test", "--select", "my_model_check_null"], expect_pass=True)
assert len(results) == 1


class TestUnitTestsFileCSVEmptyValueIsNull:
@pytest.fixture(scope="class")
def models(self):
return {
"my_model_a.sql": my_model_a_sql,
"my_model_check_null.sql": my_model_check_null_sql,
"test_my_model_file_csv_null.yml": test_my_model_file_csv_null_yml,
}

@pytest.fixture(scope="class")
def tests(self):
return {
"fixtures": {
"test_my_model_a_with_null_fixture.csv": test_my_model_a_with_null_fixture_csv,
}
}

def test_unit_test(self, project):
results = run_dbt(["run"])
assert len(results) == 2

# Select by model name
results = run_dbt(["test", "--select", "my_model_check_null"], expect_pass=True)
assert len(results) == 1


class TestUnitTestsMissingCSVFile:
@pytest.fixture(scope="class")
def models(self):
Expand Down
Loading