-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[ALT] Ensure unit test models can depend on external nodes #9367
Closed
QMalcolm
wants to merge
6
commits into
main
from
qmalcolm--8944-unit-test-models-dependent-on-external-nodes-alt
Closed
[ALT] Ensure unit test models can depend on external nodes #9367
QMalcolm
wants to merge
6
commits into
main
from
qmalcolm--8944-unit-test-models-dependent-on-external-nodes-alt
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9367 +/- ##
=======================================
Coverage 86.93% 86.94%
=======================================
Files 187 187
Lines 24954 24954
=======================================
+ Hits 21695 21696 +1
+ Misses 3259 3258 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
QMalcolm
force-pushed
the
qmalcolm--8944-unit-test-models-dependent-on-external-nodes-alt
branch
2 times, most recently
from
January 16, 2024 18:33
1935d03
to
55611b1
Compare
… postgres max Was getting test failures due to resulting lengthy model names being created by unit test task in the functional test
…package of unit test This is necessary because when we create the shadow manifest for the running the unit test node, the included models whether direct or external, get rewritten as being in the unit test's package.
…external node point to it Previously I was getting an error about the columns for the external model not being fetchable from the database via the macro `get_columns_in_relation`. By creating a seed for the tests, which creates a table in postgres, we can then tell the external model that it's database schema and identifier (the relation) is that table from the seed without make the seed an actual dependency of the external model in the dag.
External nodes generally don't have paths, but in unit tests we write out all models to sql files (as this allows us to test them). Thus external nodes need to have their paths set.
This is necessary because when executing a unit tests, the columns associated with a model in the database are retrieved. For this to be possible, the model must exist in the database, thus we must run the associated models at least once first.
QMalcolm
force-pushed
the
qmalcolm--8944-unit-test-models-dependent-on-external-nodes-alt
branch
from
January 18, 2024 18:35
55611b1
to
020a9ec
Compare
QMalcolm
changed the title
Qmalcolm 8944 unit test models dependent on external nodes alt
[ALT] Ensure unit test models can depend on external nodes
Jan 18, 2024
Closed in favor of #9343 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #8944
Problem
Solution
Checklist