moricho / tparallel

tparallel finds inappropriate usage of `t.Parallel()` method in your Go test codes
MIT License
24 stars 4 forks source link

Crash on some projects #24

Open idkw opened 1 year ago

idkw commented 1 year ago

Some projects trigger this tparallel crash. I can't share the source code that triggers this crash however.

ERRO [runner] Panic: tparallel: package "xxxxxxxx" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference: goroutine 15881 [running]:
runtime/debug.Stack()
        runtime/debug/stack.go:24 +0x65
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1()
        github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:109 +0x285
panic({0xfc6c00, 0x1a97f70})
        runtime/panic.go:884 +0x213
github.com/moricho/tparallel.appendTestMap({0x1bf83b0?, 0x0, 0x0}, {0x13622a0?, 0xc007f14c30?})
        github.com/moricho/tparallel@v0.3.1/testmap.go:52 +0x65
github.com/moricho/tparallel.getTestMap(0xc014cf1080, {0x135ab10?, 0xc006951dc0})
        github.com/moricho/tparallel@v0.3.1/testmap.go:35 +0x2f8
github.com/moricho/tparallel.run(0xc011b6eb40)
        github.com/moricho/tparallel@v0.3.1/tparallel.go:41 +0x177
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc002186410)
        github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:195 +0xa25
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
        github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:113 +0x1d
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc001251590, {0x1128b80, 0x9}, 0xc002458f48)
        github.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:111 +0x4a
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc0004ac600?)
        github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:112 +0x85
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc002186410)
        github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0xb4
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
        github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x208 
WARN [runner] Can't run linter goanalysis_metalinter: goanalysis_metalinter: tparallel: package "xxxxxxxx" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference 
ERRO Running error: 1 error occurred:
        * can't run linter goanalysis_metalinter: goanalysis_metalinter: tparallel: package "xxxxxxxx" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference
idkw commented 1 year ago

@moricho I managed to find the culprit, here's a reproducer:

func TestParallel(t *testing.T) {
    t.Parallel()

    cases := map[string]struct {
        a string
        b string
    }{
        "a": {
            "a", "b",
        },
        "b": {
            "a", "b",
        },
    }

    wg := sync.WaitGroup{}
    for name, c := range cases {
        c := c
        wg.Add(1)
        go t.Run(name, func(t *testing.T) {
            defer wg.Done()
            require.Equal(t, "a", c.a)
            require.Equal(t, "b", c.b)
        })
    }
    wg.Wait()
}

Basically this is a test I have that require to run all its subtests in parallel. We can't rely on t.Parallel() because the default number of parallel tests depends on the number of CPU cores Here we want all subtests to be run in parallel to trigger the behavior we want to test

idkw commented 1 year ago

Found this workaround to avoid the crash:

        for name, c := range cases {
                name := name
        c := c
        wg.Add(1)
        go func() {
            t.Run(name, func(t *testing.T) {
                defer wg.Done()
                    require.Equal(t, "a", c.a)
                    require.Equal(t, "b", c.b)
            })
        }()
    }