novox/mesh-sdk
18
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

buildstep: a check ask runs every level and every package after a failure, so one check names every failure
mesh/delivery-group group fix/check-every-failure checking: 3 of 3 member(s) ready
Details
mesh/check checking 43788bbfdef3 on d6c87ee1a7ae | novox/mesh-sdk#160 in group fix/check-every-failure
mesh/delivery closed: the pull request closed unmerged
Details

Browse Source 
The operator, 2026-11-02: a failed check showed only its first fault; a group's three faults took three runs.
The change's own packages still run first (issue 1289), and nothing after them is skipped.

Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL
This commit is contained in:
jschoubben
2026-11-02 16:31:55 +01:00
parent d6c87ee1a7
commit 43788bbfde
4 changed files with 39 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
go/buildstep/askrun.go
+7 -15
View File
@@ -213,16 +213,15 @@ func (s AskSteps) check(ctx context.Context, job delivery.CheckJob) (any, error)
if job.Runner != "" && (job.RunnerWhen == "" || changesUnder(f.Changed, job.RunnerWhen)) {
runner = job.Runner
}
// Every level runs, also after one that did not pass: one check names every failure, not only the first (the operator,
// 2026-11-02: "A failed check shows only its first fault").
for _, level := range job.Levels {
la := delivery.LevelAnswer{Level: level}
passed, err := s.level(ctx, job, level, f, runner, &la)
_, err := s.level(ctx, job, level, f, runner, &la)
answer.Levels = append(answer.Levels, la)
if err != nil {
return nil, err
}
if !passed {
return answer, nil // the first level that did not pass ends the check (ADR 0484 clause 13)
}
}
return answer, nil
}
@@ -261,10 +260,9 @@ func (s AskSteps) level(ctx context.Context, job delivery.CheckJob, level string
_ = json.Unmarshal(r, &t)
return t.Passed, nil
}
// The packages (or Node directories) whose tests the change touches run first, and a failure among them ends the level
// there, so a change's own failing test is answered without the rest of the suite (issue 1289; ADR 0444). Past them every
// test step of the level runs, also after one that did not pass, so one check names every failing test of the rest of
// the level and not only the first (issue 1921); the level passes only when each of its steps did.
// The packages (or Node directories) whose tests the change touches run first (issue 1289; ADR 0444), and then every
// other test step of the level, also after one that did not pass, so one check names every failing test of the level
// and not only the first (issue 1921; the operator, 2026-11-02); the level passes only when each of its steps did.
all := true
if s.Digest == nil {
// A module with no digest (Node) tests each module directory of the tree, those whose tests changed first.
@@ -279,9 +277,6 @@ func (s AskSteps) level(ctx context.Context, job delivery.CheckJob, level string
}
all = all && passed
}
if !all {
return false, nil // the change's own tests failed: the others are not run (issue 1289)
}
}
return all, nil
}
@@ -317,10 +312,7 @@ func (s AskSteps) level(ctx context.Context, job delivery.CheckJob, level string
if err != nil {
return false, fmt.Errorf("the %s level: %w", level, err)
}
all = all && passed // past the change's own packages the others still run (issue 1921)
if priority && !all {
return false, nil // the change's own tests failed: the others are not run (issue 1289)
}
all = all && passed // the others still run after a failure (issue 1921)
}
return all, nil
}
go/buildstep/askrun_test.go
+23 -26
View File
@@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"slices"
"strings"
"testing"
"time"
@@ -125,8 +126,8 @@ func TestRunAskRefusesAnotherToolchain(t *testing.T) {
}
// 1887/A3, A5a: a check ask runs the levels of one check after one fetch: it digests the unit level, takes the kept
// verdicts, tests the packages whose tests changed first and, when one of them fails, no other (issue 1289), and runs no
// level after one that did not pass (ADR 0484 clause 13).
// verdicts, tests the packages whose tests changed first and then the others, also after one of them failed, and runs
// every level, also after one that did not pass, so one check names every failure (the operator, 2026-11-02).
func TestRunAskChecksTheLevelsOfOneCheck(t *testing.T) {
r := &recorder{
fetch: map[string]any{"workspace": "/w", "changed": []string{"pkg/b/b_test.go"}},
@@ -141,16 +142,20 @@ func TestRunAskChecksTheLevelsOfOneCheck(t *testing.T) {
got := strings.Join(r.calls, "\n")
want := `fetch {"base":"` + runCommit + `","commit":"` + runCommit + `","repository":"novox/x"}
digest {"level":"unit","workspace":"/w"}
test {"level":"unit","packages":["x/pkg/b"],"workspace":"/w"}`
test {"level":"unit","packages":["x/pkg/b"],"workspace":"/w"}
test {"level":"unit","packages":["x/pkg/a"],"workspace":"/w"}
digest {"level":"integration","workspace":"/w"}
test {"level":"integration","packages":["x/pkg/b"],"workspace":"/w"}
test {"level":"integration","packages":["x/pkg/a"],"workspace":"/w"}`
if got != want {
t.Fatalf("calls:\n%s\nwant:\n%s", got, want)
}
a := out.(delivery.CheckAnswer)
if len(a.Levels) != 1 {
t.Fatalf("levels %+v, want the unit level alone", a.Levels)
if len(a.Levels) != 2 {
t.Fatalf("levels %+v, want both levels", a.Levels)
}
u := a.Levels[0]
if u.Level != delivery.LevelUnit || len(u.Kept) != 1 || u.Kept[0].Package != "x/pkg/k" || u.Kept[0].Tests != 7 || len(u.Tests) != 1 || u.Digest == nil {
if u.Level != delivery.LevelUnit || len(u.Kept) != 1 || u.Kept[0].Package != "x/pkg/k" || u.Kept[0].Tests != 7 || len(u.Tests) != 2 || u.Digest == nil {
t.Fatalf("the unit level %+v", u)
}
@@ -265,17 +270,17 @@ func TestRunAskChecksNodeModulesAndConflicts(t *testing.T) {
}
}
// Issue 1289: a failure in the packages whose tests changed ends the level there, the others are not run, and no later level
// runs. Issue 1921: past the change's own packages every package still runs, so two failing packages none of which the
// change touched are two test results of the one level. A Node check does the same with its module directories.
func TestRunAskStopsAtTheChangesOwnFailureAndRunsTheRestAfterOthers(t *testing.T) {
// Issue 1289: the packages whose tests changed run first. Since 2026-11-02 (the operator: "A failed check shows only its
// first fault") a failure among them ends nothing: every package of the level and every level still runs, so one check
// names every failing test (issue 1921). A Node check does the same with its module directories.
func TestRunAskRunsEveryPackageAndLevelAfterAFailure(t *testing.T) {
for _, row := range []struct {
name string
changed []string
tests map[string]bool
want []string // the packages tested, in order
want []string // the packages tested at each level, in order
}{
{"the change's own test fails: nothing else runs", []string{"pkg/b/b_test.go"}, map[string]bool{"x/pkg/a": false, "x/pkg/b": false}, []string{"x/pkg/b"}},
{"the change's own test fails: the rest still runs", []string{"pkg/b/b_test.go"}, map[string]bool{"x/pkg/a": false, "x/pkg/b": false}, []string{"x/pkg/b", "x/pkg/a"}},
{"the change's own test passes: the rest runs, also after a failure", []string{"pkg/b/b_test.go"}, map[string]bool{"x/pkg/a": false, "x/pkg/b": true, "x/pkg/c": false}, []string{"x/pkg/b", "x/pkg/a x/pkg/c"}},
{"no test changed: every failure is named", nil, map[string]bool{"x/pkg/a": false, "x/pkg/b": false}, []string{"x/pkg/a x/pkg/b"}},
} {
@@ -297,19 +302,11 @@ func TestRunAskStopsAtTheChangesOwnFailureAndRunsTheRestAfterOthers(t *testing.T
got = append(got, strings.Join(a.Packages, " "))
}
}
// the passing row goes on to the integration level, which tests the same packages again; the first level is what is judged
if n := len(row.want); len(got) < n || fmt.Sprint(got[:n]) != fmt.Sprint(row.want) {
t.Errorf("%s: tested %v, want %v first", row.name, got, row.want)
if want := append(slices.Clone(row.want), row.want...); fmt.Sprint(got) != fmt.Sprint(want) {
t.Errorf("%s: tested %v, want %v", row.name, got, want)
}
a := out.(delivery.CheckAnswer)
failed := false
for _, raw := range a.Levels[0].Tests {
var res struct{ Passed bool }
_ = json.Unmarshal(raw, &res)
failed = failed || !res.Passed
}
if failed && (len(a.Levels) != 1 || len(got) != len(row.want)) {
t.Errorf("%s: a failed level went on: levels %d, tests %v", row.name, len(a.Levels), got)
if a := out.(delivery.CheckAnswer); len(a.Levels) != 2 || len(a.Levels[0].Tests) != len(row.want) {
t.Errorf("%s: levels %+v, want both, each with every test result", row.name, a.Levels)
}
}
@@ -322,7 +319,7 @@ func TestRunAskStopsAtTheChangesOwnFailureAndRunsTheRestAfterOthers(t *testing.T
if err != nil {
t.Fatal(err)
}
if a := out.(delivery.CheckAnswer); len(r.calls) != 2 || len(a.Levels) != 1 || len(a.Levels[0].Tests) != 1 {
t.Fatalf("calls %v, levels %+v: want the changed directory b alone and no integration level", r.calls, a.Levels)
if a := out.(delivery.CheckAnswer); len(r.calls) != 5 || len(a.Levels) != 2 || len(a.Levels[0].Tests) != 2 {
t.Fatalf("calls %v, levels %+v: want b then a at each of the two levels", r.calls, a.Levels)
}
}
go/delivery/checker.go
+6 -5
View File
@@ -143,7 +143,7 @@ type FailedFirst struct {
Words string `json:"words,omitempty"`
}
// MaxFailures is the most entries a verdict's Failures holds (issue 1921): fifty tests named one by one, and an entry
// MaxFailures is the most entries a verdict's Failures holds (issue 1921): fifty failures named one by one, and an entry
// after them saying how many more failed.
const MaxFailures = 50
@@ -157,10 +157,11 @@ type Verdict struct {
Trunk map[string]string `json:"trunk,omitempty"`
State string `json:"state"`
Failed FailedFirst `json:"failed,omitzero"`
// Failures names every test that failed, in the order judged, each with the first line of its words (issue 1921):
// Failed keeps the first one's words whole, and a check that ran every test says all it found here. At most
// MaxFailures entries, the last of a longer list saying how many more failed. Absent in a value written before it,
// and from a verdict whose failure is no test.
// Failures names everything that failed, in the order judged, each with the first line of its words (issue 1921): every
// failed test, and every member's refused step, conflict, manifest refusal and the composition's failure, since a check
// runs every member, level and the composition to its end (the operator, 2026-11-02). Failed keeps the first one's
// words whole. At most MaxFailures entries, the last of a longer list saying how many more failed. Absent in a value
// written before it, and from a verdict with one failure that is no test.
Failures []FailedFirst `json:"failures,omitempty"`
Levels map[string]LevelResult `json:"levels,omitempty"`
Took Took `json:"took,omitzero"`
go/delivery/stepasks.go
+3 -2
View File
@@ -344,7 +344,8 @@ type AskArtifact struct {
// CheckJob is a check ask's job, the levels of one check (ADR 0525 §3): fetch the head merged onto its trunk once, then
// for each level in order digest its packages, take the kept verdicts for the digests that have one, and test the rest,
// the packages the change's own tests are in first, stopping at the first level that does not pass (CheckAnswer).
// the packages the change's own tests are in first, every level and package running to its end also after a failure,
// so one check names every failure (CheckAnswer).
type CheckJob struct {
// Fetch is the fetch tool's arguments (repository, commit, ref, base, and the build module's own: beside, package).
Fetch map[string]any `json:"fetch"`
@@ -564,7 +565,7 @@ type AskBuilt struct {
}
// CheckAnswer is a check ask's step result, each part the build module's own tool's answer whole: the fetch's, then one
// LevelAnswer per level run, in order; a level that did not pass is the last. The checker judges and keeps from it as it
// LevelAnswer per level run, in order, every asked level also after one that did not pass. The checker judges and keeps from it as it
// did from the tools' answers.
type CheckAnswer struct {
Fetch json.RawMessage `json:"fetch"`