From 4bbb60402f262214aecacb24839e75159143a43f Mon Sep 17 00:00:00 2001 From: Dmitrii Anoshin Date: Thu, 30 May 2024 12:18:33 -0700 Subject: [PATCH] [chore] Remove redundant wait group from a test (#10269) https://github.com/open-telemetry/opentelemetry-collector/pull/10258/files#r1621017272 made me wonder if the wait group is really necessary. We have another synchronization that waits for at least two requests to enter the merge function, which should be enough. So, we don't necessarily need this wait group. Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- exporter/exporterhelper/batch_sender_test.go | 26 ++++++-------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/exporter/exporterhelper/batch_sender_test.go b/exporter/exporterhelper/batch_sender_test.go index 08ab42f89b5..bc59c0d63c5 100644 --- a/exporter/exporterhelper/batch_sender_test.go +++ b/exporter/exporterhelper/batch_sender_test.go @@ -443,9 +443,10 @@ func TestBatchSender_ShutdownDeadlock(t *testing.T) { waitMerge := make(chan struct{}, 10) // blockedBatchMergeFunc blocks until the blockMerge channel is closed - blockedBatchMergeFunc := func(_ context.Context, r1 Request, _ Request) (Request, error) { + blockedBatchMergeFunc := func(_ context.Context, r1 Request, r2 Request) (Request, error) { waitMerge <- struct{}{} <-blockMerge + r1.(*fakeRequest).items += r2.(*fakeRequest).items return r1, nil } @@ -458,18 +459,11 @@ func TestBatchSender_ShutdownDeadlock(t *testing.T) { sink := newFakeRequestSink() - // Send 10 concurrent requests and wait for them to start - startWG := sync.WaitGroup{} - for i := 0; i < 10; i++ { - startWG.Add(1) - go func() { - startWG.Done() - require.NoError(t, be.send(context.Background(), &fakeRequest{items: 4, sink: sink})) - }() - } - startWG.Wait() + // Send 2 concurrent requests + go func() { require.NoError(t, be.send(context.Background(), &fakeRequest{items: 4, sink: sink})) }() + go func() { require.NoError(t, be.send(context.Background(), &fakeRequest{items: 4, sink: sink})) }() - // Wait for at least one batch to enter the merge function + // Wait for the requests to enter the merge function <-waitMerge // Initiate the exporter shutdown, unblock the batch merge function to catch possible deadlocks, @@ -485,12 +479,8 @@ func TestBatchSender_ShutdownDeadlock(t *testing.T) { close(blockMerge) <-doneShutdown - // The exporter should have sent only one "merged" batch, in some cases it might send two if the shutdown - // happens before the batch is fully merged. - assert.LessOrEqual(t, uint64(1), sink.requestsCount.Load()) - - // blockedBatchMergeFunc just returns the first request, so the items count should be 4 times the requests count. - assert.Equal(t, sink.requestsCount.Load()*4, sink.itemsCount.Load()) + assert.EqualValues(t, 1, sink.requestsCount.Load()) + assert.EqualValues(t, 8, sink.itemsCount.Load()) } func queueBatchExporter(t *testing.T, batchOption Option) *baseExporter {