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

feat: Support multiple test cases #56

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

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

Splitting the original PR into two separate ones: this one focus on enabling multiple test cases within a single test definition

@marcellovictorino
Copy link
Author

marcellovictorino commented Oct 14, 2022

@mjirv as requested, splitting the original into separate PRs
I will hold on the second PR, saving the mocked model output as view for improved error visibility, for later 👍
I will even create an issue for it

@marcellovictorino
Copy link
Author

@mjirv bumping for visibility
Thanks!

@mjirv
Copy link
Owner

mjirv commented Oct 25, 2022

@marcellovictorino thanks for bumping! I had missed that you updated this. Reviewing now!

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.

I left one comment but overall this looks awesome!

The only other thing left is to add at least one integration test (https://github.com/mjirv/dbt-datamocktool/blob/main/integration_tests/models/staging/schema.yml). The steps would be:

  1. Copy/paste your example from the README into that schema.yml file (you can just use 1,2 as the test_cases)
  2. Add a seed/expectation for ref('dmt__raw_customers_2') and ref('dmt__expected_stg_customers_2')

Would you like to add the integration test or do you want me to?

{% set individual_expected_output = adapter.get_relation(*full_path_list) %}

{# Retrieve the SQL code with the input mapping applied, using mocked input #}
{% set individual_test_sql = dbt_datamocktool.get_unit_test_sql(model, individual_input_mapping, depends_on, test_case) %}
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
{% set individual_test_sql = dbt_datamocktool.get_unit_test_sql(model, individual_input_mapping, depends_on, test_case) %}
{% set individual_test_sql = dbt_datamocktool.get_unit_test_sql(model, individual_input_mapping, depends_on) %}

I think you need to get rid of test_case here, right? Or am I missing what it's supposed to be doing?

I get an error when running:

00:39:26  Encountered an error:
Compilation Error in test dbt_datamocktool_unit_test_stg_customers__ref_dmt__expected_stg_customers_____ref_dmt__raw_customers____1__2 (models/staging/schema.yml)
  macro 'dbt_macro__get_unit_test_sql' takes not more than 3 argument(s)
  
  > in macro test_unit_test (macros/dmt_unit_test.sql)
  > called by macro get_unit_test_sql (macros/dmt_get_test_sql.sql)
  > called by test dbt_datamocktool_unit_test_stg_customers__ref_dmt__expected_stg_customers_____ref_dmt__raw_customers____1__2 (models/staging/schema.yml)

@marcellovictorino
Copy link
Author

@mjirv Thanks for the new review!
It has been a busy week. But I am interested in applying the fixes and trying to create the end-to-end test. Might take a bit longer, but would be a learning opportunity for me, and hopefully easier for future contributions 😅 🎉
I will aim to complete this before the end of this week (4th November)

@mjirv
Copy link
Owner

mjirv commented Nov 1, 2022

@marcellovictorino awesome, let me know if you need anything!

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