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

simplify shimming #27

Merged
merged 3 commits into from
May 30, 2021
Merged

Conversation

dataders
Copy link
Contributor

@dataders dataders commented May 30, 2021

high-level summary

  • refactored get_test_dates() in order to:
    • more surgically change behavior, and
    • keep shimmed macros more in line w/ dbt-date's integration tests as they update.
  • deleted dbt_date_integration_tests's _get_utils_namespaces() to align with dbt-utils's strategy of joining integration test macros to the dispatch list. (note: this will certainly change with v0.20.0, but at least it's a step in the right direction.

detail

resolves: #25
given that the only difference in get_test_dates() is the week_of_year column, this will make it easier to shim.

once this merges, making get_test_dates() working for TSQL will be as simple as adding this macro to the tsql-utils dbt-date integration tests macros folder (see: dbt-msft/tsql-utils#48)

{% macro sqlserver__get_test_week_of_year() -%}
    {{ log("in the right macro!", info=True) }}
    {# who knows what T-SQL uses?! #}
    {# see: https://github.com/calogica/dbt-date/issues/25 #}
    {{ return([49,49]) }}
{%- endmacro %}

{% macro synapse__get_test_week_of_year() -%}
     {{ return(sqlserver__get_test_week_of_year()) }}
{%- endmacro %}

@dataders dataders marked this pull request as ready for review May 30, 2021 17:50
Copy link
Contributor

@clausherther clausherther left a comment

Choose a reason for hiding this comment

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

👍

integration_tests/macros/get_test_dates.sql Outdated Show resolved Hide resolved
cast('2020-12-05' as date) as week_end_date,
-- Snowflake returns ISO week numbers with the standard config
49 as week_of_year,
{% macro get_test_week_of_year() -%}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this works for now, although this seems a bit harder to follow for anyone new than simply having a separate test dataset for each platform.

@clausherther clausherther merged commit 8de32b0 into calogica:main May 30, 2021
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.

what week of year is December 1, 2020?
2 participants