Run gofmt and go vet before the Go unit level's tests and give their time in its result, so a lint failure ends the level at once and the check's line can say what it cost (issue 1887 D7-go, review round 2 item 5; ADR 0529 §1)

This commit is contained in:
2026-10-23 16:44:22 +02:00
parent 0066b702f4
commit 14b7158fc0
5 changed files with 59 additions and 12 deletions
@@ -70,6 +70,8 @@ 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"`
@@ -72,7 +72,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, _ := workspaceFor(t, "empty")
m, states, ws, calls := 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,4 +83,10 @@ 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)
}
}
}
+20 -10
View File
@@ -197,6 +197,26 @@ 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 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...)
}
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'
}
runs := make([]dirRun, len(todo))
slots := make(chan struct{}, m.parallel())
var wg sync.WaitGroup
@@ -253,16 +273,6 @@ func (m *Module) runLevel(ctx context.Context, tree string, o levelOpts) (*TestR
if o.wall {
res.Elapsed = m.clock().Sub(began).Seconds()
}
if o.level == LevelUnit {
// gofmt and go vet of each directory the level ran are the unit level's (ADR 0529 §1; issue 1887 D7-go).
for _, d := range todo {
failed, err := m.lint(ctx, run, tree, d, append(m.goEnvFor(filepath.Join(tree, d)), o.env...))
if err != nil {
return nil, err
}
res.Failed = append(res.Failed, failed...)
}
}
res.judge(o.bounds)
return res, nil
}
@@ -66,7 +66,7 @@ func TestUnitLevelRunsTheRealGoOnTheFixtureRepositories(t *testing.T) {
"tagged": {passed: true, noTests: true},
"empty": {passed: true, noTests: true},
"slow": {passed: true},
"broken": {failed: "fixture/broken"},
"broken": {failed: "."}, // go vet, before the tests since R4g (ADR 0529 §1), names the directory and the compiler's line
}
for name, w := range want {
home := t.TempDir()
@@ -90,6 +90,32 @@ 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, before any
// test runs, 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()
m := &Module{Home: home, Work: t.TempDir(), Cache: t.TempDir()}
tree := filepath.Join(m.Work, "ws1", "tree")
if err := os.MkdirAll(filepath.Dir(tree), 0o755); err != nil {
t.Fatal(err)
}
if err := os.Rename(repository(t, "slow"), tree); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(tree, "ugly.go"), []byte("package slow\nfunc Ugly() {}\n"), 0o644); err != nil {
t.Fatal(err)
}
m.Run = StepRunner(home)
res, err := m.runLevel(context.Background(), tree, levelOpts{workspace: "ws1", level: LevelUnit, bounds: Bounds{Run: UnitBound}})
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)
}
}
// processesNamed are the processes whose command line holds text.
func processesNamed(text string) []string {
var out []string
@@ -188,6 +188,9 @@ func TestUnitLevelRunsGoTestJsonInTheModulesDirectoryWithTheStepsEnvironment(t *
func TestAGoTestThatCouldNotRunEndsTheStepFailed(t *testing.T) {
m, states, ws, _ := workspaceFor(t, "empty")
m.Run = throughBubblewrap(t, func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran {
if name == "go" && len(args) > 2 && args[2] == "vet" {
return forgekey.Ran{} // vet, before the tests, passes: this test is of go test that could not run
}
if name == "go" {
return forgekey.Ran{Status: 1, Stderr: "go: cannot find main module\n"}
}