Skip to content

Conversation

@yurishkuro
Copy link
Member

Which problem is this PR solving?

  • Part of Enable and enforce goroutine leak checks in tests #5006
  • We have only two packages remaining without goroutine leak checks, both integration tests, which are being worked on
  • Meanwhile new code / packages do not benefit from enforcement of goleak checks because the linter does not fail CI

Description of the changes

  • Add the two remaining packages into exclusion list and force a failure of the CI if anything else is missing goleak checks

How was this change tested?

Successful exit:

$ make goleak
Verifying that all packages with tests have goleak in their TestMain
🔴 Error(check-goleak): no goleak check in package ./cmd/jaeger/internal/integration/
	this package is temporarily allowed and will not cause linter failure
🔴 Error(check-goleak): no goleak check in package ./plugin/storage/integration/
	this package is temporarily allowed and will not cause linter failure
🐞 Warning(check-goleak): no goleak check in 2 package(s).
	See https://github.com/jaegertracing/jaeger/pull/5010/files
	for examples of adding the checks.

Unsuccessful exit (forced by removing one of the existing checks):

$ make goleak
Verifying that all packages with tests have goleak in their TestMain
🔴 Error(check-goleak): no goleak check in package ./cmd/jaeger/internal/integration/
	this package is temporarily allowed and will not cause linter failure
🔴 Error(check-goleak): no goleak check in package ./internal/tracegen/
🔴 Error(check-goleak): no goleak check in package ./plugin/storage/integration/
	this package is temporarily allowed and will not cause linter failure
⛔ Fatal(check-goleak): no goleak check in 3 package(s), 1 of which not allowed.
	See https://github.com/jaegertracing/jaeger/pull/5010/files
	for examples of adding the checks.
make: *** [goleak] Error 1

Signed-off-by: Yuri Shkuro <[email protected]>
@yurishkuro yurishkuro added the changelog:ci Change related to continuous integration / testing label Apr 1, 2024
@yurishkuro yurishkuro requested a review from a team as a code owner April 1, 2024 00:20
@yurishkuro yurishkuro requested a review from joe-elliott April 1, 2024 00:20
@codecov
Copy link

codecov bot commented Apr 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.16%. Comparing base (727bf18) to head (aeff532).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5316   +/-   ##
=======================================
  Coverage   95.16%   95.16%           
=======================================
  Files         340      340           
  Lines       16666    16666           
=======================================
  Hits        15860    15860           
  Misses        616      616           
  Partials      190      190           
Flag Coverage Δ
badger 12.25% <ø> (ø)
cassandra-3.x 21.14% <ø> (ø)
cassandra-4.x 21.14% <ø> (ø)
elasticsearch-5.x 17.70% <ø> (ø)
elasticsearch-6.x 17.68% <ø> (-0.02%) ⬇️
elasticsearch-7.x 17.76% <ø> (+0.01%) ⬆️
elasticsearch-8.x 17.83% <ø> (ø)
grpc 11.59% <ø> (-0.01%) ⬇️
kafka 11.84% <ø> (ø)
opensearch-1.x 17.75% <ø> (-0.02%) ⬇️
opensearch-2.x 17.76% <ø> (+0.01%) ⬆️
unittests 92.34% <ø> (ø)

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.

Copy link
Contributor

@jkowall jkowall left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@jkowall jkowall merged commit f18416a into jaegertracing:main Apr 1, 2024
@yurishkuro yurishkuro deleted the fail-on-goleak-linter branch April 1, 2024 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:ci Change related to continuous integration / testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants