Skip to content

Don't put "pass"ed test as warning, with a unit test#767

Merged
RoiTabach merged 1 commit intomasterfrom
ele-518-trying-again-for-git
Mar 22, 2023
Merged

Don't put "pass"ed test as warning, with a unit test#767
RoiTabach merged 1 commit intomasterfrom
ele-518-trying-again-for-git

Conversation

@RoiTabach
Copy link
Copy Markdown
Contributor

re opening to fix git commit mayham.
Coments from prev CR:
Or - add a unit test
Idan - make "warning" be in elif and the everything else should be skipped

@RoiTabach RoiTabach requested a review from IDoneShaveIt March 22, 2023 08:35
@linear
Copy link
Copy Markdown

linear bot commented Mar 22, 2023

ELE-518 Passed tests are listed with the Warnings in edr monitor report slack summary

Describe the bug
In the slack message that is sent with the report, we count correctly which of the tests are passed/failed/warning. However, when creating the details section, we count passed tests along with the warnings.

To Reproduce
Steps to reproduce the behavior:

  1. Run dbt and have a small number of Passing tests. (I added a not_null test on the one model from our integration tests and ran dbt build -s one
  2. Run edr monitor send-report
  3. Look at the summary numbers
  4. See the passed tests are counted properly but listed with the Warnings.

Expected behavior
No details for passed tests.

Screenshots

Environment (please complete the following information):

  • edr Version: [e.g. 0.5.3], can be found by running pip show elementary-data
  • dbt package Version: [e.g. 0.4.1], can be found in packages.yml file

Additional context
Raised in Slack thread https://elementary-community.slack.com/archives/C02CTC89LAX/p1678878540310359
I believe the fact we truncate the report on Slack if it's got too many items caused us to not see it even tho it was there for a long time - since we usually test with 100+ issues so the details view is usually not seen.

@github-actions
Copy link
Copy Markdown
Contributor

👋 @RoiTabach
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

@RoiTabach RoiTabach merged commit 2afee2a into master Mar 22, 2023
@RoiTabach RoiTabach deleted the ele-518-trying-again-for-git branch March 22, 2023 09:09
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.

1 participant