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

add basic column tests for arrays and update readme #273

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ilkernator
Copy link

Issue this PR Addresses/Closes

#272

Summary of Changes

Added two new tests to the "column_values_basic" directory and updated the readme-file:

  • expect_array_column_to_not_be_empty
  • expect_struct_column_to_not_be_empty

Why Do We Need These Changes

For tables that contain columns of type array or array(struct), the values can be sometimes empty without being NULL (e.g. [ ], or [" "]. In this case, the test to expect_column_values_to_not_be_null.sql could be misleading and mask the issue of having empty values. So we would need to make use of Hive functions to assert the array is indeed empty.

Reviewers

@clausherther

@clausherther
Copy link
Contributor

Hi, thanks for this PR.
Before I can take a look at this, we'll need to add integration tests for both macros, which requires adding the appropriately typed columns to the test datasets.
Also, I think I'd prefer implementing this more generically as a single test, ~ expect_column_to_not_be_empty. Would that be possible across all platforms we support?
Also, we'd need the positive equivalent, expect_array_column_to_be_empty.

@clausherther clausherther added the needs_tests Needs integration tests label Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs_tests Needs integration tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants