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

Feature support multiple test cases #54

Conversation

marcellovictorino
Copy link

This is a:

  • bug fix PR with no breaking changes
  • new functionality
  • a breaking change

Description & motivation

Addressing this issue, enables to define a test once, to be applied over multiple test cases.

Applicable when there are multiple test case scenarios, where the mocked input and expected output data (seed) represent a single case, having the seed file names identified by test_case_1, test_case_2 etc.

Checklist

  • I have verified that these changes work locally on the following warehouses (Note: it's okay if you do not have access to all warehouses, this helps us understand what has been covered)
    • BigQuery
    • Postgres
    • Redshift
    • Snowflake
  • I have updated the README.md (if applicable)
  • I have added tests & descriptions to my macros (and models if applicable) -> added descriptions. Not sure how to add tests

Additional comments

  1. I am not sure what is the best way to adapt the proposed code change to the new dispatch pattern. I developed these changes against an old package version (v 0.1.2). I marked it with the comment {% FIXME: ...

  2. Also, I have changed the get_test macro behavior to always store the mocked model output as a view, which provides better visibility on failing tests. As a view, it is possible to inspect its definition, quickly debugging any issues

Note: this is my first time contributing to an open-source project 🎉! Please let me know if there is anything else I could/should do 👍

@marcellovictorino
Copy link
Author

Forgot to tag you here for visibility:
@mjirv

README.md Outdated Show resolved Hide resolved
@marcellovictorino marcellovictorino force-pushed the feature-support-multiple-test-cases branch from 279ab09 to bbe6765 Compare October 3, 2022 10:04
Comment on lines +36 to +41
{# Note: possible to hardcode desired database.schema to store the mocked model output #}
{% set mock_model_relation = api.Relation.create(
database=model.database,
schema=model.schema,
identifier=identifier_name,
type='view') %}
Copy link
Author

Choose a reason for hiding this comment

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

This model.database/schema will retrieve the value from the model where the test is being applied.
Alternatively, I wanted to use the default value as defined in the dbt_project.yaml, for the seeds. Any idea how to do this?

In my use case, I was able to hard-code the value, but would be nice to have a more elegant solution

Copy link
Owner

@mjirv mjirv left a comment

Choose a reason for hiding this comment

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

Hey @marcellovictorino. Thanks again for submitting this. Great work, and it will be an awesome feature to add.

I left a few comments in the code. Curious to get your thoughts.

Also, I'm finding it a little hard to review both the "multiple test case" feature and the "save output for inspection" feature all at once. Could you split them into 2 separate PRs?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
macros/dmt_get_test_sql.sql Outdated Show resolved Hide resolved
macros/dmt_unit_test.sql Outdated Show resolved Hide resolved
macros/dmt_unit_test.sql Outdated Show resolved Hide resolved
macros/dmt_unit_test.sql Show resolved Hide resolved
@marcellovictorino
Copy link
Author

Thanks for the review @mjirv . I really appreciate it :)
Been a busy week, but I will perform the required changes before the end of the week (2022-10-14)

@marcellovictorino
Copy link
Author

Closing this in favor of splitting the different features into separate PRs: #56

@marcellovictorino marcellovictorino deleted the feature-support-multiple-test-cases branch October 14, 2022 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants