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

(#1597) summarize warnings at end of test invocations #1654

Merged
merged 1 commit into from
Aug 4, 2019

Conversation

drewbanin
Copy link
Contributor

Summary of changes:

  • Include warn as a status in the RunModelResult schema
    • set to True if a test fails with severity='warning'
  • Render out a list of warnings (in yellow) at the end of the run
  • Change list of errors at the end of the run to be red
  • Include warning count in the results status line
  • Touched up a lot of pluralization code while I was in here -- 1 warning vs 1 warnings

Some of the printer-related code here is a little gross... very happy to clean it up if there are ideas about how we can improve it.

Example output

Errors + Warnings

Running with dbt=0.14.0
Found 1 model, 5 tests, 2 snapshots, 0 analyses, 230 macros, 0 operations, 1 seed file, 0 sources

12:34:50 | Concurrency: 8 threads (target='dev')
12:34:50 | 
12:34:50 | 1 of 5 START test error1................................... [RUN]
12:34:50 | 2 of 5 START test idk...................................... [RUN]
12:34:50 | 3 of 5 START test idk2..................................... [RUN]
12:34:50 | 4 of 5 START test idk3..................................... [RUN]
12:34:50 | 5 of 5 START test warn1.................................... [RUN]
12:34:50 | 1 of 5 FAIL 2 error1....................................... [FAIL 2 in 0.05s]
12:34:50 | 4 of 5 PASS idk3........................................... [PASS in 0.06s]
12:34:50 | 3 of 5 FAIL 1 idk2......................................... [FAIL 1 in 0.06s]
12:34:50 | 5 of 5 WARN 2 warn1........................................ [WARN 2 in 0.06s]
12:34:50 | 2 of 5 WARN 1 idk.......................................... [WARN 1 in 0.07s]
12:34:50 | 
12:34:50 | Finished running 5 tests in 0.79s.

Completed with 2 errors and 2 warnings:

Failure in test error1 (tests/error1.sql)
  Got 2 results, expected 0.

  compiled SQL at target/compiled/my_new_package/data_test/error1.sql

Failure in test idk2 (tests/idk2.sql)
  Got 1 result, expected 0.

  compiled SQL at target/compiled/my_new_package/data_test/idk2.sql

Warning in test warn1 (tests/warn1.sql)
  Got 2 results, expected 0.

  compiled SQL at target/compiled/my_new_package/data_test/warn1.sql

Warning in test idk (tests/idk.sql)
  Got 1 result, expected 0.

  compiled SQL at target/compiled/my_new_package/data_test/idk.sql

Done. PASS=1 WARN=2 ERROR=2 SKIP=0 TOTAL=5

Just warnings

Running with dbt=0.14.0
Found 1 model, 5 tests, 2 snapshots, 0 analyses, 230 macros, 0 operations, 1 seed file, 0 sources

12:37:44 | Concurrency: 8 threads (target='dev')
12:37:44 | 
12:37:44 | 1 of 1 START test warn1......................................... [RUN]
12:37:44 | 1 of 1 WARN 2 warn1............................................. [WARN 2 in 0.02s]
12:37:44 | 
12:37:44 | Finished running 1 test in 0.65s.

 Completed with 1 warning:

 Warning in test warn1 (tests/warn1.sql)
  Got 2 results, expected 0.

  compiled SQL at target/compiled/my_new_package/data_test/warn1.sql

Done. PASS=0 WARN=1 ERROR=0 SKIP=0 TOTAL=1

Screen Shot 2019-08-03 at 12 41 39 PM

@drewbanin drewbanin requested a review from beckjake August 3, 2019 17:49
Copy link
Contributor

@beckjake beckjake left a comment

Choose a reason for hiding this comment

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

Some of the printer-related code here is a little gross... very happy to clean it up if there are ideas about how we can improve it.

I think the ultimate fix here is to change how results work, to have pass, error, warn, and fail be statuses of an enum rather than individual booleans. Or maybe error remains a boolean and the other three become an enum used in tests (including source freshness). Both seem reasonable! Then we'll have to mess with dbt.ui.printer anyway, which will be a good chance to clean it up a bit.

This is one of those parts of the code that has experienced organic growth - states clearly just get tacked on as needed even though they're basically mutually exclusive, because it always feels much safer to add another boolean than it does to change it all to set an enum.

That's probably out of scope for this PR, and the 0.14.x branch is probably not place the to make that change - hologram and dataclasses will make it a lot easier and lower-risk.

@@ -481,3 +481,10 @@ def translate_aliases(kwargs, aliases):
result[canonical_key] = value

return result


def pluralize(count, string):
Copy link
Contributor

Choose a reason for hiding this comment

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

Long overdue!

@drewbanin drewbanin merged commit e135681 into dev/0.14.1 Aug 4, 2019
@drewbanin drewbanin deleted the fix/summarize-warns branch August 4, 2019 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants