diff --git a/modules/mesh-build-go/cmd/mesh-build-go/levels.go b/modules/mesh-build-go/cmd/mesh-build-go/levels.go index d247f1a9..05f20600 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/levels.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/levels.go @@ -73,7 +73,7 @@ type TestResult struct { Elapsed float64 `json:"elapsed"` // Setup is the level's setup time in seconds, when it has a setup. Setup float64 `json:"setup,omitempty"` - // Lint is the unit level's gofmt and go vet time in seconds, before its tests (ADR 0529 §1). + // Lint is the unit level's gofmt and go vet time in seconds, run beside its tests (ADR 0529 §1; issue 1921). Lint float64 `json:"lint,omitempty"` // Bound and SetupBound are the bounds the level was judged by, in seconds. Bound float64 `json:"bound"` 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 6accd280..75240ec5 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/lint.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/lint.go @@ -188,14 +188,17 @@ func (m *Module) lint(ctx context.Context, run forgekey.Runner, tree, d string, // 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). byDir names -// the packages the level runs in each directory (packagesByDir); nil lints each directory whole. -func (m *Module) lintAll(ctx context.Context, run forgekey.Runner, tree string, dirs []string, env func(d string) []string, byDir map[string][]string) ([]Failure, error) { +// the packages the level runs in each directory (packagesByDir); nil lints each directory whole. slots is the bound the +// lint shares with the level's tests; nil gives it one of its own. +func (m *Module) lintAll(ctx context.Context, run forgekey.Runner, tree string, dirs []string, env func(d string) []string, byDir map[string][]string, slots chan struct{}) ([]Failure, error) { type answer struct { failed []Failure err error } answers := make([]answer, len(dirs)) - slots := make(chan struct{}, m.parallel()) + if slots == nil { + 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 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 ec4ac01a..3f0d3126 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 @@ -10,6 +10,7 @@ import ( "testing" "time" + "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/forgekey" ) @@ -76,7 +77,7 @@ func TestTheUnitLevelRunsGofmtAndVetWithEveryTag(t *testing.T) { // 1887/D7-go: through the step, the unit level of a tree holding an unformatted file fails, naming the file. func TestAnUnformattedFileFailsTheUnitLevel(t *testing.T) { - m, states, ws, calls := workspaceFor(t, "empty") + m, states, ws, _ := workspaceFor(t, "empty") tree := filepath.Join(m.Work, ws, "tree") for name, body := range map[string]string{"go.mod": "module x\n", "x.go": "package x\nfunc X() {}\n"} { if err := os.WriteFile(filepath.Join(tree, name), []byte(body), 0o644); err != nil { @@ -87,51 +88,49 @@ func TestAnUnformattedFileFailsTheUnitLevel(t *testing.T) { if res.Passed || len(res.Failed) != 1 || res.Failed[0].Test != "gofmt" || !strings.Contains(res.Failed[0].Output, "x.go") { t.Fatalf("result = %+v, want the level failed naming x.go", res) } - // Review round 2 item 5: before the level's tests, which do not run once gofmt failed. - for _, c := range *calls { - if strings.Contains(c.line, " test -json") { - t.Errorf("the tests ran after gofmt failed: %s", c.line) - } - } } -// 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])} +// 1921/S4: the lint runs beside the unit level's tests, not before them: both have started before either ends, a lint +// failure still fails the level, and the result reports the lint's time and the tests' failures both. +func TestTheLintRunsBesideTheUnitTestsAndItsFailureStillFailsTheLevel(t *testing.T) { + m, states, ws, _ := workspaceFor(t, "failing") + tree := filepath.Join(m.Work, ws, "tree") + for name, body := range map[string]string{"go.mod": "module fixture/failing\n", "x.go": "package x\nfunc X() {}\n"} { + if err := os.WriteFile(filepath.Join(tree, name), []byte(body), 0o644); err != nil { + t.Fatal(err) } - return forgekey.Ran{} } - m := &Module{Parallel: 3} - failed, err := m.lintAll(context.Background(), run, tree, dirs, func(string) []string { return nil }, nil) - if err != nil { - t.Fatal(err) + m.Parallel = 4 + inner := m.Run + var vetStarted, testStarted sync.Once + vet, test := make(chan struct{}), make(chan struct{}) + timeout := time.After(2 * time.Second) + m.Run = func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + line := strings.Join(args, " ") + switch { + case strings.Contains(line, " vet "): + vetStarted.Do(func() { close(vet) }) + select { // the vet ends only once a test has started + case <-test: + case <-timeout: + t.Error("no test started while the vet ran") + } + case strings.Contains(line, " test -json"): + testStarted.Do(func() { close(test) }) + select { + case <-vet: + case <-timeout: + t.Error("no vet started while the tests ran") + } + } + return inner(ctx, env, name, args...) } - if most < 2 || most > 3 { - t.Errorf("%d lint commands ran at once, want 2 or 3 (the bound)", most) + res, st := runUnit(t, m, states, ws) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) } - if len(failed) != 2 || failed[0].Package != "b" || failed[1].Package != "e" { - t.Errorf("failed = %+v, want b's then e's", failed) + if res.Passed || len(res.Failed) < 2 || res.Failed[0].Test != "gofmt" || res.Failed[1].Test != "TestThreeIsFour" { + t.Fatalf("failed = %+v, want gofmt's failure, then the test's", res.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 97a86bbc..931192d2 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/test.go @@ -205,29 +205,35 @@ func (m *Module) runLevel(ctx context.Context, tree string, o levelOpts) (*TestR todo = append(todo, d) } } + // The level's commands share one bound on processes: the lint's and the tests' (issue 1921, try speed S4). + slots := make(chan struct{}, m.parallel()) + var lintFailed []Failure + var lintErr error + var lintDone chan struct{} + var crash any // a panic of a run or of the lint goes on in the caller, which cleans up after it + var crashMu sync.Mutex if o.level == LevelUnit { - // 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() - failed, err := m.lintAll(ctx, run, tree, todo, func(d string) []string { - return append(m.goEnvFor(filepath.Join(tree, d)), o.env...) - }, byDir) - if err != nil { - return nil, err - } - res.Lint = m.clock().Sub(linted).Seconds() - if len(failed) > 0 { - res.Failed = failed - res.judge(o.bounds) - return res, nil - } - began = m.clock() // the level's running time is its tests' + // gofmt and go vet of each directory the level runs, beside its tests (ADR 0529 §1; issue 1887 D7-go; issue 1921, + // try speed S4): a directory that fails them fails the level, and their time is the result's Lint. + lintDone = make(chan struct{}) + go func() { + defer close(lintDone) + defer func() { + if p := recover(); p != nil { + crashMu.Lock() + crash = p + crashMu.Unlock() + } + }() + linted := m.clock() + lintFailed, lintErr = m.lintAll(ctx, run, tree, todo, func(d string) []string { + return append(m.goEnvFor(filepath.Join(tree, d)), o.env...) + }, byDir, slots) + res.Lint = m.clock().Sub(linted).Seconds() + }() } runs := make([]dirRun, len(todo)) - slots := make(chan struct{}, m.parallel()) var wg sync.WaitGroup - var crash any // a panic of a run goes on in the caller, which cleans up after it - var crashMu sync.Mutex for i, d := range todo { wg.Add(1) slots <- struct{}{} @@ -258,9 +264,16 @@ func (m *Module) runLevel(ctx context.Context, tree string, o levelOpts) (*TestR }() } wg.Wait() + if lintDone != nil { + <-lintDone + } if crash != nil { panic(crash) } + if lintErr != nil { + return nil, lintErr + } + res.Failed = append(res.Failed, lintFailed...) // the lint's failures first, then the tests' for i, d := range todo { dr := runs[i] if (dr.r.Status != 0 || dr.r.Err != "") && len(dr.rd.pkgs) == 0 { diff --git a/modules/mesh-build-go/cmd/mesh-build-go/test_test.go b/modules/mesh-build-go/cmd/mesh-build-go/test_test.go index aa6d1d1a..d55f9609 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/test_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/test_test.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "time" @@ -25,8 +26,11 @@ func capturedRunner(t *testing.T, captured string, calls *[]cmdRecord) forgekey. if strings.Contains(string(raw), `"Action":"fail"`) { status = 1 // go test exits 1 when a test or a build failed } + var mu sync.Mutex // the lint and the tests of a level run at once (issue 1921, try speed S4) return func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + mu.Lock() *calls = append(*calls, cmdRecord{env: env, line: name + " " + strings.Join(args, " ")}) + mu.Unlock() switch { case name == "git" && args[2] == "ls-files": return forgekey.Ran{Stdout: "go.mod\n"}