[darakanoit]

📑 Description

Related to goravel/goravel#1006 (#1564)

Found while running the worker tests with -race after #1564 (multiple queues in the worker)
landed. The race is not introduced by that change: see below.

go test -race -run TestWorkerTestSuite ./queue/ fails on master in 2 of 3 runs with
race detected during execution of test, in subtests such as
Test_run/job_failed,_insert_failed_job and
TestShutdown/shutdown_blocks_until_goroutines_complete. CI runs these tests without
-race, so it went unnoticed.

The race is in the tests, not in queue/worker.go. Every report pairs the same two
accesses:

• write: testify's suite.(*Suite).SetT(), which restores the parent T when a
  s.Run subtest returns;
• read: the goroutine that runs the worker and asserts on its result:

go func() {
	err := s.worker.run()
	s.NoError(err) // reads s.T()
}()

time.Sleep(500 * time.Millisecond)

s.NoError(s.worker.Shutdown())

Shutdown() waits for jobWg and failedJobWg, but run() also waits on jobWg and
returns in parallel with Shutdown(). Nothing orders the goroutine's s.NoError before the
subtest returns, so the assertion can run while testify is resetting T. Shutdown() does
not promise that run() has returned, and no production code reads shared state after it
does, so the worker itself is fine.

The pattern predates #1564: dab1968c^ fails the same way (2 of 5 runs with -race).

What changed

Two suite helpers, following the pattern worker_multi_queue_test.go already uses:

• startWorker() runs the worker in a goroutine and returns a channel that receives
  run()'s error;
• shutdownWorker(runErrChan) calls Shutdown(), then waits for that error (5s timeout)
  and asserts on it from the subtest goroutine.

All 20 background runs in Test_run, Test_runWithReceive and TestShutdown use them.
The two retry subtests now set s.worker.tries before starting the worker instead of
inside the goroutine.

Verification

	before	after
go test -race -count=1 -run TestWorkerTestSuite ./queue/ (x3)	2 of 3 fail	pass
go test -race -count=10 -run TestWorkerTestSuite ./queue/	—	pass, 0 DATA RACE
go test -race ./queue/...	fail	pass
go test ./queue/...	pass	pass

Scope

• queue/worker_test.go only. No production code changed.

✅ Checks

• Added test cases for my code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.84%. Comparing base (9385000) to head (5a8b9fc).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1566   +/-   ##
=======================================
  Coverage   72.84%   72.84%           
=======================================
  Files         412      412           
  Lines       26846    26846           
=======================================
  Hits        19557    19557           
  Misses       7287     7287           
  Partials        2        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[darakanoit]

The integration job fails in TestValidationTestSuite. That test fails on master as well, since #1548, so it is not caused by this PR. The fix is in goravel/example#155.

[hwbrzzl]

Thanks!