Skip to content

Conversation

@akagami-harsh
Copy link
Member

Which problem is this PR solving?

Description of the changes

  • added goleak checks to all tests that do not fail
  • minor bug fix in check-goleak-file.sh
    • make goleak was listing directories that do not contain any test, added a if condition to prevent that

How was this change tested?

  • make test

Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
@akagami-harsh akagami-harsh requested a review from a team as a code owner December 21, 2023 19:39
Signed-off-by: Harshvir Potpose <[email protected]>
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

Awesome!

@yurishkuro
Copy link
Member

@akagami-harsh some packages do fail, e.g.

goleak: Errors on successful test run: found unexpected goroutines:
[Goroutine 6 in state select, with github.com/golang/glog.(*fileSink).flushDaemon on top of the stack:
github.com/golang/glog.(*fileSink).flushDaemon(0x322b1d8)
	/home/runner/go/pkg/mod/github.com/golang/[email protected]/glog_file.go:351 +0x17c
created by github.com/golang/glog.init.1 in goroutine 1
	/home/runner/go/pkg/mod/github.com/golang/[email protected]/glog_file.go:166 +0x225
]
❌ FAIL	github.com/jaegertracing/jaeger/plugin/storage/integration	3.360s

What method did you use to validate "tests that do not fail"?

@yurishkuro
Copy link
Member

The integration package contains numerous conditions (build tags and env var checks) that disable/enable various storage tests, so it's difficult to test locally to ensure full coverage. I removed it for now.

@yurishkuro yurishkuro added the changelog:ci Change related to continuous integration / testing label Dec 21, 2023
@akagami-harsh
Copy link
Member Author

akagami-harsh commented Dec 21, 2023

What method did you use to validate "tests that do not fail"?

i tested it locally by using make test and by running package tests

@codecov
Copy link

codecov bot commented Dec 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5ef59f6) 95.62% compared to head (9a02522) 95.61%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5025      +/-   ##
==========================================
- Coverage   95.62%   95.61%   -0.02%     
==========================================
  Files         319      319              
  Lines       18786    18786              
==========================================
- Hits        17965    17963       -2     
- Misses        659      661       +2     
  Partials      162      162              
Flag Coverage Δ
cassandra-3.x 25.63% <ø> (ø)
cassandra-4.x 25.63% <ø> (ø)
elasticsearch-5.x 19.90% <ø> (ø)
elasticsearch-6.x 19.89% <ø> (-0.02%) ⬇️
elasticsearch-7.x 20.02% <ø> (-0.02%) ⬇️
elasticsearch-8.x 20.13% <ø> (ø)
grpc-badger 19.53% <ø> (+0.01%) ⬆️
kafka 14.12% <ø> (ø)
opensearch-1.x 20.02% <ø> (-0.02%) ⬇️
opensearch-2.x 20.02% <ø> (-0.02%) ⬇️
unittests 93.36% <ø> (-0.02%) ⬇️

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.

@yurishkuro yurishkuro merged commit eb2a723 into jaegertracing:main Dec 21, 2023
@akagami-harsh akagami-harsh deleted the add-goleak-in-tests branch December 22, 2023 05:34
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