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.

Sign up for GitHub

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

Jump to bottom

Fix #8652: Use seed file from disk for unit testing if rows not specified in YAML config #9064

Merged
merged 19 commits into from
Nov 16, 2023
Merged

Fix #8652: Use seed file from disk for unit testing if rows not specified in YAML config #9064

merged 19 commits into from
Nov 16, 2023

Conversation

aranke
Copy link
Member

@aranke aranke commented Nov 13, 2023 •
edited
Loading

Resolves #8652

Problem

Seed values must be specified for unit testing.

Solution

Read seed from disk if no rows are specified in YAML config, instead of throwing an error.

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
  • This PR includes type annotations for new and modified functions

@cla-bot cla-bot bot added the cla:yes label Nov 13, 2023
@aranke aranke changed the base branch from main to unit_testing_feature_branch November 13, 2023 15:46
@codecov Codecov
Copy link

codecov bot commented Nov 13, 2023 •
edited
Loading

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (35f579e) 86.82% compared to head (6d4f4a6) 86.79%.

Files Patch % Lines
core/dbt/parser/unit_tests.py 95.23% 1 Missing ⚠️
Additional details and impacted files 
@@                       Coverage Diff                       @@
##           unit_testing_feature_branch    #9064      +/-   ##
===============================================================
- Coverage                        86.82%   86.79%   -0.04%     
===============================================================
  Files                              181      181              
  Lines                            27107    27126      +19     
===============================================================
+ Hits                             23537    23543       +6     
- Misses                            3570     3583      +13
Flag Coverage Δ
integration 83.74% <95.23%> (-0.10%) ⬇️
unit 64.51% <19.04%> (-0.04%) ⬇️

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

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

@aranke aranke marked this pull request as ready for review November 14, 2023 14:51
@aranke aranke requested a review from a team as a code owner November 14, 2023 14:51
@aranke aranke requested a review from QMalcolm November 14, 2023 14:51
@aranke aranke requested review from gshank, emmyoop and MichelleArk and removed request for QMalcolm November 14, 2023 15:03
@aranke aranke changed the title fix 8652 Fix #8652: Use seed file from disk for unit testing if rows not specified in YAML config Nov 14, 2023
@aranke
Copy link
Member Author

aranke commented Nov 14, 2023

@MichelleArk I think I addressed all your comments above, ready for re-review. Thanks!

@aranke
Copy link
Member Author

aranke commented Nov 14, 2023 •
edited
Loading

I am not sure why the log schema is failing in CI, investigating.

@aranke
Copy link
Member Author

aranke commented Nov 15, 2023

@MichelleArk Addressed second round of comments, thanks for the detailed feedback!

MichelleArk
MichelleArk approved these changes Nov 15, 2023
View reviewed changes
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 refactoring suggestions, otherwise LGTM!

@aranke aranke merged commit 3432436 into unit_testing_feature_branch Nov 16, 2023
49 checks passed
@aranke aranke deleted the fix_8652 branch November 16, 2023 16:24
gshank added a commit that referenced this pull request Jan 16, 2024
@gshank @MichelleArk
@emmyoop @jtcohen6 @aranke
Unit testing feature branch pull request (#8411)
b5a0c4c 
* 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
Reviewers

@MichelleArk MichelleArk MichelleArk approved these changes

@gshank gshank Awaiting requested review from gshank

@emmyoop emmyoop Awaiting requested review from emmyoop

Assignees
No one assigned
Labels
cla:yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CT-3123] Make seed inputs optional for unit tests
2 participants
@aranke @MichelleArk