Fix goroutine leaks raised by plugin/storage/integration/cassandra_test.go #5311
Conversation
There doesn't seem to be any leaks detected for this package. Signed-off-by: Will Sewell <[email protected]>
yurishkuro
added
the
changelog:test
|
I think |
Pull request was converted to draft
Signed-off-by: Will Sewell <[email protected]>
WillSewell
force-pushed
the
add-goleak-to-plugin-storage-integration
branch
from
a17c814 to
55089ae
Compare
Signed-off-by: Will Sewell <[email protected]>
Signed-off-by: Will Sewell <[email protected]>
Signed-off-by: Will Sewell <[email protected]>
There are still a number of violations and I think it's more important we prioritise getting the current fixes to cassandra/factory.go rather than risk these not getting merged at all while we are blocked on other violations in the package. Signed-off-by: Will Sewell <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5311 +/- ##
==========================================
+ Coverage 95.15% 95.26% +0.11%
==========================================
Files 340 340
Lines 16666 16677 +11
==========================================
+ Hits 15858 15888 +30
+ Misses 617 600 -17
+ Partials 191 189 -2
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: Will Sewell <[email protected]>
Signed-off-by: Will Sewell <[email protected]>
| @@ -109,7 +112,7 @@ func (s *CassandraStorageIntegration) initializeDependencyReaderAndWriter(t *tes | |||
| } | |||
|
|
|||
| func TestCassandraStorage(t *testing.T) { | |||
| skipUnlessEnv(t, "cassandra") | |||
| // skipUnlessEnv(t, "cassandra") | |||
WillSewell
changed the title
Signed-off-by: Will Sewell <[email protected]>
|
@yurishkuro this has stalled because I got bogged down fixing various other goroutine leaks that goleak detects in this package. I fixed the ones in the grpc tests, but lost momentum on the ones in the elasticsearch tests because at least one leak is actually originating in the So my thinking is rather than a single big PR that fixes all leaks (and risks not getting done at all), I'll instead aim to do some smaller PRs to cover the things I have managed to fixed. No guarantees whether I'll get to the elasticsearch fixes... |
yeah, you can stop this sentence here, I am always +1 to smaller PRs :-) |
Which problem is this PR solving?
Description of the changes
How was this change tested?
STORAGE=cassandra go test -v ./plugin/storage/integrationChecklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test