-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Create RuntimeConfig
fixture
#10242
Create RuntimeConfig
fixture
#10242
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.
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. |
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.
f8a4064
to
ce46f02
Compare
tests/unit/config/test_runtime.py
Outdated
str(config) | ||
|
||
|
||
class TestRuntimeConfigOLD(BaseConfigTest): |
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.
This class, TestRuntimeConfigOLD
, goes away in a later commit (ce46f02) after we've moved all the tests it contains.
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_config
pytest fixture and use it to rewrite some existing tests 🙂Checklist