Skip to content

Commit

Permalink
Fix flaky TestReusableGoroutinesPool test (#596)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienduchesne authored Oct 4, 2024
1 parent 687ec48 commit a711ce3
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 12 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/find-flaky-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ jobs:
matrix:
runs: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20] # Run the build workflow 20 times
uses: ./.github/workflows/test-build.yml
with:
concurrency-group: '-find-flaky-tests-${{ matrix.runs }}'
6 changes: 5 additions & 1 deletion .github/workflows/test-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ on:
- main
pull_request:
workflow_call:
inputs:
concurrency-group:
required: true
type: string

concurrency:
# Cancel any running workflow for the same branch when new commits are pushed.
# We group both by ref_name (available when CI is triggered by a push to a branch/tag)
# and head_ref (available when CI is triggered by a PR).
group: "${{ github.ref_name }}-${{ github.head_ref }}"
group: "${{ github.ref_name }}-${{ github.head_ref }}${{ inputs.concurrency-group }}"
cancel-in-progress: true

jobs:
Expand Down
35 changes: 24 additions & 11 deletions concurrency/worker_test.go
Original file line number Diff line number Diff line change
@@ -1,48 +1,61 @@
package concurrency

import (
"regexp"
"runtime"
"strings"
"testing"
"time"

"github.com/stretchr/testify/require"
)

func TestReusableGoroutinesPool(t *testing.T) {
baseGoroutines := runtime.NumGoroutine()
const workers = 2
buf := make([]byte, 1<<20)
buf = buf[:runtime.Stack(buf, false)]
testGoroutine := regexp.MustCompile(`goroutine (\d+) \[running\]:`).FindSubmatch(buf)[1]
require.NotEmpty(t, testGoroutine, "test goroutine not found")

countGoroutines := func() int {
buf := make([]byte, 1<<20)
buf = buf[:runtime.Stack(buf, true)]
// Count the number of goroutines created by this test
// This ensures that the test isn't affected by leaked goroutines from other tests.
return strings.Count(string(buf), " in goroutine "+string(testGoroutine))
}

w := NewReusableGoroutinesPool(workers)
require.Equal(t, baseGoroutines+workers, runtime.NumGoroutine())
const workerCount = 2
w := NewReusableGoroutinesPool(workerCount)
require.Equal(t, workerCount, countGoroutines())

// Wait a little bit so both goroutines would be waiting on the jobs chan.
time.Sleep(10 * time.Millisecond)

ch := make(chan struct{})
w.Go(func() { <-ch })
require.Equal(t, baseGoroutines+workers, runtime.NumGoroutine())
require.Equal(t, workerCount, countGoroutines())
w.Go(func() { <-ch })
require.Equal(t, baseGoroutines+workers, runtime.NumGoroutine())
require.Equal(t, workerCount, countGoroutines())
w.Go(func() { <-ch })
require.Equal(t, baseGoroutines+workers+1, runtime.NumGoroutine())
require.Equal(t, workerCount+1, countGoroutines())

// end workloads, we should have only the workers again.
close(ch)
for i := 0; i < 1000; i++ {
if runtime.NumGoroutine() == baseGoroutines+workers {
if countGoroutines() == workerCount {
break
}
time.Sleep(time.Millisecond)
}
require.Equal(t, baseGoroutines+workers, runtime.NumGoroutine())
require.Equal(t, workerCount, countGoroutines())

// close the workers, eventually they should be gone.
w.Close()
for i := 0; i < 1000; i++ {
if runtime.NumGoroutine() == baseGoroutines {
if countGoroutines() == 0 {
return
}
time.Sleep(time.Millisecond)
}
t.Fatalf("expected %d goroutines after closing, got %d", baseGoroutines, runtime.NumGoroutine())
t.Fatalf("expected %d goroutines after closing, got %d", 0, countGoroutines())
}

0 comments on commit a711ce3

Please sign in to comment.