-
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
Fix configuration of turning test warnings into failures #9347
Fix configuration of turning test warnings into failures #9347
Conversation
@MichelleArk |
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.
Let's add a test to these changes!
There isn't really a great existing file for these changes because we're planning on refactoring the existing tests/functional/adapter/test_singular_test.py soon (#9513)
I'd recommend creating a functional test as follows:
- Create a new file under
tests/functional/test_singular_tests.py
- Set up test class with a
tests
fixture. Something like:
single_test_sql = """
{{ config(warn_if = '>0', error_if ="> 10") }}
select 1 as issue
"""
class TestSingularTestWarnError:
@pytest.fixture(scope="class")
def tests(self):
return {"single_test.sql": single_test_sql"}
- Implementing test methods that run
dbt test
with--warn-error-options
as in your PR description, and assert the expected error is raised (similar example)
You'll need postgres setup locally for dbt-core to run the functional tests, there's some documentation here
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9347 +/- ##
==========================================
- Coverage 88.07% 78.33% -9.74%
==========================================
Files 178 178
Lines 22458 22459 +1
==========================================
- Hits 19779 17594 -2185
- Misses 2679 4865 +2186
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
44bc7cf
to
2950572
Compare
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.
Looks great! Thank you for adding the testing. Merging this in now, and it should be available for the 1.8rc1 on May 2: #9784
@MichelleArk thank you for review! 🥹 |
resolves #7761
Problem
dbt --warn-error-option '{"include":"all"}' test -s single_line
and
dbt --warn-error test -s single_line
have different results.Solution
Modfiy suggested line, and then have same results
Checklist