fix: unit tests with versioned refs #10889
Conversation
missing latest prop
|
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. |
1 similar comment
|
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10889 +/- ##
==========================================
- Coverage 89.18% 79.27% -9.91%
==========================================
Files 183 183
Lines 23429 23448 +19
==========================================
- Hits 20895 18589 -2306
- Misses 2534 4859 +2325
Flags with carried forward coverage won't be shown. Click here to find out more.
|
deleted before due a mistake
|
Hello @dbeatty10. This is a PR with a very simple fix that closes 3 open issues. It's covered by functional tests, but I’m not sure why this class doesn’t have any unit tests, so I left that aside. |
graciegoheen
added
the
ready_for_review
| ): | ||
|
|
||
| if not resource_type == NodeType.Model: | ||
| continue |
There was a problem hiding this comment.
This is a little tricky to read given we're already in a resource_type in (NodeType.Model,NodeType.Seed, NodeType.Snapshot) block.
I'd suggest refactoring to be more straightforward to read, something like:
if resource_type == NodeType.model:
input_node = ModelNode(
**common_fields,
defer_relation=original_input_node.defer_relation,
latest_version=original_input_node.latest_version,
version=original_input_node.version
)
elif resource_type in (NodeType.Seed, NodeType.Snapshot):
...
elif resource_type == NodeType.Source:
...
| @@ -147,23 +145,27 @@ def parse_unit_test_case(self, test_case: UnitTestDefinition): | |||
| "name": input_name, | |||
| "path": f"{input_name}.sql", | |||
| } | |||
| resource_type = original_input_node.resource_type | |||
There was a problem hiding this comment.
nice refactor! thank you
| @@ -309,6 +309,141 @@ def test_basic(self, project): | |||
| assert len(results) == 2 | |||
|
|
|||
|
|
|||
| schema_ref_with_version = """ | |||
| models: | |||
| - name: source | |||
There was a problem hiding this comment.
for clarity, i'd rename this to versioned_model
| def test_basic(self, project): | ||
| results = run_dbt(["run"]) | ||
| assert len(results) == 3 | ||
| # TODO: How to capture an compilation Error? pytest.raises(CompilationError) not working |
There was a problem hiding this comment.
Yeah, this can be tricky to do in certain contexts depending on where the CompilationError is thrown internally. If pytest.raises didn't capture it, expect_pass=False is fine.
However, you could change run_dbt to run_dbt_and_capture and assert the CompilationError message was found in the stdout logs.
|
Thank you for the contribution @devmessias! This is looking good overall, but it looks like this PR is failing on unrelated functional tests with the same 'strategy' error for tests related to snapshots. We've been doing a lot of work on snapshots recently so it may just be an intermittent issue that has since been resolved. Could you please merge Otherwise, just a refactoring suggestion for clarity, as well as some examples of testing for expected errors in logs. |
Resolves #10880; Resolves #10528; Resolves #10623
Problem
The error raises due the method
RefableLookup.findat dbt-my/core/dbt/contracts/graph/manifest.py:Tracking the storage object, without specifying a version in the model we will obtain something like that for the Unit Test Node running phase
{'upstream_model_versioned': {}, 'upstream_model_versioned.v<LATEST>': {'demo_dbt_py': 'model.demo_dbt_py.upstream_model_versioned'}}Which it seems to me that is not right, because if I specified the
latest_versionfor my model, I should get a Storage like this{ 'upstream_model_versioned': {'demo_dbt_py': 'model.demo_dbt_py.upstream_model_versioned'}, 'upstream_model_versioned.v <LATEST> ': {'demo_dbt_py': 'model.demo_dbt_py.upstream_model_versioned'} }I run some different cases and obtain the following table
However, even in cases where the tests were successful, the storage objects it seems that not properly constructed. For example, when using
ref('upstream_model_versioned', v=1)I got this:{'upstream_model_versioned': {}, 'upstream_model_versioned.v1': {'demo_dbt_py': 'model.demo_dbt_py.upstream_model_versioned'}}This worked not because the storage structure was correct, but because there was a key named
upstream_model_versioned.v1.Solution
The only problem that I found is that the Node object for the UnitTest was not copying the value from latest_version prop if it was available
Checklist