-
Notifications
You must be signed in to change notification settings - Fork 20
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/unit testing phase1 #101
Conversation
I will..squash before merge 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fivetran-sheringuyen this is great and will really help us unsure that changes to this package have more coverage than just releasing and hoping a potential bug isn't introduced!
I have one small comment, but other than that - this looks great to go! I do want to hold off merging and releasing this until PR #100 is completed.
version: [">=0.6.0", "<0.7.0"] | ||
- package: fivetran/zendesk | ||
version: [">=0.10.0", "<0.11.0"] | ||
- package: fivetran/hubspot | ||
version: [">=0.8.0", "<0.9.0"] | ||
- package: fivetran/jira | ||
version: [">=0.11.0", "<0.12.0"] | ||
- package: fivetran/klaviyo | ||
version: [">=0.5.0", "<0.6.0"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we maybe make these ranges larger? I would want to make sure we are testing on the latest versions of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a couple of different things we could do here.
- Broaden the range as you are suggesting and having the upper bound be < 1.0.0
or - Having
revision: main
for the git reference. This ensures that it's always the latest version of main, although not necessarily a release. So there is a minor caveat here but for this context, I think it would be ok.
What change does this PR introduce?
Adds a dbt compile test to our Fivetran Utils package
If this PR introduces a new macro, how did you test the new macro?
If this PR introduces a modification to an existing macro, which packages is the macro currently present in and what steps were taken to test compatibility across packages?
Did you update the README to reflect the macro addition/modifications?
Integration testing for internal uses