-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[chore] enable coverage for unit tests #22966
[chore] enable coverage for unit tests #22966
Conversation
This re-enables unit test coverage for the repository. Coverage will not immediately include integration tests. Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #22966 +/- ##
===========================================
- Coverage 91.91% 83.09% -8.82%
===========================================
Files 494 1603 +1109
Lines 23939 152186 +128247
===========================================
+ Hits 22003 126464 +104461
- Misses 1429 22123 +20694
- Partials 507 3599 +3092
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
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.
Thank you for adding this, I appreciate being able to run CI steps locally using make :D
Note that I ran into this issue when enabling coverage. We may want to disable ci failures for coverage uploads. Will need to keep an eye on how frequently this impacts the builds. |
@@ -96,7 +96,12 @@ common: lint | |||
.PHONY: test | |||
test: | |||
$(GOTEST) $(GOTEST_OPT) ./... | |||
|
|||
|
|||
.PHONY: test-with-cover |
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.
Should we remove / deprecate do-unit-tests-with-cover
? It's not clear to me if there's a meaningful difference.
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.
Yeah that's a follow up PR, i wanted to keep it until i figured out the integration tests :)
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.
Possibly relevant, in case you haven't seen it: #22808
This re-enables unit test coverage for the repository. Coverage will not immediately include integration tests. Fixes open-telemetry#8394 --------- Signed-off-by: Alex Boten <[email protected]>
This re-enables unit test coverage for the repository. Coverage will not immediately include integration tests.
Fixes #8394