-
Notifications
You must be signed in to change notification settings - Fork 1.8k
patch for bigquery unittest with sql_header #11386
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
base: main
Are you sure you want to change the base?
Conversation
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @takemikami |
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11386 +/- ##
==========================================
- Coverage 88.90% 88.82% -0.08%
==========================================
Files 191 191
Lines 24391 24394 +3
==========================================
- Hits 21684 21669 -15
- Misses 2707 2725 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
b043074
to
478cd62
Compare
@@ -237,6 +237,12 @@ def execute_unit_test( | |||
"Invalid materialization context generated, missing config: {}".format(context) | |||
) | |||
|
|||
# allows test code to access the configuration settings of the component it's testing | |||
if unit_test_node.tested_node_unique_id: |
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.
@takemikami These commits trace back to a blog post you made. Could you explain the use case behind these lines? After some thoughtful experimentation, I don't see how they are used or what user feature they are enabling. I was able to use the other code in this PR to enable injection of a sql header inside and will have a PR for this soon. However, could you answer what these lines are for? (I tried a few obvious things like {{ tested_node_config }} but this leads to a compiler use before instantiation problem)
Resolves #9775
Based of @takemikami post on fixing SQL header integration in unit tests for BigQuery.
Problem
Solution
Checklist