From 663d65640dbe689d6e70d46ec7b8eebfdebd5667 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 11 Nov 2026 14:32:41 +0100 Subject: [PATCH] A-C3: gofmt and vet are the lint step's, not the unit level's (issue 1966) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lint is a pipeline step of its own, served by the build seat (ADR 0666 section 7; design 47 *The steps and who serves them* line 571 at hq b229f5c4), so the unit level runs tests alone and the lint step runs gofmt and go vet, with every tag the files name, over each go.mod directory of the artifact's module directory; a module directory holding no go.mod has nothing to lint (prepare finding F12). Milestone 47/G, research 086 area A. Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ (Joined from a46e01a2.) Builds: design 47 G §328, §417 (BLD6) Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ --- .../mesh-build-go/cmd/mesh-build-go/asks.go | 1 + .../mesh-build-go/cmd/mesh-build-go/levels.go | 2 - .../mesh-build-go/cmd/mesh-build-go/lint.go | 49 ++++++++++++++++--- .../cmd/mesh-build-go/lint_test.go | 36 +++++++++----- .../mesh-build-go/cmd/mesh-build-go/test.go | 15 ------ .../test_real_integration_test.go | 16 ++++-- 6 files changed, 77 insertions(+), 42 deletions(-) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/asks.go b/modules/mesh-build-go/cmd/mesh-build-go/asks.go index fa751589a..6d4430e1f 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/asks.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/asks.go @@ -56,6 +56,7 @@ func (m *Module) askSteps(steps []StepTool) buildstep.AskSteps { Has: m.Has, Release: func(ws string) error { return m.spaces().Release(ws) }, Logf: m.Say, + Lint: m.lintStep, Node: m.Node, SayStep: m.sayStep(), } 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 bfc2a1d7a..903636356 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/levels.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/levels.go @@ -54,8 +54,6 @@ 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 float64 `json:"lint,omitempty"` // Bound and SetupBound are the bounds the level was judged by, in seconds. Bound float64 `json:"bound"` SetupBound float64 `json:"setup_bound,omitempty"` 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 8a9f4fc1c..90138afad 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/lint.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/lint.go @@ -19,11 +19,12 @@ import ( "git.novox.be/novox/mesh-sdk/go/forgekey" ) -// **gofmt and go vet are the unit level's** (novox/hq ADR 0529 §1; issue 1887 D7-go): what every repository's script ran -// beside its tests, run for every repository by the build module that runs its unit level. In each go.mod directory the -// level runs, every Go file outside testdata and vendor must be as gofmt writes it, and `go vet ./...` must pass with no -// tag and with every build tag the directory's files name together (integration, isolation, ...), so code behind a tag -// nothing else compiles is vetted too. A failure is the level's, naming the files or vet's lines. +// **gofmt and go vet are the pipeline step lint's** (novox/hq ADR 0666 §7; design 47 *The steps and who serves them*, +// line 571 at b229f5c4: lint is a step of its own, served by the build seat; ADR 0529 §1 had them in the unit level). In +// each go.mod directory of the artifact's module directory, every Go file outside testdata and vendor must be as gofmt +// writes it, and `go vet ./...` must pass with no tag and with every build tag the directory's files name together +// (integration, isolation, ...), so code behind a tag nothing else compiles is vetted too. A failure is the step's, +// naming the files or vet's lines; the unit level runs tests alone. // buildTag is one name in a //go:build line. var buildTag = regexp.MustCompile(`[A-Za-z_][A-Za-z0-9_.]*`) @@ -134,8 +135,7 @@ func (m *Module) lint(ctx context.Context, run forgekey.Runner, tree, d string, // 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) { // The ask's phases say it (issue 1890): lint of the directories, then their tests. - buildstep.Phase(ctx, fmt.Sprintf("unit: lint of %d directories", len(dirs))) - defer buildstep.Phase(ctx, fmt.Sprintf("unit: tests of %d directories", len(dirs))) + buildstep.Phase(ctx, fmt.Sprintf("lint of %d directories", len(dirs))) type answer struct { failed []Failure err error @@ -174,3 +174,38 @@ func (m *Module) lintAll(ctx context.Context, run forgekey.Runner, tree string, } return failed, nil } + +// lintStep is the body of the pipeline step lint (buildstep.AskSteps.Lint): gofmt and go vet of every go.mod directory of +// the fetched workspace under the module's directory dir (the whole tree when dir is empty), answering a +// buildstep.LintAnswer; a tree with no go.mod there has nothing to lint. +func (m *Module) lintStep(ctx context.Context, args map[string]any) (any, error) { + workspace, _ := args["workspace"].(string) + dir, _ := args["dir"].(string) + if !workspaceOK.MatchString(workspace) { + return nil, fmt.Errorf("%q is not a workspace id", workspace) + } + tree := filepath.Join(m.Work, workspace, "tree") + all, err := m.goModDirs(ctx, tree) + if err != nil { + return nil, err + } + dir = strings.Trim(filepath.ToSlash(filepath.Clean(dir)), "/") + var dirs []string + for _, d := range all { + if dir == "" || dir == "." || d == dir || strings.HasPrefix(d, dir+"/") { + dirs = append(dirs, d) + } + } + if len(dirs) == 0 { + return buildstep.LintAnswer{Nothing: true}, nil + } + run, err := m.stepRunner(workspace, false) + if err != nil { + return nil, err + } + failed, err := m.lintAll(ctx, run, tree, dirs, func(d string) []string { return m.goEnvFor(filepath.Join(tree, d)) }) + if err != nil { + return nil, err + } + return buildstep.LintAnswer{Failed: 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 3836304af..efd981bff 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 @@ -9,6 +9,7 @@ import ( "testing" "time" + "git.novox.be/novox/mesh-catalog/modules/mesh-build-go/buildstep" "git.novox.be/novox/mesh-sdk/go/forgekey" ) @@ -72,10 +73,11 @@ 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; its tests -// still run, so one check names the lint's failure and every failing test (the operator, 2026-11-02). -func TestAnUnformattedFileFailsTheUnitLevel(t *testing.T) { - m, states, ws, calls := workspaceFor(t, "empty") +// 086 A-C3 (design 47 *The steps and who serves them*, line 571 at b229f5c4; ADR 0666 §7): gofmt and vet are the lint +// step's, not the unit level's: a tree holding an unformatted file passes its unit level, and the lint step's body fails +// naming the file; a module directory holding no go.mod has nothing to lint. +func TestAnUnformattedFileFailsTheLintStepAndNotTheUnitLevel(t *testing.T) { + 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 { @@ -83,16 +85,24 @@ func TestAnUnformattedFileFailsTheUnitLevel(t *testing.T) { } } res, _ := runUnit(t, m, states, ws) - 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) + for _, f := range res.Failed { + if f.Test == "gofmt" { + t.Errorf("the unit level still lints: %+v", res.Failed) + } } - // Review round 2 item 5: before the level's tests, which still run once gofmt failed. - tested := false - for _, c := range *calls { - tested = tested || strings.Contains(c.line, " test -json") - } - if !tested { - t.Errorf("the tests did not run after gofmt failed: %v", *calls) + for _, row := range []struct { + dir string + failed bool + nothing bool + }{{"", true, false}, {"modules/none", false, true}} { + out, err := m.lintStep(context.Background(), map[string]any{"workspace": ws, "dir": row.dir}) + if err != nil { + t.Fatal(err) + } + la := out.(buildstep.LintAnswer) + if got := len(la.Failed) == 1 && la.Failed[0].Test == "gofmt" && strings.Contains(la.Failed[0].Output, "x.go"); got != row.failed || la.Nothing != row.nothing { + t.Errorf("lint of %q = %+v, want failed %v nothing %v", row.dir, la, row.failed, row.nothing) + } } } 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 f9dcccb74..3ca5d79be 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/test.go @@ -280,21 +280,6 @@ func (m *Module) runLevel(ctx context.Context, tree string, o levelOpts) (*TestR todo = append(todo, d) } } - 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, named first, and the tests still run, so one check names the lint's - // failures and every failing test (the operator, 2026-11-02); 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...) - }) - if err != nil { - return nil, err - } - res.Lint = m.clock().Sub(linted).Seconds() - res.Failed = failed - began = m.clock() // the level's running time is its tests' - } runs := make([]dirRun, len(todo)) slots := make(chan struct{}, m.parallel()) var wg sync.WaitGroup diff --git a/modules/mesh-build-go/cmd/mesh-build-go/test_real_integration_test.go b/modules/mesh-build-go/cmd/mesh-build-go/test_real_integration_test.go index 2b04bc9a6..369b38491 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/test_real_integration_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/test_real_integration_test.go @@ -14,6 +14,8 @@ import ( "strings" "testing" "time" + + "git.novox.be/novox/mesh-catalog/modules/mesh-build-go/buildstep" ) // TestMain makes this test binary the bus when the module starts it as one (bus.go). @@ -99,9 +101,9 @@ func TestUnitLevelRunsTheRealGoOnTheFixtureRepositories(t *testing.T) { } } -// 1887/D7-go against the real go: a tree holding an unformatted file fails the unit level naming the file first, its -// tests still run after it (ADR 0591: one check names every failure), and the level's result carries the lint's time. -func TestAnUnformattedFileFailsTheRealUnitLevel(t *testing.T) { +// 086 A-C3 against the real go (design 47 line 571 at b229f5c4): a tree holding an unformatted file passes its unit +// level, its two tests run, and the lint step's body fails naming the file. +func TestAnUnformattedFileFailsTheRealLintStepAndNotTheUnitLevel(t *testing.T) { needTools(t, "the unit level's gofmt and vet", "go", "git", "bwrap") home := t.TempDir() m := &Module{Home: home, Work: t.TempDir(), Cache: t.TempDir()} @@ -120,8 +122,12 @@ func TestAnUnformattedFileFailsTheRealUnitLevel(t *testing.T) { if err != nil { t.Fatal(err) } - if res.Passed || len(res.Failed) == 0 || res.Failed[0].Test != "gofmt" || !strings.Contains(res.Failed[0].Output, "ugly.go") || res.Tests != 2 || res.Lint <= 0 { - t.Fatalf("result = %+v, want the level failed by gofmt naming ugly.go, then the fixture's two tests run", res) + if !res.Passed || len(res.Failed) != 0 || res.Tests != 2 { + t.Fatalf("result = %+v, want the level passed with the fixture's two tests", res) + } + out, err := m.lintStep(context.Background(), map[string]any{"workspace": "ws1"}) + if la, _ := out.(buildstep.LintAnswer); err != nil || len(la.Failed) == 0 || la.Failed[0].Test != "gofmt" || !strings.Contains(la.Failed[0].Output, "ugly.go") { + t.Fatalf("lint = %+v, %v; want gofmt naming ugly.go", out, err) } }