diff --git a/modules/mesh-build-go/cmd/mesh-build-go/lint.go b/modules/mesh-build-go/cmd/mesh-build-go/lint.go index e493f12b..7fb41f5f 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/lint.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/lint.go @@ -12,6 +12,7 @@ import ( "regexp" "sort" "strings" + "sync" "git.novox.be/novox/mesh-sdk/go/forgekey" ) @@ -127,3 +128,45 @@ func (m *Module) lint(ctx context.Context, run forgekey.Runner, tree, d string, } return failed, nil } + +// lintAll lints each directory, up to m.parallel() at once like the level's tests; the failures join in directory order +// and the error is the first directory's, so the answer does not depend on which ended first (issue 1930). +func (m *Module) lintAll(ctx context.Context, run forgekey.Runner, tree string, dirs []string, env func(d string) []string) ([]Failure, error) { + type answer struct { + failed []Failure + err error + } + answers := make([]answer, len(dirs)) + slots := make(chan struct{}, m.parallel()) + var wg sync.WaitGroup + var crash any // a panic of a lint goes on in the caller, as a panic of a test run does + var crashMu sync.Mutex + for i, d := range dirs { + wg.Add(1) + slots <- struct{}{} + go func() { + defer wg.Done() + defer func() { <-slots }() + defer func() { + if p := recover(); p != nil { + crashMu.Lock() + crash = p + crashMu.Unlock() + } + }() + answers[i].failed, answers[i].err = m.lint(ctx, run, tree, d, env(d)) + }() + } + wg.Wait() + if crash != nil { + panic(crash) + } + var failed []Failure + for _, a := range answers { + if a.err != nil { + return nil, a.err + } + failed = append(failed, a.failed...) + } + return failed, nil +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/lint_test.go b/modules/mesh-build-go/cmd/mesh-build-go/lint_test.go index 602d68b7..fef621b0 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/lint_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/lint_test.go @@ -5,7 +5,9 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" + "time" "git.novox.be/novox/mesh-sdk/go/forgekey" ) @@ -90,3 +92,43 @@ func TestAnUnformattedFileFailsTheUnitLevel(t *testing.T) { } } } + +// Issue 1930: the lint of many directories runs side by side up to the level's bound, not one after another, and the +// failures still join in directory order. +func TestTheLintOfManyDirectoriesRunsAtOnceUpToTheBound(t *testing.T) { + files := map[string]string{} + var dirs []string + for _, d := range []string{"a", "b", "c", "d", "e", "f"} { + files[d+"/go.mod"] = "module x/" + d + "\n" + files[d+"/x.go"] = "package x\n" + dirs = append(dirs, d) + } + tree := writeTree(t, files) + var mu sync.Mutex + running, most := 0, 0 + run := func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + mu.Lock() + running++ + most = max(most, running) + mu.Unlock() + time.Sleep(30 * time.Millisecond) + mu.Lock() + running-- + mu.Unlock() + if strings.HasSuffix(args[1], "b") || strings.HasSuffix(args[1], "e") { + return forgekey.Ran{Status: 1, Stderr: "bad " + filepath.Base(args[1])} + } + return forgekey.Ran{} + } + m := &Module{Parallel: 3} + failed, err := m.lintAll(context.Background(), run, tree, dirs, func(string) []string { return nil }) + if err != nil { + t.Fatal(err) + } + if most < 2 || most > 3 { + t.Errorf("%d lint commands ran at once, want 2 or 3 (the bound)", most) + } + if len(failed) != 2 || failed[0].Package != "b" || failed[1].Package != "e" { + t.Errorf("failed = %+v, want b's then e's", failed) + } +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/test.go b/modules/mesh-build-go/cmd/mesh-build-go/test.go index 613fe61c..e7d9624f 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/test.go @@ -209,13 +209,11 @@ func (m *Module) runLevel(ctx context.Context, tree string, o levelOpts) (*TestR // gofmt and go vet of each directory the level runs come before its tests (ADR 0529 ยง1; issue 1887 D7-go): a // directory that fails them fails the level at once, and their time is the result's Lint. linted := m.clock() - var failed []Failure - for _, d := range todo { - f, err := m.lint(ctx, run, tree, d, append(m.goEnvFor(filepath.Join(tree, d)), o.env...)) - if err != nil { - return nil, err - } - failed = append(failed, f...) + failed, err := m.lintAll(ctx, run, tree, todo, func(d string) []string { + return append(m.goEnvFor(filepath.Join(tree, d)), o.env...) + }) + if err != nil { + return nil, err } res.Lint = m.clock().Sub(linted).Seconds() if len(failed) > 0 {