Merged
Conversation
Previously in `test_unsupported_version_extra_config` and `test_archive_not_allowed` we were checking for `DbtProjectError`. This worked because `ProjectContractError` is a subclass of `DbtProjectError`. However, if we check for `DbtProjectError` in these tests than, some tangential failure which raises a `DbtProejctError` type error would go undetected. As we plan on modifying these tests to be pytest in the coming commits, we want to ensure that the tests are succeeding for the right reason.
QMalcolm
force-pushed
the
qmalcolm--10000-create-runtime-config-fixture
branch
from
b4cad77 to
5215807
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10242 +/- ##
==========================================
+ Coverage 88.59% 88.67% +0.08%
==========================================
Files 180 180
Lines 22435 22477 +42
==========================================
+ Hits 19876 19932 +56
+ Misses 2559 2545 -14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
6 tasks
QMalcolm
force-pushed
the
qmalcolm--10000-create-runtime-config-fixture
branch
2 times, most recently
from
630edea to
f8a4064
Compare
…ng fixtures While converting `test_from_parts` I noticed the comment > TODO(jeb): Adapters must assert that quoting is populated? This led me to beleive that `quoting` shouldn't be "fully" realized in our project fixture unless we're saying that it's gone through adapter instantiation. Thus I update the `quoting` on our project fixture to be an empty dict. This change affected `test__str__` in `test_project.py` which we thus needed to update accordingly.
…est_project We've done two things in this commit, which arguably _should_ have been done in two commits. First we moved the version specifier tests from `test_runtime.py::TestRuntimeConfig` to `test_project.py::TestGetRequiredVersion` this is because what is really being tested is the `_get_required_version` method. Doing it via `RuntimeConfig.from_parts` method made actually testing it a lot harder as it requires setting up more of the world and running with a _full_ project config dict. The second thing we did was convert it from the old unittest implementation to a pytest implementation. This saves us from having to create most of the world as we were doing previously in these tests. Of note, I did not move the test `test_unsupported_version_range_bad_config`. This test is a bit different from the rest of the version specifier tests. It was introduced in [1eb5857](1eb5857) of [#2726](#2726) to resolve [#2638](#2638). The focus of #2726 was to ensure the version specifier checks were run _before_ the validation of the `dbt_project.yml`. Thus what this test is actually testing for is order of operations at parse time. As such, this is really more a _functional_ test than a unit test. In the next commit we'll get this test moved (and renamed)
…ject schema validation
We do already have tests that ensure "extra" keys aren't allowed in the dbt_project.yaml. This test is different because it's checking that a specific key, `archive`, isn't allowed. We do this because at one point in time `archive` _was_ an allowed key. Specifically, we stopped allowing `archive` in dbt-core 0.15.0 via commit [f26948d](f26948d). Given that it's been 5 years and a major version, we could probably remove this test, but let's keep it around unless we start supporting `archive` again.
QMalcolm
force-pushed
the
qmalcolm--10000-create-runtime-config-fixture
branch
from
f8a4064 to
ce46f02
Compare
QMalcolm
commented
May 31, 2024
tests/unit/config/test_runtime.py
Outdated
| str(config) | ||
|
|
||
|
|
||
| class TestRuntimeConfigOLD(BaseConfigTest): |
Contributor
Author
There was a problem hiding this comment.
This class, TestRuntimeConfigOLD, goes away in a later commit (ce46f02) after we've moved all the tests it contains.
gshank
approved these changes
May 31, 2024
peterallenwebb
approved these changes
May 31, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 #10000
Problem
We haven't had a RuntimeConfig pytest fixture for unit tests. This made writing a lot of unit tests hard because a lot of our code depends on a RuntimeConfig existing.
Solution
Create a
runtime_configpytest fixture and use it to rewrite some existing tests 🙂Checklist