From b9ce46e95ac5a64ab36e7c6ed32543e5d2eb328b Mon Sep 17 00:00:00 2001 From: Zhao Chen Date: Tue, 7 Jul 2026 22:17:22 +0800 Subject: [PATCH] test(build): fix data race in TestBuildLayer under -race BuildLayer hands OutputLayer a live *io.PipeReader backed by a background tar writer goroutine. The generated testify mock formats every argument with fmt (mock.Arguments.Diff) to match the call, which reflects into the pipe's internal mutex concurrently with the writer -- a data race under `go test -race`. No matcher avoids it: Diff formats the actual argument before any matcher logic runs. Replace the testify mock in the "successful build layer" subtest with a small hand-written OutputStrategy that reads the reader (as the real local/remote strategies do). This avoids reflecting over the live pipe and unblocks the writer goroutine. Production is unchanged. Claude-Session: https://claude.ai/code/session_01HHaMSeWpe3apQEx7n4UvRg Signed-off-by: Zhao Chen --- pkg/backend/build/builder_test.go | 45 +++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/pkg/backend/build/builder_test.go b/pkg/backend/build/builder_test.go index 3c8f117a..bf2d4680 100644 --- a/pkg/backend/build/builder_test.go +++ b/pkg/backend/build/builder_test.go @@ -115,6 +115,39 @@ func (s *BuilderTestSuite) TestNewBuilder() { } } +// recordingStrategy is a hand-written OutputStrategy used by TestBuildLayer. +// +// BuildLayer hands OutputLayer a live *io.PipeReader whose background tar +// writer goroutine is mid-write. The generated testify mock formats every +// argument with fmt (via mock.Arguments.Diff) when matching the call, which +// reflects into the pipe's internal state concurrently with that writer -- a +// data race reported under `go test -race`. This fake reads the reader like +// the real strategies do, which avoids the reflection and unblocks the writer +// goroutine. +type recordingStrategy struct { + OutputStrategy // OutputConfig/OutputManifest are unused by this test. + + desc ocispec.Descriptor + err error + + called bool + mediaType string + relPath string + destPath string + size int64 + gotReader bool +} + +func (r *recordingStrategy) OutputLayer(_ context.Context, mediaType, relPath, destPath, _ string, size int64, reader io.Reader, _ hooks.Hooks) (ocispec.Descriptor, error) { + r.called = true + r.mediaType, r.relPath, r.destPath, r.size = mediaType, relPath, destPath, size + r.gotReader = reader != nil + if reader != nil { + _, _ = io.Copy(io.Discard, reader) + } + return r.desc, r.err +} + func (s *BuilderTestSuite) TestBuildLayer() { s.Run("successful build layer", func() { expectedDesc := ocispec.Descriptor{ @@ -123,11 +156,19 @@ func (s *BuilderTestSuite) TestBuildLayer() { Size: 100, } - s.mockOutputStrategy.On("OutputLayer", mock.Anything, "test/media-type.tar", "test-file.txt", "", mock.AnythingOfType("string"), mock.AnythingOfType("int64"), mock.AnythingOfType("*io.PipeReader"), mock.Anything). - Return(expectedDesc, nil) + // Use a hand-written strategy instead of the testify mock: BuildLayer + // passes OutputLayer a live *io.PipeReader, and the mock's argument + // diffing formats it with fmt, racing with the pipe's writer goroutine. + strategy := &recordingStrategy{desc: expectedDesc} + s.builder.strategy = strategy desc, err := s.builder.BuildLayer(context.Background(), "test/media-type.tar", s.tempDir, s.tempFile, "", hooks.NewHooks()) s.NoError(err) + s.True(strategy.called) + s.Equal("test/media-type.tar", strategy.mediaType) + s.Equal("test-file.txt", strategy.relPath) + s.Equal("", strategy.destPath) + s.True(strategy.gotReader) s.Equal(expectedDesc.MediaType, desc.MediaType) s.Equal(expectedDesc.Digest, desc.Digest) s.Equal(expectedDesc.Size, desc.Size)