diff --git a/queue/worker_test.go b/queue/worker_test.go index a52525d12..17357b627 100644 --- a/queue/worker_test.go +++ b/queue/worker_test.go @@ -339,14 +339,11 @@ func (s *WorkerTestSuite) Test_run() { s.SetupTest() s.mockDriver.EXPECT().Pop(queue).Return(nil, errors.QueueDriverNoJobFound).Once() - go func() { - err := s.worker.run() - s.NoError(err) - }() + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("failed to pop job", func() { @@ -355,14 +352,11 @@ func (s *WorkerTestSuite) Test_run() { s.mockLog.EXPECT().Error(errors.QueueDriverFailedToPop.Args(queue, assert.AnError)).Once() - go func() { - err := s.worker.run() - s.NoError(err) - }() + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("job failed, insert failed job", func() { @@ -390,14 +384,11 @@ func (s *WorkerTestSuite) Test_run() { s.mockDB.EXPECT().Table("failed_jobs").Return(mockQuery).Once() mockQuery.EXPECT().Insert(failedJob).Return(nil, nil).Once() - go func() { - err := s.worker.run() - s.NoError(err) - }() + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("failed to insert failed job", func() { @@ -426,14 +417,11 @@ func (s *WorkerTestSuite) Test_run() { mockQuery.EXPECT().Insert(failedJob).Return(nil, assert.AnError).Once() s.mockLog.EXPECT().Error(errors.QueueFailedToSaveFailedJob.Args(assert.AnError, *failedJob)).Once() - go func() { - err := s.worker.run() - s.NoError(err) - }() + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("chain job failed, insert failed job", func() { @@ -498,14 +486,11 @@ func (s *WorkerTestSuite) Test_run() { FailedAt: carbon.NewDateTime(carbon.Now()), }).Return(nil, nil).Once() - go func() { - err := s.worker.run() - s.NoError(err) - }() + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("happy path", func() { @@ -533,14 +518,11 @@ func (s *WorkerTestSuite) Test_run() { mockReservedJob.EXPECT().Delete().Return(nil).Once() s.mockDriver.EXPECT().Pop(queue).Return(nil, errors.QueueDriverNoJobFound).Once() - go func() { - err := s.worker.run() - s.NoError(err) - }() + runErrChan := s.startWorker() time.Sleep(1500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("happy path with chain", func() { @@ -572,14 +554,11 @@ func (s *WorkerTestSuite) Test_run() { mockReservedJob.EXPECT().Delete().Return(nil).Once() s.mockDriver.EXPECT().Pop(queue).Return(nil, errors.QueueDriverNoJobFound).Once() - go func() { - err := s.worker.run() - s.NoError(err) - }() + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("retry job failed, insert failed job", func() { @@ -644,15 +623,12 @@ func (s *WorkerTestSuite) Test_run() { FailedAt: carbon.NewDateTime(carbon.Now()), }).Return(nil, nil).Once() - go func() { - s.worker.tries = 2 // Set tries to 2 for retry - err := s.worker.run() - s.NoError(err) - }() + s.worker.tries = 2 // Set tries to 2 for retry + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) s.worker.tries = 1 }) @@ -696,15 +672,12 @@ func (s *WorkerTestSuite) Test_run() { mockReservedJob.EXPECT().Delete().Return(nil).Once() s.mockDriver.EXPECT().Pop(queue).Return(nil, errors.QueueDriverNoJobFound) - go func() { - s.worker.tries = 3 // Set tries to 3 for retry - err := s.worker.run() - s.NoError(err) - }() + s.worker.tries = 3 // Set tries to 3 for retry + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) s.worker.tries = 1 }) } @@ -759,12 +732,10 @@ func (s *WorkerTestSuite) Test_runWithReceive() { return nil, ctx.Err() }).Once() - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() time.Sleep(200 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("receive error", func() { @@ -788,12 +759,10 @@ func (s *WorkerTestSuite) Test_runWithReceive() { s.mockLog.EXPECT().Error(errors.QueueDriverFailedToReceive.Args(queue, assert.AnError)).Once() - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("happy path", func() { @@ -828,12 +797,10 @@ func (s *WorkerTestSuite) Test_runWithReceive() { s.mockJob.EXPECT().Call(successTask.Job.Signature(), make([]any, 0)).Return(nil).Once() - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("job failed, insert failed job", func() { @@ -881,12 +848,10 @@ func (s *WorkerTestSuite) Test_runWithReceive() { s.mockDB.EXPECT().Table("failed_jobs").Return(mockQuery).Once() mockQuery.EXPECT().Insert(matchFailedJob("test-receive-err")).Return(nil, nil).Once() - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("batch messages", func() { @@ -925,12 +890,10 @@ func (s *WorkerTestSuite) Test_runWithReceive() { s.mockJob.EXPECT().Call(testJobOne.Signature(), make([]any, 0)).Return(nil).Times(3) - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() time.Sleep(500 * time.Millisecond) - s.NoError(s.worker.Shutdown()) + s.shutdownWorker(runErrChan) }) s.Run("shutdown waits for receive loop", func() { @@ -962,16 +925,13 @@ func (s *WorkerTestSuite) Test_runWithReceive() { mockDriverWithReceive.EXPECT().Receive(mock.Anything, queue, s.worker.concurrent). Return([]contractsqueue.ReservedJob{mockReservedJob}, nil).Once() - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() // Shutdown is called while job is still running (job takes 300ms, shutdown at 100ms) time.Sleep(100 * time.Millisecond) // Shutdown should block until the in-flight job completes - err := s.worker.Shutdown() - s.NoError(err) + s.shutdownWorker(runErrChan) s.True(called) }) } @@ -1000,17 +960,13 @@ func (s *WorkerTestSuite) TestShutdown() { s.mockDriver.EXPECT().Pop(s.worker.queue).Return(nil, errors.QueueDriverNoJobFound).Once() // Start the worker - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() // Give worker time to start time.Sleep(100 * time.Millisecond) // Call shutdown - it should wait for WaitGroup - err := s.worker.Shutdown() - - s.NoError(err) + s.shutdownWorker(runErrChan) s.True(s.worker.isShutdown.Load()) }) @@ -1038,16 +994,13 @@ func (s *WorkerTestSuite) TestShutdown() { mockReservedJob.EXPECT().Delete().Return(nil).Once() // Start the worker - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() // Give worker time to start processing time.Sleep(100 * time.Millisecond) // Shutdown should wait for the job to complete - err := s.worker.Shutdown() - s.NoError(err) + s.shutdownWorker(runErrChan) s.True(called) }) @@ -1086,16 +1039,13 @@ func (s *WorkerTestSuite) TestShutdown() { mockQuery.EXPECT().Insert(matchFailedJob("test-error")).Return(nil, nil).Once() // Start the worker - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() // Give worker time to process the failed job time.Sleep(200 * time.Millisecond) // Shutdown should wait for failed job logger goroutine - err := s.worker.Shutdown() - s.NoError(err) + s.shutdownWorker(runErrChan) }) s.Run("shutdown when a task is running but fails finally", func() { @@ -1136,16 +1086,13 @@ func (s *WorkerTestSuite) TestShutdown() { mockQuery.EXPECT().Insert(matchFailedJob("test-error")).Return(nil, nil).Once() // Start the worker - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() // Give worker time to process the failed job time.Sleep(200 * time.Millisecond) // Shutdown should wait for failed job logger goroutine - err := s.worker.Shutdown() - s.NoError(err) + s.shutdownWorker(runErrChan) }) s.Run("shutdown with multiple jobs", func() { @@ -1180,19 +1127,39 @@ func (s *WorkerTestSuite) TestShutdown() { s.mockDriver.EXPECT().Pop(s.worker.queue).Return(nil, errors.QueueDriverNoJobFound).Twice() // Start the worker - go func() { - s.NoError(s.worker.run()) - }() + runErrChan := s.startWorker() // Give workers time to process time.Sleep(200 * time.Millisecond) // Shutdown should wait for all workers to complete - err := s.worker.Shutdown() - s.NoError(err) + s.shutdownWorker(runErrChan) }) } +// startWorker runs the worker in the background and returns a channel that receives run's result. +// Asserting on that result from the subtest goroutine, instead of inside the background goroutine, +// keeps it from racing with testify resetting the suite's T once the subtest ends. +func (s *WorkerTestSuite) startWorker() <-chan error { + runErrChan := make(chan error, 1) + go func() { + runErrChan <- s.worker.run() + }() + + return runErrChan +} + +// shutdownWorker shuts the worker down and waits for the run started by startWorker to return. +func (s *WorkerTestSuite) shutdownWorker(runErrChan <-chan error) { + s.NoError(s.worker.Shutdown()) + select { + case err := <-runErrChan: + s.NoError(err) + case <-time.After(5 * time.Second): + s.Fail("the worker did not stop after shutdown") + } +} + // matchFailedJob is a helper to match FailedJob with specific UUID func matchFailedJob(uuid string) interface{} { return mock.MatchedBy(func(job *models.FailedJob) bool {