From fccf857bce835fee542263f4bb0fd22ba829c6cb Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Sat, 16 Aug 2025 05:15:38 +0000 Subject: [PATCH] test: fix data race triggered by testing mocks in TestSetJobResultConcurrency (#869) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #862 caused a **new** data race in its mock testing while fixing `setJobResult`'s data race. 🤣 I'm going backwards! ``` ================== WARNING: DATA RACE Read at 0x00c000168d88 by goroutine 943: code.forgejo.org/forgejo/runner/v9/act/model.(*Workflow).GetJob() /home/debian/.cache/act/8afb5309fced9ef8/hostexecutor/act/model/workflow.go:742 +0x244 code.forgejo.org/forgejo/runner/v9/act/model.(*Run).Job() /home/debian/.cache/act/8afb5309fced9ef8/hostexecutor/act/model/planner.go:50 +0xab code.forgejo.org/forgejo/runner/v9/act/runner.setJobResult() /home/debian/.cache/act/8afb5309fced9ef8/hostexecutor/act/runner/job_executor.go:166 +0x7c code.forgejo.org/forgejo/runner/v9/act/runner.TestSetJobResultConcurrency.func2() /home/debian/.cache/act/8afb5309fced9ef8/hostexecutor/act/runner/job_executor_test.go:404 +0xfb Previous write at 0x00c000168d88 by goroutine 944: code.forgejo.org/forgejo/runner/v9/act/model.(*Workflow).GetJob() /home/debian/.cache/act/8afb5309fced9ef8/hostexecutor/act/model/workflow.go:743 +0x258 code.forgejo.org/forgejo/runner/v9/act/model.(*Run).Job() /home/debian/.cache/act/8afb5309fced9ef8/hostexecutor/act/model/planner.go:50 +0xab code.forgejo.org/forgejo/runner/v9/act/runner.setJobResult() /home/debian/.cache/act/8afb5309fced9ef8/hostexecutor/act/runner/job_executor.go:166 +0x7c code.forgejo.org/forgejo/runner/v9/act/runner.TestSetJobResultConcurrency.func3() /home/debian/.cache/act/8afb5309fced9ef8/hostexecutor/act/runner/job_executor_test.go:409 +0xf8 ... ================== ``` - other - [PR](https://code.forgejo.org/forgejo/runner/pulls/869): test: fix data race triggered by testing mocks in TestSetJobResultConcurrency Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/869 Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- act/runner/job_executor_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/act/runner/job_executor_test.go b/act/runner/job_executor_test.go index 3915b971..918ece94 100644 --- a/act/runner/job_executor_test.go +++ b/act/runner/job_executor_test.go @@ -362,6 +362,12 @@ func TestSetJobResultConcurrency(t *testing.T) { }, }, } + // Hack: Job() invokes GetJob() which can mutate the job name, this will trip the data race detector if it is + // encountered later when `setJobResult()` is being tested. This is a false-positive caused by this test invoking + // setJobResult outside of the regular RunContext, so it's invoked here before the goroutines are spawned to prevent + // the false positive. + rc1.Run.Job() + rc2.Run.Job() jim.On("matrix").Return(map[string]interface{}{ "python": []string{"3.10", "3.11", "3.12"},