-
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
Move unit testing to test and build commands #9108
Move unit testing to test and build commands #9108
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## unit_testing_feature_branch #9108 +/- ##
===============================================================
- Coverage 86.85% 86.81% -0.04%
===============================================================
Files 181 180 -1
Lines 27156 27083 -73
===============================================================
- Hits 23586 23512 -74
- Misses 3570 3571 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
class TestRunner(CompileRunner): | ||
_ANSI_ESCAPE = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])") |
Check warning
Code scanning / CodeQL
Overly permissive regular expression range Medium test
class TestRunner(CompileRunner): | ||
_ANSI_ESCAPE = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])") |
Check warning
Code scanning / CodeQL
Overly permissive regular expression range Medium test
I believe this also resolves #9052 |
core/dbt/task/runnable.py
Outdated
@@ -496,7 +493,7 @@ def run(self): | |||
|
|||
if self.args.write_json: | |||
# args.which used to determine file name for unit test manifest |
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.
small nit: comment can be deleted now too
actual = [str(r.status) for r in results] | ||
expected = ["error"] * 1 + ["skipped"] * 5 + ["pass"] * 2 + ["success"] * 5 | ||
expected = ["error"] * 1 + ["skipped"] * 6 + ["pass"] * 2 + ["success"] * 5 |
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 will need to be updated once we fix the order of unit tests -> model -> data tests, as expected 👍
self.yaml.path.original_file_path, | ||
unit_test.model, | ||
unit_test.name, | ||
) |
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.
I took this for a spin locally and it worked as expected! 🎩
--- dbt_project.yml
...
unit-tests:
jaffle_shop:
unit_testing:
my_model:
+tags: project
schema.yml in models/unit_testing/schema.yml
dbt build -s +my_model,test_type:unit --exclude tag:project
=> finds no unit tests. moving the config around at various levels also worked as expected.
Would be good to add some functional testing around this either in this PR or in a follow-on issue if it balloons the scope too much. We do have tests on the fqn in tests/unit/test_unit_test_parser.py so for the scope of this refactor those are good!
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.
💪 big lift!
* Initial implementation of unit testing (from pr #2911) Co-authored-by: Michelle Ark <[email protected]> * 8295 unit testing artifacts (#8477) * unit test config: tags & meta (#8565) * Add additional functional test for unit testing selection, artifacts, etc (#8639) * Enable inline csv format in unit testing (#8743) * Support unit testing incremental models (#8891) * update unit test key: unit -> unit-tests (#8988) * convert to use unit test name at top level key (#8966) * csv file fixtures (#9044) * Unit test support for `state:modified` and `--defer` (#9032) Co-authored-by: Michelle Ark <[email protected]> * Allow use of sources as unit testing inputs (#9059) * Use daff for diff formatting in unit testing (#8984) * Fix #8652: Use seed file from disk for unit testing if rows not specified in YAML config (#9064) Co-authored-by: Michelle Ark <[email protected]> Fix #8652: Use seed value if rows not specified * Move unit testing to test and build commands (#9108) * Enable unit testing in non-root packages (#9184) * convert test to data_test (#9201) * Make fixtures files full-fledged members of manifest and enable partial parsing (#9225) * In build command run unit tests before models (#9273) --------- Co-authored-by: Michelle Ark <[email protected]> Co-authored-by: Michelle Ark <[email protected]> Co-authored-by: Emily Rockman <[email protected]> Co-authored-by: Jeremy Cohen <[email protected]> Co-authored-by: Kshitij Aranke <[email protected]>
resolves #8979
Problem
We've decided to move the unit testing functionality to the 'test' command from the 'unit-test' command. In addition this pull request will support running unit tests in the 'build' command.
Solution
Move code from task/unit_test.py to task/test.py. Construct a mini-manifest from just the executing test node, instead of constructing it all at once.
Checklist