mesh-build-go: run a unit level's lint beside its tests under one bound, so its wall time is the longer of the two; a lint failure still fails the level (issue 1921, try speed S4)

This commit is contained in:
2026-10-25 01:04:35 +02:00
parent 2e69099354
commit d5d6fd7412
5 changed files with 82 additions and 63 deletions
@@ -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"`
@@ -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
@@ -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)
}
}
+32 -19
View File
@@ -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 {
@@ -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"}