-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat: add message when test_result is setup #331
Conversation
To help users confirm that test_results is setup correctly we want to display a message near the header saying that that is the case and all tests passed. These changes accomplish that by creating a new `NotificationContext` in the `ComparisonProxy` and carrying the info to the notifier.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #331 +/- ##
==========================================
- Coverage 98.11% 98.11% -0.01%
==========================================
Files 385 385
Lines 31955 31989 +34
==========================================
+ Hits 31354 31386 +32
- Misses 601 603 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention:
@@ Coverage Diff @@
## main #331 +/- ##
==========================================
- Coverage 98.11% 98.11% -0.01%
==========================================
Files 385 385
Lines 31955 31989 +34
==========================================
+ Hits 31354 31386 +32
- Misses 601 603 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention: Patch coverage is
Changes have been made to critical files, which contain lines commonly executed in production. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #331 +/- ##
==========================================
- Coverage 98.11% 98.10% -0.01%
==========================================
Files 416 416
Lines 32655 32689 +34
==========================================
+ Hits 32038 32070 +32
- Misses 617 619 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
This change has been scanned for critical changes. Learn more |
yield ("") | ||
yield ( | ||
":heavy_check_mark: Test ingestion set up successfully. No failed tests found. :relaxed:" | ||
) |
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.
Looks like this message will fire every time the all_test_passed
bool is true. @giovanni-guidini can you confirm?
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.
That is accurate.
Keep in mind that if the setup for test_results is complete but not all tests passed then we don't notify with this notifier.
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.
in that case... after first time setup every time all test pass, we'll show a message titled "Test ingestion set up successfully..."
Which is a little weird..
Maybe we alter that message to say "All tests successful...
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.
@Adal3n3 @giovanni-guidini and @rohan-at-sentry aligned
Tweak the message to
✅ All tests successful. No failed tests found
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
To help users confirm that test_results is setup correctly we want to display
a message near the header saying that that is the case and all tests passed.
These changes accomplish that by creating a new
NotificationContext
in theComparisonProxy
and carrying the info to the notifier.Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.