diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ca5972f1..068c9ebf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,4 +20,4 @@ jobs: run: go build -v ./... - name: Test - run: go test -v -cover ./... + run: go test -race -v -cover ./... diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..1ee26609 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,49 @@ +version: "2" + +linters: + default: standard + enable: + - asasalint + - asciicheck + - bodyclose + - contextcheck + - durationcheck + - errorlint + - gocognit + - gocritic + - gocyclo + - gosec + - misspell + - nilerr + - nilnil + - prealloc + - revive + - unparam + - wastedassign + + settings: + gocognit: + min-complexity: 30 + gocyclo: + min-complexity: 30 + gosec: + excludes: + - G115 + - G404 + - G304 + - G501 + - G401 + + exclusions: + rules: + - path: cmd/gateway/banner/banner\.go + linters: [gosec] + text: "G602" + - path: main\.go + linters: [gosec] + text: "G108|G114" + - path: pkg/preparation/preparation_test\.go + linters: [gosec] + text: "G301" + - linters: [errcheck] + source: "fmt\\.Fprint(f|ln)?\\((cmd\\.OutOrStdout|cmd\\.OutOrStderr|os\\.Stdout|os\\.Stderr|w)" diff --git a/pkg/preparation/preparation_test.go b/pkg/preparation/preparation_test.go index 100c8a2c..fbf8f83b 100644 --- a/pkg/preparation/preparation_test.go +++ b/pkg/preparation/preparation_test.go @@ -10,6 +10,7 @@ import ( "net/http" "os" "path/filepath" + "sync" "testing" "time" @@ -83,6 +84,7 @@ func prepareTestClient( uploadAddCaps *[]ucan.Capability[uploadcap.AddCaveats], ) *ctestutil.ClientWithCustomPut { t.Helper() + var capsMu sync.Mutex client := &ctestutil.ClientWithCustomPut{ Client: helpers.Must(ctestutil.Client( ctestutil.WithSpaceBlobAdd(), @@ -98,7 +100,9 @@ func prepareTestClient( inv invocation.Invocation, context server.InvocationContext, ) (result.Result[spaceindexcap.AddOk, failure.IPLDBuilderFailure], fx.Effects, error) { + capsMu.Lock() *indexCaps = append(*indexCaps, cap) + capsMu.Unlock() return result.Ok[spaceindexcap.AddOk, failure.IPLDBuilderFailure](spaceindexcap.AddOk{}), nil, nil }, ), @@ -114,7 +118,9 @@ func prepareTestClient( inv invocation.Invocation, context server.InvocationContext, ) (result.Result[spaceblobcap.ReplicateOk, failure.IPLDBuilderFailure], fx.Effects, error) { + capsMu.Lock() *replicateCaps = append(*replicateCaps, cap) + capsMu.Unlock() sitePromises := make([]types.Promise, cap.Nb().Replicas) for i := range sitePromises { siteDigest, err := multihash.Encode(fmt.Appendf(nil, "test-replicated-site-%d", i), multihash.IDENTITY) @@ -147,7 +153,9 @@ func prepareTestClient( inv invocation.Invocation, context server.InvocationContext, ) (result.Result[filecoincap.OfferOk, failure.IPLDBuilderFailure], fx.Effects, error) { + capsMu.Lock() *offerCaps = append(*offerCaps, cap) + capsMu.Unlock() return result.Ok[filecoincap.OfferOk, failure.IPLDBuilderFailure]( filecoincap.OfferOk{ Piece: cap.Nb().Piece, @@ -167,7 +175,9 @@ func prepareTestClient( inv invocation.Invocation, context server.InvocationContext, ) (result.Result[uploadcap.AddOk, failure.IPLDBuilderFailure], fx.Effects, error) { + capsMu.Lock() *uploadAddCaps = append(*uploadAddCaps, cap) + capsMu.Unlock() return result.Ok[uploadcap.AddOk, failure.IPLDBuilderFailure](uploadcap.AddOk{ Root: cap.Nb().Root, Shards: cap.Nb().Shards, diff --git a/pkg/preparation/uploads/worker_test.go b/pkg/preparation/uploads/worker_test.go index ae4b1a50..89058ad0 100644 --- a/pkg/preparation/uploads/worker_test.go +++ b/pkg/preparation/uploads/worker_test.go @@ -2,6 +2,7 @@ package uploads_test import ( "errors" + "sync/atomic" "testing" "time" @@ -26,31 +27,35 @@ func TestWorker(t *testing.T) { t.Run("runs the work function for every signal received, then the finalize function when the channel closes", func(t *testing.T) { signalChan := make(chan struct{}, 1) resultChan := make(chan error, 1) - var runs int - var finalizes int + var runs atomic.Int32 + var finalizes atomic.Int32 go func() { defer close(resultChan) resultChan <- uploads.Worker(t.Context(), signalChan, func() error { - runs++ + runs.Add(1) return nil }, func() error { - finalizes++ + finalizes.Add(1) return nil }) }() - require.Equal(t, 0, runs, "worker should not run before signal") + require.Equal(t, int32(0), runs.Load(), "worker should not run before signal") signalChan <- struct{}{} - e(t, func(t *assert.CollectT) { require.Equal(t, 1, runs, "worker should run once after signal") }) + e(t, func(t *assert.CollectT) { + require.Equal(t, int32(1), runs.Load(), "worker should run once after signal") + }) signalChan <- struct{}{} - e(t, func(t *assert.CollectT) { require.Equal(t, 2, runs, "worker should run again after second signal") }) + e(t, func(t *assert.CollectT) { + require.Equal(t, int32(2), runs.Load(), "worker should run again after second signal") + }) - require.Equal(t, 0, finalizes, "finalize function should be called until the channel closes") + require.Equal(t, int32(0), finalizes.Load(), "finalize function should be called until the channel closes") close(signalChan) e(t, func(t *assert.CollectT) { - require.Equal(t, 1, finalizes, "finalize function should be called once the channel closes") + require.Equal(t, int32(1), finalizes.Load(), "finalize function should be called once the channel closes") }) result := <-resultChan @@ -61,20 +66,20 @@ func TestWorker(t *testing.T) { workerErr := errors.New("error in doWork") signalChan := make(chan struct{}, 3) resultChan := make(chan error, 1) - var runs int - var finalizes int + var runs atomic.Int32 + var finalizes atomic.Int32 go func() { defer close(resultChan) resultChan <- uploads.Worker(t.Context(), signalChan, func() error { - runs++ + n := runs.Add(1) // Fail on the second run - if runs == 2 { + if n == 2 { return workerErr } return nil }, func() error { - finalizes++ + finalizes.Add(1) return nil }) }() @@ -88,20 +93,20 @@ func TestWorker(t *testing.T) { require.True(t, ok, "result should be a wrapped error") require.ErrorContains(t, result, "worker encountered an error: error in doWork") require.Equal(t, workerErr, result.Unwrap(), "worker should send back the error it encountered, wrapped") - require.Equal(t, 2, runs, "worker should have stopped after encountering an error") - require.Equal(t, 0, finalizes, "finalize function should not have be called") + require.Equal(t, int32(2), runs.Load(), "worker should have stopped after encountering an error") + require.Equal(t, int32(0), finalizes.Load(), "finalize function should not have be called") }) t.Run("responds with any finalize error", func(t *testing.T) { finalizerErr := errors.New("error in finalize") signalChan := make(chan struct{}, 3) resultChan := make(chan error, 1) - var runs int + var runs atomic.Int32 go func() { defer close(resultChan) resultChan <- uploads.Worker(t.Context(), signalChan, func() error { - runs++ + runs.Add(1) return nil }, func() error { return finalizerErr @@ -118,26 +123,26 @@ func TestWorker(t *testing.T) { require.True(t, ok, "result should be a wrapped error") require.ErrorContains(t, result, "worker finalize encountered an error: error in finalize") require.Equal(t, finalizerErr, result.Unwrap(), "worker should send back the error it encountered, wrapped") - require.Equal(t, 3, runs, "worker should have run all three times") + require.Equal(t, int32(3), runs.Load(), "worker should have run all three times") }) t.Run("ignores a nil finalizer", func(t *testing.T) { signalChan := make(chan struct{}, 1) resultChan := make(chan error, 1) - var ran bool + var ran atomic.Bool go func() { defer close(resultChan) resultChan <- uploads.Worker(t.Context(), signalChan, func() error { - ran = true + ran.Store(true) return nil }, nil) }() - require.False(t, ran, "worker should not run before signal") + require.False(t, ran.Load(), "worker should not run before signal") signalChan <- struct{}{} - e(t, func(t *assert.CollectT) { require.True(t, ran, "worker should run after signal") }) + e(t, func(t *assert.CollectT) { require.True(t, ran.Load(), "worker should run after signal") }) close(signalChan) result := <-resultChan require.Nil(t, result, "result should be nil after successful runs and no finalizer")