Skip to content
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

Add additional functional test for unit testing selection, artifacts, etc #8639

Merged
merged 2 commits into from
Sep 13, 2023

Conversation

gshank
Copy link
Contributor

@gshank gshank commented Sep 12, 2023

resolves #8512

Problem

Previous pull request didn't include necessary functional tests.

Solution

Add more functional tests.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

@gshank gshank requested review from a team as code owners September 12, 2023 23:08
@gshank gshank requested review from eddowh, nathaniel-may and QMalcolm and removed request for a team September 12, 2023 23:08
@cla-bot cla-bot bot added the cla:yes label Sep 12, 2023
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.09% 🎉

Comparison is base (12342ca) 86.68% compared to head (0b149d5) 86.77%.

Additional details and impacted files
@@                       Coverage Diff                       @@
##           unit_testing_feature_branch    #8639      +/-   ##
===============================================================
+ Coverage                        86.68%   86.77%   +0.09%     
===============================================================
  Files                              176      176              
  Lines                            26006    26002       -4     
===============================================================
+ Hits                             22543    22564      +21     
+ Misses                            3463     3438      -25     
Flag Coverage Δ
integration 83.68% <ø> (+0.16%) ⬆️
unit 64.98% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MichelleArk MichelleArk changed the base branch from main to unit_testing_feature_branch September 13, 2023 10:11
@@ -118,5 +124,52 @@ def test_basic(self, project):
results = run_dbt(["run"])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ can't comment one line above but - I don't think the run_dbt(["deps"]) is necessary here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. That was left over from an unnecessary dependency that was removed.

Comment on lines +131 to +141
# Test select by test name
results = run_dbt(["unit-test", "--select", "test_name:test_my_model_string_concat"])
assert len(results) == 1

# Select, method not specified
results = run_dbt(["unit-test", "--select", "test_my_model_overrides"])
assert len(results) == 1

# Select using tag
results = run_dbt(["unit-test", "--select", "tag:test_this"])
assert len(results) == 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about splitting these assertions into their own test - perhaps test_selection? They will still require the call to run_dbt(["run"]) as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer that, but having more than one test method per test class leads to flaky tests, which would probably happen here due to the partial parsing test.

Copy link
Contributor

@MichelleArk MichelleArk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple small suggestions, otherwise LGTM!

@gshank gshank merged commit c48e34c into unit_testing_feature_branch Sep 13, 2023
@gshank gshank deleted the 8512-unit_testing_tests branch September 13, 2023 14:46
gshank added a commit that referenced this pull request Jan 16, 2024
* Initial implementation of unit testing (from pr #2911)

Co-authored-by: Michelle Ark <michelle.ark@dbtlabs.com>

* 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 <michelle.ark@dbtlabs.com>

* 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 <MichelleArk@users.noreply.github.com>
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 <michelle.ark@dbtlabs.com>
Co-authored-by: Michelle Ark <MichelleArk@users.noreply.github.com>
Co-authored-by: Emily Rockman <emily.rockman@dbtlabs.com>
Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>
Co-authored-by: Kshitij Aranke <kshitij.aranke@dbtlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-3043] Add more functional tests for unit test selection and artifact generation
2 participants