From b83a1abe5f00c6a337e20e58aec8d375abb6fe6f Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 2 Nov 2026 18:24:08 +0100 Subject: [PATCH] mesh-build-go: one check ask names every failure, so a group's faults are not found one run at a time (ADR 0591) A unit level whose gofmt or vet fails still runs its tests, and the real-go integration tests expect that. The runner's askrun change (mesh-sdk 43788bb) arrived with the move. Cherry-picks of 06358f94 and 831f34e5, without the pin 86be0149. Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL --- .../cmd/mesh-build-go/lint_test.go | 13 ++++++---- .../mesh-build-go/cmd/mesh-build-go/test.go | 9 +++---- .../test_real_integration_test.go | 24 ++++++++++++------- 3 files changed, 26 insertions(+), 20 deletions(-) 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 f5cef2eb..3836304a 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 @@ -72,7 +72,8 @@ 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. +// 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") tree := filepath.Join(m.Work, ws, "tree") @@ -85,11 +86,13 @@ 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. + // Review round 2 item 5: before the level's tests, which still run once gofmt failed. + tested := false for _, c := range *calls { - if strings.Contains(c.line, " test -json") { - t.Errorf("the tests ran after gofmt failed: %s", c.line) - } + tested = tested || strings.Contains(c.line, " test -json") + } + if !tested { + t.Errorf("the tests did not run after gofmt failed: %v", *calls) } } 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 ece2c4a3..f02dcc07 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/test.go @@ -250,7 +250,8 @@ func (m *Module) runLevel(ctx context.Context, tree string, o levelOpts) (*TestR } 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. + // 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...) @@ -259,11 +260,7 @@ func (m *Module) runLevel(ctx context.Context, tree string, o levelOpts) (*TestR return nil, err } res.Lint = m.clock().Sub(linted).Seconds() - if len(failed) > 0 { - res.Failed = failed - res.judge(o.bounds) - return res, nil - } + res.Failed = failed began = m.clock() // the level's running time is its tests' } runs := make([]dirRun, len(todo)) 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 8ece1554..2b04bc9a 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 @@ -63,13 +63,15 @@ func TestUnitLevelRunsTheRealGoOnTheFixtureRepositories(t *testing.T) { needTools(t, "the unit level on fixtures", "go", "git", "bwrap") want := map[string]struct { passed, noTests bool - failed string + failed []string // the packages of the failures, in order }{ - "failing": {failed: "fixture/failing/bad"}, + "failing": {failed: []string{"fixture/failing/bad"}}, "tagged": {passed: true, noTests: true}, "empty": {passed: true, noTests: true}, "slow": {passed: true}, - "broken": {failed: "."}, // go vet, before the tests since R4g (ADR 0529 §1), names the directory and the compiler's line + // go vet, before the tests since R4g (ADR 0529 §1), names the directory and the compiler's line; the tests still + // run after it (ADR 0591), and the package that does not build is their failure. + "broken": {failed: []string{".", "fixture/broken"}}, } for name, w := range want { home := t.TempDir() @@ -87,14 +89,18 @@ func TestUnitLevelRunsTheRealGoOnTheFixtureRepositories(t *testing.T) { if err != nil { t.Fatalf("%s: %v", name, err) } - if res.Passed != w.passed || res.NoTests != w.noTests || (w.failed != "" && (len(res.Failed) != 1 || res.Failed[0].Package != w.failed)) { - t.Errorf("%s: result = %+v", name, res) + var failed []string + for _, f := range res.Failed { + failed = append(failed, f.Package) + } + if res.Passed != w.passed || res.NoTests != w.noTests || strings.Join(failed, " ") != strings.Join(w.failed, " ") { + t.Errorf("%s: result = %+v, want the failures of %v", name, res, w.failed) } } } -// 1887/D7-go against the real go: a tree holding an unformatted file fails the unit level naming the file, before any -// test runs, and the level's result carries the lint's time. +// 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) { needTools(t, "the unit level's gofmt and vet", "go", "git", "bwrap") home := t.TempDir() @@ -114,8 +120,8 @@ 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 != 0 { - t.Fatalf("result = %+v, want the level failed by gofmt naming ugly.go and no test run", res) + 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) } }